From f5b3eea90fbde80c9b34777d831a0df1ac374d49 Mon Sep 17 00:00:00 2001 From: sidpremkumar Date: Jul 12 2019 22:19:05 +0000 Subject: [PATCH 1/4] Adding support to email owners in the situation that duplicate issues are created --- diff --git a/README.rst b/README.rst index 74ae335..1b81aa4 100644 --- a/README.rst +++ b/README.rst @@ -46,7 +46,8 @@ Note: Overwrite set to True will ensure that upstream issue fields will clear do issue fields, overwrite set to False will never delete downstream issue fields only append. The optional owner field can be used to specify a username that should be used if -the program cannot find a matching downstream user to assigne an issue too. +the program cannot find a matching downstream user to assigne an issue too. The owner +field will also be used to alert users if duplicate downstream issues exist. Development ----------- diff --git a/requirements.txt b/requirements.txt index aade641..b8059ef 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,4 +2,5 @@ jira requests fedmsg PyGithub -urllib3 \ No newline at end of file +urllib3 +jinja2 \ No newline at end of file diff --git a/sync2jira/downstream.py b/sync2jira/downstream.py index 0e7bf1d..5a049cc 100644 --- a/sync2jira/downstream.py +++ b/sync2jira/downstream.py @@ -26,8 +26,10 @@ import arrow import jira.client from jira import JIRAError from datetime import datetime +import jinja2 from sync2jira.intermediary import Issue +from sync2jira.mailer import Mailer # The date the service was upgraded # This is used to ensure legacy comments are not touched @@ -36,6 +38,7 @@ UPDATE_DATE = datetime(2019, 7, 9, 18, 18, 36, 480291) log = logging.getLogger(__name__) remote_link_title = "Upstream issue" +duplicate_issues_subject = 'FYI: Duplicate Sync2jira Issues' jira_cache = {} @@ -161,11 +164,87 @@ def _matching_jira_issue_query(client, issue, config, free=False): # Return the final_results log.debug("Found %i results for query %r", len(final_results), query) + + # Alert the owner + if issue.downstream.get('owner'): + alert_user_of_duplicate_issues(issue, final_results, + results_of_query, + config, client) return final_results else: return results_of_query +def alert_user_of_duplicate_issues(issue, final_result, results_of_query, + config, client): + """ + Alerts owner of duplicate downstream issues + Args: + issue (sync2jira.intermediate.Issue): Upstream Issue object + final_result (list): Issue selected by matching algorithm + results_of_query (list): Result of JQL query + config (dict): Config dict + client (jira.client.JIRA): JIRA client + Returns: + Nothing + """ + # First remove final_result from results_of_query + results_of_query.remove(final_result[0]) + + # Check that all duplicate issues are closed + updated_results = [] + for result in results_of_query: + if result.fields.status.name != 'Closed': + updated_results.append(result) + if not updated_results: + # Nothing to alert the owner of + return + + # Get base URL + jira_instance = issue.downstream.get('jira_instance', False) + if not jira_instance: + jira_instance = config['sync2jira'].get('default_jira_instance', False) + if not jira_instance: + log.error(" No jira_instance for issue and there is no default in the config") + raise Exception + base_url = config['sync2jira']['jira'][jira_instance]['options']['server'] + '/browse/' + + # Format the updated results + template_ready = [] + for update in updated_results: + url = base_url + update.key + new_entry = {'url': url, 'title': update.key} + template_ready.append(new_entry) + + # Get owner name and email from Jira + ret = client.search_users(issue.downstream.get('owner')) + if len(ret) > 1: + log.warning(' Found multiple users for username %s') % issue.downstream.get('owner') + return + + user = {'name': ret[0].displayName, 'email': ret[0].emailAddress} + + # Format selected issue + selected_issue = {'url': base_url + final_result[0].key, + 'title': final_result[0].key} + + # Create and send email + templateLoader = jinja2.FileSystemLoader(searchpath='sync2jira/') + templateEnv = jinja2.Environment(loader=templateLoader) + template = templateEnv.get_template('email_template.jinja') + html_text = template.render(user=user, + issue=issue, + selected_issue=selected_issue, + duplicate_issues=template_ready) + + # Send mail + Mailer().send(recipients=[user['email']], + subject=duplicate_issues_subject, + text=html_text) + log.info(' Alerted %s about %s duplicate issue(s)' % + (user['email'], len(template_ready))) + + def find_username(issue, config): """ Finds JIRA username for an issue object diff --git a/sync2jira/email_template.jinja b/sync2jira/email_template.jinja new file mode 100644 index 0000000..3591a88 --- /dev/null +++ b/sync2jira/email_template.jinja @@ -0,0 +1,15 @@ + + +

