Separate the labeling and commenting logic

* Trying to integrate the latest changes for labels from here
  https://github.com/ianpittwood/Gerrit-to-Github-Issues/pull/11/commits

Change-Id: I7759c01ba1447ba33708ae510e7af010606d6ced

gerrit_to_github_issues/engine.py

@@ -31,72 +31,108 @@ def update(gerrit_url: str, gerrit_repo_name: str, github_project_id: int,
     repo = gh.get_repo(github_repo_name)
     project_board = gh.get_project(github_project_id)
     change_list = gerrit.get_changes(gerrit_url, gerrit_repo_name, change_age=change_age)
-    for change in change_list['data']:
+
-        if 'commitMessage' in change:
+    issue_map = {}
-            process_change(gh, change, repo, project_board, skip_approvals)
+    for change in change_list:
+        issue_numbers_dict = github_issues.parse_issue_number(change['commitMessage'])
+        issue_numbers_dict = github_issues.remove_duplicated_issue_numbers(issue_numbers_dict)
+
+        add_comments(gh, change, issue_numbers_dict, repo, skip_approvals)
+
+        # accumulate the affected issues for later when adding labels
+        for _, issue_list in issue_numbers_dict.items():
+            for issue_number in issue_list:
+                if issue_number in issue_map:
+                    issue_map[issue_number] += [change]
+                else:
+                    issue_map[issue_number] = [change]
+
+    for issue in issue_map:
+        add_labels(gh, issue, issue_map[issue], repo, project_board)
 
     # Handle the incoming issue assignment requests
     github_issues.assign_issues(repo)
 
 
-def process_change(gh: github.Github, change: dict, repo: Repository,
+# add_labels iterates over all of the changes that affect this issue and verifies that
-                   project_board: Project, skip_approvals: bool = False):
+# it has the correct label
-    issue_numbers_dict = github_issues.parse_issue_number(change['commitMessage'])
+def add_labels(gh: github.Github, issue_number: int, affecting_changes: list,
-    issue_numbers_dict = github_issues.remove_duplicated_issue_numbers(issue_numbers_dict)
+           repo: Repository, project_board: Project):
-    if not issue_numbers_dict:
+    try:
-        LOG.warning(f'No issue tag found for change #{change["number"]}')
+        issue = repo.get_issue(issue_number)
+    except github.GithubException:
+        LOG.warning(f'Issue #{issue_number} not found for project')
         return
-    for key, issues_list in issue_numbers_dict.items():
+
+    # Assume these conditions and prove otherwise by iterating over affecting changes
+    is_wip = False
+    is_closed = True
+
+    for change in affecting_changes:
+        if 'WIP' in change['commitMessage'] or 'DNM' in change['commitMessage']:
+            is_wip = True
+        if change['status'] == 'NEW':
+            is_closed = False
+
+    if is_closed:
+        LOG.debug(f'Issue #{issue_number} is closed, removing labels.')
+        remove_label(issue, 'wip')
+        remove_label(issue, 'ready for review')
+    elif is_wip:
+        Log.debug(f'Issue #{issue_number} is WIP, adding the "wip" label and removing ' \
+                  f'the "ready for review" label.')
+        remove_label(issue, 'ready for review')
+        add_label(issue, 'wip')
+        move_issue(project_board, issue, 'In Progress')
+    else:
+        Log.debug(f'Issue #{issue_number} is ready to be reviewed, adding the "ready ' \
+                  f'for review" label and removing the "wip" label.')
+        remove_label(issue, 'wip')
+        add_label(issue, 'ready for review')
+        move_issue(project_board, issue, 'Submitted on Gerrit')
+
+
+# remove_label removes the label from issue if it exists
+def remove_label(issue: github.Issue, label: str):
+    try:
+        LOG.debug(f'Removing `{label}` label from issue #{issue_number}')
+        issue.remove_from_labels(label)
+    except github.GithubException:
+        LOG.debug(f'`{label}` tag does not exist on issue #{issue_number}')
+
+
+# add_comments iterates over all of the issues affected by this change and verifies they
+# have the appropriate comments. If the bot hasn't created a comment related to this
+# change on an issue, it will create a new comment, otherwise it will edit its prior
+# comment.
+def add_comments(gh: github.Github, change: dict, affected_issues: dict,
+           repo: Repository, skip_approvals: bool = False):
+    for key, issues_list in affected_issues.items():
         for issue_number in issues_list:
             try:
                 issue = repo.get_issue(issue_number)
             except github.GithubException:
                 LOG.warning(f'Issue #{issue_number} not found for project')
                 return
-            bot_comment = github_issues.get_bot_comment(issue, gh.get_user().login, change['number'])
+
-            if issue.state == 'closed' and not bot_comment:
+            comment_msg = get_issue_comment(change, key, skip_approvals)
+            if issue.state == 'closed':
                 LOG.debug(f'Issue #{issue_number} was closed, reopening...')
 
                 # NOTE(howell): Reopening a closed issue will move it from the
                 # "Done" column to the "In Progress" column on the project
                 # board via Github automation.
                 issue.edit(state='open')
-                issue.create_comment('Issue reopened due to new activity on Gerrit.\n\n')
+                comment_msg += '\n\nIssue reopened due to new activity on Gerrit.'
 
-            labels = [str(l.name) for l in list(issue.get_labels())]
+            bot_comment = github_issues.get_bot_comment(issue, gh.get_user().login, change['number'])
-            if 'WIP' in change['commitMessage'] or 'DNM' in change['commitMessage']:
-                if 'wip' not in labels:
-                    LOG.debug(f'add `wip` to #{issue_number}')
-                    issue.add_to_labels('wip')
-                if 'ready for review' in labels:
-                    try:
-                        LOG.debug(f'rm `ready for review` to #{issue_number}')
-                        issue.remove_from_labels('ready for review')
-                    except github.GithubException:
-                        LOG.debug(f'`ready for review` tag does not exist on issue #{issue_number}')
-                move_issue(project_board, issue, 'In Progress')
-            else:
-                if 'ready for review' not in labels:
-                    LOG.debug(f'add `ready for review` to #{issue_number}')
-                    issue.add_to_labels('ready for review')
-                if 'wip' in labels:
-                    try:
-                        LOG.debug(f'rm `wip` to #{issue_number}')
-                        issue.remove_from_labels('wip')
-                    except github.GithubException:
-                        LOG.debug(f'`wip` tag does not exist on issue #{issue_number}')
-                move_issue(project_board, issue, 'Submitted on Gerrit')
-            comment_msg = get_issue_comment(change, key, skip_approvals)
             if not bot_comment:
-                if key == 'closes':
-                    comment_msg += '\n\nThis change will close this issue when merged.'
                 LOG.debug(f'Comment to post on #{issue_number}: {comment_msg}')
                 issue.create_comment(comment_msg)
                 LOG.info(f'Comment posted to issue #{issue_number}')
             else:
                 LOG.debug(f'Comment to edit on #{issue_number}: {comment_msg}')
-                comment = github_issues.get_bot_comment(issue, gh.get_user().login, change['number'])
+                bot_comment.edit(comment_msg)
-                comment.edit(comment_msg)
                 LOG.info(f'Comment edited to issue #{issue_number}')
 
 
@@ -161,3 +197,4 @@ def move_issue(project_board: Project, issue: Issue, to_col_name: str):
         LOG.info(f'Moved issue "{issue.title}" to column "{to_col_name}"')
     else:
         LOG.warning(f'Failed to move issue "{issue.title}" to column "{to_col_name}"')
+

gerrit_to_github_issues/gerrit.py

@@ -14,14 +14,15 @@ import json
 from fabric import Connection
 
 
-def get_changes(gerrit_url: str, project_name: str, port: int = 29418, change_age: str = None) -> dict:
+def get_changes(gerrit_url: str, project_name: str, port: int = 29418, change_age: str = None) -> list:
     cmd = f'gerrit query --format=JSON --current-patch-set project:{project_name}'
     if change_age:
         cmd += f' -- -age:{change_age}'
     result = Connection(gerrit_url, port=port).run(cmd)
     processed_stdout = '{"data":[%s]}' % ','.join(list(filter(None, result.stdout.split('\n'))))
     data = json.loads(processed_stdout)
-    return data
+    changes = [c for c in data['data'] if 'commitMessage' in c]
+    return changes
 
 
 def make_gerrit_url(gerrit_url: str, change_number: str, protocol: str = 'https'):