#76 Adding support to email owners in the situation that duplicate issues are found
Merged 4 years ago by sidpremkumar. Opened 4 years ago by sidpremkumar.
sidpremkumar/sync-to-jira alert-owner-email  into  develop

file modified
+8 -1
@@ -46,7 +46,14 @@ 

  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 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

  -----------

file modified
+4 -1
@@ -19,6 +19,9 @@ 

  

  config = {

      'sync2jira': {

+         # Admins to be cc'd in duplicate emails

+         'admins': ['demo_jira_username'],

+ 

          # Scrape sources at startup

          'initialize': True,

  
@@ -62,4 +65,4 @@ 

              },

          }

      },

- }

+ } 

\ No newline at end of file

file modified
+2 -1
@@ -2,4 +2,5 @@ 

  requests

  fedmsg

  PyGithub

- urllib3 

\ No newline at end of file

+ urllib3

+ jinja2 

\ No newline at end of file

file modified
+107
@@ -26,8 +26,10 @@ 

  import jira.client

  from jira import JIRAError

  from datetime import datetime

+ import jinja2

  

  from sync2jira.intermediary import Issue

+ from sync2jira.mailer import send_mail

  

  # The date the service was upgraded

  # This is used to ensure legacy comments are not touched
@@ -36,6 +38,7 @@ 

  log = logging.getLogger(__name__)

  

  remote_link_title = "Upstream issue"

+ duplicate_issues_subject = 'FYI: Duplicate Sync2jira Issues'

  

  jira_cache = {}

  
@@ -127,6 +130,7 @@ 

          # 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:
@@ -161,11 +165,114 @@ 

  

          # 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'))

+         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}

+ 

+     # Format selected issue

+     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

+     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)))

+ 

+ 

  def find_username(issue, config):

      """

      Finds JIRA username for an issue object

@@ -0,0 +1,23 @@ 

+ <html>

+   <body style="font-family: Arial; font-size: 14px;">

+     <p> Hello {{ user['name'] }}, <br>It looks like you have some duplicate issues for

+         upstream issue <a href="{{ issue.url }}">{{ issue._title }}</a></p>

+   <p>But these issues were also found:</p>

+     <ul>

+       {% for duplicate in duplicate_issues %}

+           <li>

+             <a  style="" href="{{ duplicate['url'] }}">{{ duplicate['title'] }}</a>

+           </li>

+       {% endfor %}

+     </ul>

+   <p>Make sure to mark these duplicate issues as 'Closed' to avoid these emails!</p>

+     {% if admins|length > 0 %}

+             <p>Questions? Get in contact with one of the admins:

+                 {% for admin in admins %}

+                     <a href="mailto:{{ admin.email }}">{{ admin.name }}</a>

+                     {{ "," if not loop.last }}

+                 {% endfor %}

+             </p>

+     {% endif %}

+   </body>

+ </html> 

\ No newline at end of file

file added
+37
@@ -0,0 +1,37 @@ 

+ #!/usr/bin/env python3

+ """

+ This script is used to send emails

+ """

+ 

+ import smtplib

+ 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']

+ 

+ 

+ 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()

file modified
+63 -4
@@ -5,7 +5,7 @@ 

      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
@@ -31,10 +31,12 @@ 

                  '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

+                 'legacy_matching': False,

+                 'admins': ['mock_admin']

              },

          }

  
@@ -1051,13 +1053,15 @@ 

              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 +1075,7 @@ 

          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 +1086,13 @@ 

  

          # 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 +1106,53 @@ 

              self.mock_config

          )

  

+     @mock.patch(PATH + 'jinja2')

+     @mock.patch(PATH + 'send_mail')

+     @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_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'},

+             user={'name': 'mock_name', 'email': 'mock_email'})

+         mock_mailer().send.asset_called_with('test')

+ 

      def test_find_username(self):

          """

          Tests 'find_username' function

file modified
+3
@@ -2,6 +2,9 @@ 

  envlist = py37,flake8

  

  [testenv]

+ setenv =

+     DEFAULT_FROM = mock_email@mock.com

+     DEFAULT_SERVER = mock_server

  basepython =

      py37: python3.7

  deps =

When the optional owner field is set sync2jira will alert the owner (via email) if any duplicate issues are not closed.

1 new commit added

  • Mailer gets config values from enviormental variables
4 years ago

Nice!

Can you add an additional config option for a list of global admins (i.e., rbean@email.com and spremkum@email.com) such that we get cc'd on all these notifications.

I can imagine a situation where an owner of a downstream project will get one of these emails, and they would want to reply-all to ask "what am I supposed to do about this?"

1 new commit added

  • adding admin support for emails
4 years ago

This Mailer().method(...)is an anti-pattern in my opinion.

https://www.youtube.com/watch?v=o9pEzgHorH0

Consider removing the class, and keeping only the callable such that in this module you would write from sync2jira.mailer import send_mail and then send_mail(recipients=....).

Consider the comment above. It's not a blocker. You have my :+1: to merge without changing anything.

1 new commit added

  • Removing mailer class and replacing with static class
4 years ago

+1 to that idea. Changed it in the last commit

4 new commits added

  • Removing mailer class and replacing with static functions
  • adding admin support for emails
  • Mailer gets config values from enviormental variables
  • Adding support to email owners in the situation that duplicate issues are created
4 years ago

4 new commits added

  • Removing mailer class and replacing with static function
  • adding admin support for emails
  • Mailer gets config values from enviormental variables
  • Adding support to email owners in the situation that duplicate issues are created
4 years ago

Pull-Request has been merged by sidpremkumar

4 years ago