Hello {{ user['name'] }},
It looks like you have some duplicate issues for + upstream issue {{ issue._title }}

+

But these issues were also found:

+ +

Make sure to mark these duplicate issues as 'Closed' to avoid these emails!

+ + \ No newline at end of file diff --git a/sync2jira/mailer.py b/sync2jira/mailer.py new file mode 100644 index 0000000..dcc2629 --- /dev/null +++ b/sync2jira/mailer.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python3 +""" +This script is used to send emails +""" + +import smtplib +from email.mime.text import MIMEText +from email.mime.multipart import MIMEMultipart + + +class Mailer: + """ A mailer component used to send emails + """ + + DEFAULT_FROM = "" + DEFAULT_SERVER = "" + + def __init__(self): + """Returns mailer object + """ + self._cfg = {} + self._cfg.setdefault("server", self.DEFAULT_SERVER) + self._cfg.setdefault("from", self.DEFAULT_FROM) + + def send(self, recipients, subject, text): + """ Sends email to recipients + :param list recipiencts : recipients of email + :param string subject : subject of the email + :param: bool attachments : bool to attach images + :pram string text: text of the email + """ + sender = self._cfg["from"] + msg = MIMEMultipart('related') + msg["Subject"] = subject + msg["From"] = sender + msg["To"] = ", ".join(recipients) + server = smtplib.SMTP(self._cfg["server"]) + part = MIMEText(text, 'html', 'utf-8') + msg.attach(part) + + server.sendmail(sender, recipients, msg.as_string()) + server.quit() diff --git a/tests/test_downstream.py b/tests/test_downstream.py index 0d50f51..b6bc66f 100644 --- a/tests/test_downstream.py +++ b/tests/test_downstream.py @@ -31,7 +31,8 @@ class TestDownstream(unittest.TestCase): 'default_jira_instance': 'another_jira_instance', 'jira': { 'mock_jira_instance': {'mock_jira': 'mock_jira'}, - 'another_jira_instance': {'basic_auth': ['mock_user']} + 'another_jira_instance': {'basic_auth': ['mock_user'], + 'options': {'server': 'mock_server'}} }, 'testing': {}, 'legacy_matching': False @@ -1051,13 +1052,15 @@ class TestDownstream(unittest.TestCase): resolution={'name': 'Duplicate'} ) + @mock.patch(PATH + 'alert_user_of_duplicate_issues') @mock.patch(PATH + 'find_username') @mock.patch(PATH + 'check_comments_for_duplicate') @mock.patch('jira.client.JIRA') def test_matching_jira_issue_query(self, mock_client, mock_check_comments_for_duplicates, - mock_find_username): + mock_find_username, + mock_alert_user_of_duplicate_issues): """ This tests '_matching_jira_query' function """ @@ -1071,6 +1074,7 @@ class TestDownstream(unittest.TestCase): mock_client.search_issues.return_value = [mock_downstream_issue, bad_downstream_issue] mock_check_comments_for_duplicates.return_value = True mock_find_username.return_value = 'mock_username' + mock_alert_user_of_duplicate_issues.return_value = True # Call the function response = d._matching_jira_issue_query( @@ -1081,6 +1085,13 @@ class TestDownstream(unittest.TestCase): # Assert everything was called correctly self.assertEqual(response, [mock_downstream_issue]) + mock_alert_user_of_duplicate_issues.assert_called_with( + self.mock_issue, + [mock_downstream_issue], + mock_client.search_issues.return_value, + self.mock_config, + mock_client + ) mock_client.search_issues.assert_called_with( 'issueFunction in linkedIssuesOfRemote("Upstream issue")' ' and issueFunction in linkedIssuesOfRemote("mock_url")') @@ -1094,6 +1105,51 @@ class TestDownstream(unittest.TestCase): self.mock_config ) + @mock.patch(PATH + 'jinja2') + @mock.patch(PATH + 'Mailer') + @mock.patch('jira.client.JIRA') + def test_alert_user(self, + mock_client, + mock_mailer, + mock_jinja,): + """ + This tests 'alert_user_of_duplicate_issues' function + """ + # Set up return values + mock_downstream_issue = MagicMock() + mock_downstream_issue.key = 'mock_key' + bad_downstream_issue = MagicMock() + bad_downstream_issue.key = 'mock_key' + bad_downstream_issue.fields.status.name = 'To Do' + mock_results_of_query = [mock_downstream_issue, bad_downstream_issue] + mock_search_user_result = MagicMock() + mock_search_user_result.displayName = 'mock_name' + mock_search_user_result.emailAddress = 'mock_email' + mock_client.search_users.return_value = [mock_search_user_result] + mock_template = MagicMock(name='template') + mock_template.render.return_value = 'mock_html_text' + mock_template_env = MagicMock(name='templateEnv') + mock_template_env.get_template.return_value = mock_template + mock_jinja.Environment.return_value = mock_template_env + + # Call the function + d.alert_user_of_duplicate_issues( + issue=self.mock_issue, + final_result=[mock_downstream_issue], + results_of_query=mock_results_of_query, + config=self.mock_config, + client=mock_client + ) + + # Assert everything was called correctly + mock_client.search_users.assert_called_with('mock_owner') + mock_template.render.assert_called_with( + duplicate_issues=[{'url': 'mock_server/browse/mock_key', 'title': 'mock_key'}], + issue=self.mock_issue, + selected_issue={'url': 'mock_server/browse/mock_key', 'title': 'mock_key'}, + user={'name': 'mock_name', 'email': 'mock_email'}) + mock_mailer().send.asset_called_with('test') + def test_find_username(self): """ Tests 'find_username' function From c620b81babce1fab19218cd730b31ad8e7d37a92 Mon Sep 17 00:00:00 2001 From: sidpremkumar Date: Jul 12 2019 22:39:24 +0000 Subject: [PATCH 2/4] Mailer gets config values from enviormental variables --- diff --git a/README.rst b/README.rst index 1b81aa4..14eb2dc 100644 --- a/README.rst +++ b/README.rst @@ -46,9 +46,15 @@ Note: Overwrite set to True will ensure that upstream issue fields will clear do issue fields, overwrite set to False will never delete downstream issue fields only append. The optional owner field can be used to specify a username that should be used if -the program cannot find a matching downstream user to assigne an issue too. The owner +the program cannot find a matching downstream user to assignee an issue too. The owner field will also be used to alert users if duplicate downstream issues exist. +To set up the mailer you need to set the following environmental variables: + +1. DEFAULT_FROM - Email address used to send emails + +2. DEFAULT_SERVER - Mail server to be used + Development ----------- diff --git a/sync2jira/mailer.py b/sync2jira/mailer.py index dcc2629..75784b9 100644 --- a/sync2jira/mailer.py +++ b/sync2jira/mailer.py @@ -4,6 +4,7 @@ This script is used to send emails """ import smtplib +import os from email.mime.text import MIMEText from email.mime.multipart import MIMEMultipart @@ -12,8 +13,8 @@ class Mailer: """ A mailer component used to send emails """ - DEFAULT_FROM = "" - DEFAULT_SERVER = "" + DEFAULT_FROM = os.environ['DEFAULT_FROM'] + DEFAULT_SERVER = os.environ['DEFAULT_SERVER'] def __init__(self): """Returns mailer object From 54042f8a4fb3866a3d82906d47f3c32f10328de2 Mon Sep 17 00:00:00 2001 From: sidpremkumar Date: Jul 15 2019 15:06:31 +0000 Subject: [PATCH 3/4] adding admin support for emails --- diff --git a/fedmsg.d/sync2jira.py b/fedmsg.d/sync2jira.py index 3c81ee7..e1e78e5 100644 --- a/fedmsg.d/sync2jira.py +++ b/fedmsg.d/sync2jira.py @@ -19,6 +19,9 @@ config = { 'sync2jira': { + # Admins to be cc'd in duplicate emails + 'admins': ['demo_email@demo.com'], + # Scrape sources at startup 'initialize': True, @@ -62,4 +65,4 @@ config = { }, } }, -} +} \ No newline at end of file diff --git a/sync2jira/downstream.py b/sync2jira/downstream.py index 5a049cc..374f659 100644 --- a/sync2jira/downstream.py +++ b/sync2jira/downstream.py @@ -219,8 +219,15 @@ def alert_user_of_duplicate_issues(issue, final_result, results_of_query, # Get owner name and email from Jira ret = client.search_users(issue.downstream.get('owner')) if len(ret) > 1: - log.warning(' Found multiple users for username %s') % issue.downstream.get('owner') - return + log.warning(' Found multiple users for username %s' % issue.downstream.get('owner')) + found = False + for person in ret: + if person.key == issue.downstream.get('owner'): + ret = [person] + found = True + break + if not found: + log.warning(' Could not find JIRA user for username %s' % issue.downstream.get('owner')) user = {'name': ret[0].displayName, 'email': ret[0].emailAddress} @@ -228,17 +235,37 @@ def alert_user_of_duplicate_issues(issue, final_result, results_of_query, selected_issue = {'url': base_url + final_result[0].key, 'title': final_result[0].key} + # Get admin information + admins = [] + admin_template = [] + for admin in config['sync2jira']['admins']: + ret = client.search_users(admin) + if len(ret) > 1: + log.warning(' Found multiple users for admin %s' % issue.downstream.get('owner')) + found = False + for person in ret: + if person.key == issue.downstream.get('owner'): + ret = [person] + found = True + break + if not found: + log.warning(' Could not find JIRA user for admin %s' % issue.downstream.get('owner')) + admins.append(ret[0].emailAddress) + admin_template.append({'name': ret[0].displayName, 'email': ret[0].emailAddress}) + # Create and send email templateLoader = jinja2.FileSystemLoader(searchpath='sync2jira/') templateEnv = jinja2.Environment(loader=templateLoader) template = templateEnv.get_template('email_template.jinja') html_text = template.render(user=user, + admins=admin_template, issue=issue, selected_issue=selected_issue, duplicate_issues=template_ready) # Send mail Mailer().send(recipients=[user['email']], + cc=admins, subject=duplicate_issues_subject, text=html_text) log.info(' Alerted %s about %s duplicate issue(s)' % diff --git a/sync2jira/email_template.jinja b/sync2jira/email_template.jinja index 3591a88..0a8dc2f 100644 --- a/sync2jira/email_template.jinja +++ b/sync2jira/email_template.jinja @@ -11,5 +11,13 @@ {% endfor %}

Make sure to mark these duplicate issues as 'Closed' to avoid these emails!

+ {% if admins|length > 0 %} +

Questions? Get in contact with one of the admins: + {% for admin in admins %} + {{ admin.name }} + {{ "," if not loop.last }} + {% endfor %} +

+ {% endif %} \ No newline at end of file diff --git a/sync2jira/mailer.py b/sync2jira/mailer.py index 75784b9..f245ada 100644 --- a/sync2jira/mailer.py +++ b/sync2jira/mailer.py @@ -23,11 +23,11 @@ class Mailer: self._cfg.setdefault("server", self.DEFAULT_SERVER) self._cfg.setdefault("from", self.DEFAULT_FROM) - def send(self, recipients, subject, text): + def send(self, recipients, subject, text, cc): """ Sends email to recipients :param list recipiencts : recipients of email :param string subject : subject of the email - :param: bool attachments : bool to attach images + :param: list cc : cc of the email :pram string text: text of the email """ sender = self._cfg["from"] @@ -35,6 +35,7 @@ class Mailer: msg["Subject"] = subject msg["From"] = sender msg["To"] = ", ".join(recipients) + msg['Cc'] = ", ".join(cc) server = smtplib.SMTP(self._cfg["server"]) part = MIMEText(text, 'html', 'utf-8') msg.attach(part) diff --git a/tests/test_downstream.py b/tests/test_downstream.py index b6bc66f..938ccd6 100644 --- a/tests/test_downstream.py +++ b/tests/test_downstream.py @@ -5,7 +5,7 @@ try: from unittest.mock import MagicMock # noqa: F401 except ImportError: from mock import MagicMock # noqa: F401 - +import os import sync2jira.downstream as d from sync2jira.intermediary import Issue @@ -35,7 +35,8 @@ class TestDownstream(unittest.TestCase): 'options': {'server': 'mock_server'}} }, 'testing': {}, - 'legacy_matching': False + 'legacy_matching': False, + 'admins': ['mock_admin'] }, } @@ -1142,8 +1143,10 @@ class TestDownstream(unittest.TestCase): ) # Assert everything was called correctly - mock_client.search_users.assert_called_with('mock_owner') + mock_client.search_users.assert_any_call('mock_owner') + mock_client.search_users.assert_any_call('mock_admin') mock_template.render.assert_called_with( + admins=[{'name': 'mock_name', 'email': 'mock_email'}], duplicate_issues=[{'url': 'mock_server/browse/mock_key', 'title': 'mock_key'}], issue=self.mock_issue, selected_issue={'url': 'mock_server/browse/mock_key', 'title': 'mock_key'}, diff --git a/tox.ini b/tox.ini index 443c0d1..7dbb338 100644 --- a/tox.ini +++ b/tox.ini @@ -2,6 +2,9 @@ envlist = py37,flake8 [testenv] +setenv = + DEFAULT_FROM = mock_email@mock.com + DEFAULT_SERVER = mock_server basepython = py37: python3.7 deps = From c22b2120951f097905cb4828c3da2b1d7cba6ae0 Mon Sep 17 00:00:00 2001 From: sidpremkumar Date: Jul 15 2019 18:08:49 +0000 Subject: [PATCH 4/4] Removing mailer class and replacing with static function --- diff --git a/fedmsg.d/sync2jira.py b/fedmsg.d/sync2jira.py index e1e78e5..2ccc1ef 100644 --- a/fedmsg.d/sync2jira.py +++ b/fedmsg.d/sync2jira.py @@ -20,8 +20,8 @@ config = { 'sync2jira': { # Admins to be cc'd in duplicate emails - 'admins': ['demo_email@demo.com'], - + 'admins': ['demo_jira_username'], + # Scrape sources at startup 'initialize': True, diff --git a/sync2jira/downstream.py b/sync2jira/downstream.py index 374f659..f76f3c2 100644 --- a/sync2jira/downstream.py +++ b/sync2jira/downstream.py @@ -29,7 +29,7 @@ from datetime import datetime import jinja2 from sync2jira.intermediary import Issue -from sync2jira.mailer import Mailer +from sync2jira.mailer import send_mail # The date the service was upgraded # This is used to ensure legacy comments are not touched @@ -130,6 +130,7 @@ def _matching_jira_issue_query(client, issue, config, free=False): # We need to ensure that we are not catching a dropped issue # Loop through the results of the query and make sure the ids match final_results = [] + for result in results_of_query: # If the queried JIRA issue has the id of the upstream issue or the same title if issue.id in result.fields.description or issue.title == result.fields.summary: @@ -264,10 +265,10 @@ def alert_user_of_duplicate_issues(issue, final_result, results_of_query, duplicate_issues=template_ready) # Send mail - Mailer().send(recipients=[user['email']], - cc=admins, - subject=duplicate_issues_subject, - text=html_text) + send_mail(recipients=[user['email']], + cc=admins, + subject=duplicate_issues_subject, + text=html_text) log.info(' Alerted %s about %s duplicate issue(s)' % (user['email'], len(template_ready))) diff --git a/sync2jira/mailer.py b/sync2jira/mailer.py index f245ada..edbfed2 100644 --- a/sync2jira/mailer.py +++ b/sync2jira/mailer.py @@ -8,37 +8,30 @@ import os from email.mime.text import MIMEText from email.mime.multipart import MIMEMultipart +DEFAULT_FROM = os.environ['DEFAULT_FROM'] +DEFAULT_SERVER = os.environ['DEFAULT_SERVER'] -class Mailer: - """ A mailer component used to send emails - """ - - DEFAULT_FROM = os.environ['DEFAULT_FROM'] - DEFAULT_SERVER = os.environ['DEFAULT_SERVER'] - def __init__(self): - """Returns mailer object - """ - self._cfg = {} - self._cfg.setdefault("server", self.DEFAULT_SERVER) - self._cfg.setdefault("from", self.DEFAULT_FROM) - - def send(self, recipients, subject, text, cc): - """ Sends email to recipients - :param list recipiencts : recipients of email - :param string subject : subject of the email - :param: list cc : cc of the email - :pram string text: text of the email - """ - sender = self._cfg["from"] - msg = MIMEMultipart('related') - msg["Subject"] = subject - msg["From"] = sender - msg["To"] = ", ".join(recipients) - msg['Cc'] = ", ".join(cc) - server = smtplib.SMTP(self._cfg["server"]) - part = MIMEText(text, 'html', 'utf-8') - msg.attach(part) +def send_mail(recipients, subject, text, cc): + """ Sends email to recipients + :param list recipients : recipients of email + :param string subject : subject of the email + :param string text: HTML text + :param: list cc : cc of the email + :pram string text: text of the email + """ + _cfg = {} + _cfg.setdefault("server", DEFAULT_SERVER) + _cfg.setdefault("from", DEFAULT_FROM) + sender = _cfg["from"] + msg = MIMEMultipart('related') + msg["Subject"] = subject + msg["From"] = sender + msg["To"] = ", ".join(recipients) + msg['Cc'] = ", ".join(cc) + server = smtplib.SMTP(_cfg["server"]) + part = MIMEText(text, 'html', 'utf-8') + msg.attach(part) - server.sendmail(sender, recipients, msg.as_string()) - server.quit() + server.sendmail(sender, recipients, msg.as_string()) + server.quit() diff --git a/tests/test_downstream.py b/tests/test_downstream.py index 938ccd6..ca729f3 100644 --- a/tests/test_downstream.py +++ b/tests/test_downstream.py @@ -1107,7 +1107,7 @@ class TestDownstream(unittest.TestCase): ) @mock.patch(PATH + 'jinja2') - @mock.patch(PATH + 'Mailer') + @mock.patch(PATH + 'send_mail') @mock.patch('jira.client.JIRA') def test_alert_user(self, mock_client,