From dd981112d044a7e5eb1d898c4b83ae89f27cc203 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Mon, 3 Jun 2024 15:42:41 -0400 Subject: [PATCH 01/19] Initial (incomplete) `inactive_revision` rule commit --- bugbot/rules/inactive_revision.py | 104 ++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 bugbot/rules/inactive_revision.py diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py new file mode 100644 index 000000000..dbd13ca18 --- /dev/null +++ b/bugbot/rules/inactive_revision.py @@ -0,0 +1,104 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +import re + +from dateutil.relativedelta import relativedelta +from libmozdata import utils as lmdutils +from libmozdata.phabricator import PhabricatorAPI + +from bugbot import utils +from bugbot.bzcleaner import BzCleaner +from bugbot.user_activity import UserActivity + +PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt") +PHAB_TABLE_PAT = re.compile(r"^\|\ \[D([0-9]+)\]\(h", flags=re.M) + + +class InactiveRevision(BzCleaner): + """Bugs with patches that are waiting for review from inactive reviewers""" + + def __init__(self, old_patch_months: int = 6): + """Constructor + + Args: + old_patch_months: number of months since creation of the patch to be + considered old. If the bug has an old patch, we will mention + abandon the patch as an option. + """ + super(InactiveRevision, self).__init__() + self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"]) + self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab) + self.ni_template = self.get_needinfo_template() + self.old_patch_limit = ( + lmdutils.get_date_ymd("today") - relativedelta(months=old_patch_months) + ).timestamp() + + def description(self): + return "Bugs with inactive patch reviewers" + + def columns(self): + return ["id", "summary", "revisions"] + + def get_bugs(self, date="today", bug_ids=[], chunk_size=None): + bugs = super().get_bugs(date, bug_ids, chunk_size) + + rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]} + revisions = self._get_revisions_with_inactive_action(list(rev_ids)) + + for bugid, bug in list(bugs.items()): + inactive_revs = [ + revisions[rev_id] for rev_id in bug["rev_ids"] if rev_id in revisions + ] + if inactive_revs: + bug["revisions"] = inactive_revs + self._add_needinfo(bugid, inactive_revs) + else: + del bugs[bugid] + + self.query_url = utils.get_bz_search_url({"bug_id": ",".join(bugs.keys())}) + + return bugs + + def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: + for revision in inactive_revs: + last_action_by, _ = self._find_last_action(revision["rev_id"]) + + if last_action_by == "author": + ni_mail = revision["reviewers"][0]["phab_username"] + summary = ( + "The last action was by the author, so needinfoing the reviewer." + ) + elif last_action_by == "reviewer": + ni_mail = revision["author"]["phab_username"] + summary = ( + "The last action was by the reviewer, so needinfoing the author." + ) + else: + continue + + comment = self.ni_template.render( + revisions=[revision], + nicknames=revision["author"]["nick"], + reviewers_status_summary=summary, + has_old_patch=revision["created_at"] < self.old_patch_limit, + plural=utils.plural, + documentation=self.get_documentation(), + ) + + self.autofix_changes[bugid] = { + "comment": {"body": comment}, + "flags": [ + { + "name": "needinfo", + "requestee": ni_mail, + "status": "?", + "new": "true", + } + ], + } + + def _find_last_action(self, revision_id): + # TODO: implement actual logic finding last action taker + return "author" From b2fc12fcf25d00e0477e173c4b083d299eb97f97 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Mon, 3 Jun 2024 16:07:29 -0400 Subject: [PATCH 02/19] Updated function to find the author of the last action --- bugbot/rules/inactive_revision.py | 36 +++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index dbd13ca18..ba53eb618 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -100,5 +100,37 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: } def _find_last_action(self, revision_id): - # TODO: implement actual logic finding last action taker - return "author" + details = self._fetch_revision_details(revision_id) + + if not details: + return None, None + + revision = details[0] + author_phid = revision["fields"]["authorPHID"] + reviewers = [ + reviewer["reviewerPHID"] + for reviewer in revision["attachments"]["reviewers"]["reviewers"] + ] + + transactions = self._fetch_revision_transactions(revision["phid"]) + + last_transaction = None + for transaction in transactions: + if ( + last_transaction is None + or transaction["dateCreated"] > last_transaction["dateCreated"] + ): + last_transaction = transaction + + if last_transaction: + last_action_by_phid = last_transaction["authorPHID"] + if last_action_by_phid == author_phid: + last_action_by = "author" + elif last_action_by_phid in reviewers: + last_action_by = "reviewer" + else: + last_action_by = "other" + else: + last_action_by = "unknown" + + return last_action_by, last_transaction From e7517fc94717f460dd97abf98ac271a637f88879 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 4 Jun 2024 10:25:17 -0400 Subject: [PATCH 03/19] Added functions from `inactive_reviewer.py` --- bugbot/rules/inactive_revision.py | 199 +++++++++++++++++++++++++++++- 1 file changed, 197 insertions(+), 2 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index ba53eb618..93db471fe 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -2,15 +2,18 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at http://mozilla.org/MPL/2.0/. +import datetime import re from dateutil.relativedelta import relativedelta from libmozdata import utils as lmdutils +from libmozdata.connection import Connection from libmozdata.phabricator import PhabricatorAPI from bugbot import utils from bugbot.bzcleaner import BzCleaner -from bugbot.user_activity import UserActivity +from bugbot.history import History +from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt") PHAB_TABLE_PAT = re.compile(r"^\|\ \[D([0-9]+)\]\(h", flags=re.M) @@ -100,7 +103,7 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: } def _find_last_action(self, revision_id): - details = self._fetch_revision_details(revision_id) + details = self._fetch_revisions([revision_id]) if not details: return None, None @@ -134,3 +137,195 @@ def _find_last_action(self, revision_id): last_action_by = "unknown" return last_action_by, last_transaction + + def _get_revisions_with_inactive_action(self, rev_ids: list) -> dict: + revisions = [] + + for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): + for revision in self._fetch_revisions(_rev_ids): + # Filter out irrelevant revisions + if ( + len(revision["attachments"]["reviewers"]["reviewers"]) == 0 + or revision["fields"]["status"]["value"] != "needs-review" + or revision["fields"]["isDraft"] + ): + continue + + reviewers = [ + { + "phid": reviewer["reviewerPHID"], + "is_group": reviewer["reviewerPHID"].startswith("PHID-PROJ"), + "is_blocking": reviewer["isBlocking"], + "is_accepted": reviewer["status"] == "accepted", + "is_resigned": reviewer["status"] == "resigned", + } + for reviewer in revision["attachments"]["reviewers"]["reviewers"] + ] + + # Skip revisions where all reviewers are active or no blocking reviewer is inactive + if any( + reviewer["is_group"] or reviewer["is_accepted"] + for reviewer in reviewers + ) and not any( + not reviewer["is_accepted"] + for reviewer in reviewers + if reviewer["is_blocking"] + ): + continue + + revisions.append( + { + "rev_id": revision["id"], + "title": revision["fields"]["title"], + "created_at": revision["fields"]["dateCreated"], + "author_phid": revision["fields"]["authorPHID"], + "reviewers": reviewers, + } + ) + + user_phids = set() + for revision in revisions: + user_phids.add(revision["author_phid"]) + for reviewer in revision["reviewers"]: + user_phids.add(reviewer["phid"]) + + users = self.user_activity.get_phab_users_with_status( + list(user_phids), keep_active=True + ) + + result = {} + for revision in revisions: + author_info = users[revision["author_phid"]] + if author_info["status"] != UserStatus.ACTIVE: + continue + + revision["author"] = author_info + + inactive_reviewers = [] + for reviewer in revision["reviewers"]: + if reviewer["is_group"]: + continue + + reviewer_info = users[reviewer["phid"]] + if ( + not reviewer["is_resigned"] + and reviewer_info["status"] == UserStatus.ACTIVE + ): + continue + + reviewer["info"] = reviewer_info + inactive_reviewers.append(reviewer) + + last_action_by, last_transaction = self._find_last_action( + revision["rev_id"] + ) + + if last_action_by in ["author", "reviewer"]: + revision["last_action_by"] = last_action_by + revision["last_transaction"] = last_transaction + revision["reviewers"] = [ + { + "phab_username": reviewer["info"]["phab_username"], + "status_note": self._get_reviewer_status_note(reviewer), + } + for reviewer in inactive_reviewers + ] + result[revision["rev_id"]] = revision + + return result + + def _fetch_revisions(self, revision_ids): + return self.phab.request( + "differential.revision.search", + constraints={"ids": revision_ids}, + attachments={"reviewers": True}, + )["data"] + + def _fetch_revision_transactions(self, revision_phid): + response = self.phab.request( + "transaction.search", objectIdentifier=revision_phid + ) + return response["data"] + + def handle_bug(self, bug, data): + rev_ids = [ + # To avoid loading the attachment content (which can be very large), + # we extract the revision id from the file name, which is in the + # format of "phabricator-D{revision_id}-url.txt". + # len("phabricator-D") == 13 + # len("-url.txt") == 8 + int(attachment["file_name"][13:-8]) + for attachment in bug["attachments"] + if attachment["content_type"] == "text/x-phabricator-request" + and PHAB_FILE_NAME_PAT.match(attachment["file_name"]) + and not attachment["is_obsolete"] + ] + + if not rev_ids: + return + + # We should not comment about the same patch more than once. + rev_ids_with_ni = set() + for comment in bug["comments"]: + if comment["creator"] == History.BOT and comment["raw_text"].startswith( + "The following patch" + ): + rev_ids_with_ni.update( + int(id) for id in PHAB_TABLE_PAT.findall(comment["raw_text"]) + ) + + if rev_ids_with_ni: + rev_ids = [id for id in rev_ids if id not in rev_ids_with_ni] + + if not rev_ids: + return + + # It will be nicer to show a sorted list of patches + rev_ids.sort() + + bugid = str(bug["id"]) + data[bugid] = { + "rev_ids": rev_ids, + } + return bug + + def get_bz_params(self, date): + fields = [ + "comments.raw_text", + "comments.creator", + "attachments.file_name", + "attachments.content_type", + "attachments.is_obsolete", + ] + params = { + "include_fields": fields, + "resolution": "---", + "f1": "attachments.mimetype", + "o1": "equals", + "v1": "text/x-phabricator-request", + } + + return params + + @staticmethod + def _get_reviewer_status_note(reviewer: dict) -> str: + if reviewer["is_resigned"]: + return "Resigned from review" + + status = reviewer["info"]["status"] + if status == UserStatus.UNAVAILABLE: + until = reviewer["info"]["unavailable_until"] + if until: + return_date = datetime.date.fromtimestamp(until).strftime("%b %-d, %Y") + return f"Back {return_date}" + + return "Unavailable" + + if status == UserStatus.DISABLED: + return "Disabled" + + return "Inactive" + + +if __name__ == "__main__": + InactiveRevision().run() From 960fca79f1a9e297a802e4144bfa3cf18f074dcc Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Mon, 10 Jun 2024 16:40:54 -0400 Subject: [PATCH 04/19] Added functions from `inactive_reviewer.py` to maintain similar functionality --- bugbot/rules/inactive_revision.py | 69 +++++++++++++++++-------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 93db471fe..dd7deb7b7 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -2,13 +2,15 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at http://mozilla.org/MPL/2.0/. -import datetime import re +from datetime import datetime +from typing import Dict, List from dateutil.relativedelta import relativedelta from libmozdata import utils as lmdutils from libmozdata.connection import Connection from libmozdata.phabricator import PhabricatorAPI +from tenacity import retry, stop_after_attempt, wait_exponential from bugbot import utils from bugbot.bzcleaner import BzCleaner @@ -65,10 +67,16 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): return bugs def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: + has_old_patch = any( + revision["created_at"] < self.old_patch_limit for revision in inactive_revs + ) + for revision in inactive_revs: last_action_by, _ = self._find_last_action(revision["rev_id"]) - if last_action_by == "author": + print(f"\n\nLAST ACTION BY >> {last_action_by}\n\n") + + if last_action_by == "author" and revision["reviewers"]: ni_mail = revision["reviewers"][0]["phab_username"] summary = ( "The last action was by the author, so needinfoing the reviewer." @@ -85,7 +93,7 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: revisions=[revision], nicknames=revision["author"]["nick"], reviewers_status_summary=summary, - has_old_patch=revision["created_at"] < self.old_patch_limit, + has_old_patch=has_old_patch, plural=utils.plural, documentation=self.get_documentation(), ) @@ -138,12 +146,10 @@ def _find_last_action(self, revision_id): return last_action_by, last_transaction - def _get_revisions_with_inactive_action(self, rev_ids: list) -> dict: - revisions = [] - + def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: + revisions: List[dict] = [] for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): for revision in self._fetch_revisions(_rev_ids): - # Filter out irrelevant revisions if ( len(revision["attachments"]["reviewers"]["reviewers"]) == 0 or revision["fields"]["status"]["value"] != "needs-review" @@ -162,7 +168,6 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> dict: for reviewer in revision["attachments"]["reviewers"]["reviewers"] ] - # Skip revisions where all reviewers are active or no blocking reviewer is inactive if any( reviewer["is_group"] or reviewer["is_accepted"] for reviewer in reviewers @@ -193,7 +198,7 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> dict: list(user_phids), keep_active=True ) - result = {} + result: Dict[int, dict] = {} for revision in revisions: author_info = users[revision["author_phid"]] if author_info["status"] != UserStatus.ACTIVE: @@ -234,10 +239,33 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> dict: return result - def _fetch_revisions(self, revision_ids): + @staticmethod + def _get_reviewer_status_note(reviewer: dict) -> str: + if reviewer["is_resigned"]: + return "Resigned from review" + + status = reviewer["info"]["status"] + if status == UserStatus.UNAVAILABLE: + until = reviewer["info"]["unavailable_until"] + if until: + return_date = datetime.fromtimestamp(until).strftime("%b %-d, %Y") + return f"Back {return_date}" + + return "Unavailable" + + if status == UserStatus.DISABLED: + return "Disabled" + + return "Inactive" + + @retry( + wait=wait_exponential(min=4), + stop=stop_after_attempt(5), + ) + def _fetch_revisions(self, ids: list): return self.phab.request( "differential.revision.search", - constraints={"ids": revision_ids}, + constraints={"ids": ids}, attachments={"reviewers": True}, )["data"] @@ -307,25 +335,6 @@ def get_bz_params(self, date): return params - @staticmethod - def _get_reviewer_status_note(reviewer: dict) -> str: - if reviewer["is_resigned"]: - return "Resigned from review" - - status = reviewer["info"]["status"] - if status == UserStatus.UNAVAILABLE: - until = reviewer["info"]["unavailable_until"] - if until: - return_date = datetime.date.fromtimestamp(until).strftime("%b %-d, %Y") - return f"Back {return_date}" - - return "Unavailable" - - if status == UserStatus.DISABLED: - return "Disabled" - - return "Inactive" - if __name__ == "__main__": InactiveRevision().run() From f3716bbe816d7a88ab57c46987f1e8c227c9cead Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Mon, 10 Jun 2024 16:42:41 -0400 Subject: [PATCH 05/19] Removed print statement --- bugbot/rules/inactive_revision.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index dd7deb7b7..2a0bc5582 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -73,9 +73,6 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: for revision in inactive_revs: last_action_by, _ = self._find_last_action(revision["rev_id"]) - - print(f"\n\nLAST ACTION BY >> {last_action_by}\n\n") - if last_action_by == "author" and revision["reviewers"]: ni_mail = revision["reviewers"][0]["phab_username"] summary = ( From 7e17ee3b1fdc080029a98b290d8a5ad4c0662331 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 11 Jun 2024 14:59:19 -0400 Subject: [PATCH 06/19] Initial templates for `inactive_revision.py` rule --- templates/inactive_revision.html | 31 ++++++++++++++++++++++++ templates/inactive_revision_needinfo.txt | 19 +++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 templates/inactive_revision.html create mode 100644 templates/inactive_revision_needinfo.txt diff --git a/templates/inactive_revision.html b/templates/inactive_revision.html new file mode 100644 index 000000000..b331bf748 --- /dev/null +++ b/templates/inactive_revision.html @@ -0,0 +1,31 @@ +

The following {{ plural('bug has', data, pword='bugs have') }} patches with inactive reviewers:

+ + + + + + + + + + {% for i, (bugid, summary, revisions) in enumerate(data) -%} + + + + + + {% endfor -%} + +
BugSummaryPatches
+ {{ bugid }} + {{ summary | e }} + +
diff --git a/templates/inactive_revision_needinfo.txt b/templates/inactive_revision_needinfo.txt new file mode 100644 index 000000000..0bfc46dcd --- /dev/null +++ b/templates/inactive_revision_needinfo.txt @@ -0,0 +1,19 @@ +The following {{ plural('patch is', revisions, 'patches are') }} waiting for review from {{ reviewers_status_summary }}: + +| ID | Title | Author | Reviewer Status | +| --- | :---- | ------ | :-------------- | +{% for rev in revisions -%} +{% for reviewer in rev['reviewers'] -%} +{% if rev['reviewers'][0] == reviewer -%} +| [D{{ rev['rev_id'] }}](https://phabricator.services.mozilla.com/D{{ rev['rev_id'] }}) | {{ rev['title'] | e }} | [{{ rev['author']['nick'] }}](https://bugzilla.mozilla.org/user_profile?user_id={{ rev['author']['id'] }}) | +{%- else -%} +| | | | +{%- endif -%} + +[{{ reviewer['phab_username'] }}](https://phabricator.services.mozilla.com/p/{{ reviewer['phab_username'] }}/): {{ reviewer["status_note"] }} | +{% endfor -%} +{% endfor %} + +:{{ nicknames }}, could you please find another reviewer {%- if has_old_patch %} or abandon the patch if it is no longer relevant {%- endif %}? + +{{ documentation }} From 20210a153b1ff8277ada8ef087e6dd654f2b8e90 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Wed, 3 Jul 2024 16:23:09 -0400 Subject: [PATCH 07/19] Added separate needinfo comments depending on who is being needinfo'ed --- bugbot/rules/inactive_revision.py | 11 ++++++++++- templates/inactive_revision_needinfo_author.txt | 3 +++ templates/inactive_revision_needinfo_reviewer.txt | 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 templates/inactive_revision_needinfo_author.txt create mode 100644 templates/inactive_revision_needinfo_reviewer.txt diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 2a0bc5582..b8b4702a4 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -7,6 +7,7 @@ from typing import Dict, List from dateutil.relativedelta import relativedelta +from jinja2 import Environment, FileSystemLoader, Template from libmozdata import utils as lmdutils from libmozdata.connection import Connection from libmozdata.phabricator import PhabricatorAPI @@ -66,6 +67,12 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): return bugs + def load_template(self, template_filename: str) -> Template: + """Load a template given its filename""" + env = Environment(loader=FileSystemLoader("templates")) + template = env.get_template(template_filename) + return template + def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: has_old_patch = any( revision["created_at"] < self.old_patch_limit for revision in inactive_revs @@ -78,15 +85,17 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: summary = ( "The last action was by the author, so needinfoing the reviewer." ) + template = self.load_template(self.name() + "_needinfo_reviewer.txt") elif last_action_by == "reviewer": ni_mail = revision["author"]["phab_username"] summary = ( "The last action was by the reviewer, so needinfoing the author." ) + template = self.load_template(self.name() + "_needinfo_author.txt") else: continue - comment = self.ni_template.render( + comment = template.render( revisions=[revision], nicknames=revision["author"]["nick"], reviewers_status_summary=summary, diff --git a/templates/inactive_revision_needinfo_author.txt b/templates/inactive_revision_needinfo_author.txt new file mode 100644 index 000000000..3825f9b5f --- /dev/null +++ b/templates/inactive_revision_needinfo_author.txt @@ -0,0 +1,3 @@ +:{{ nicknames }}, your patch is still waiting for action. Could you please address the feedback from the reviewer {%- if has_old_patch %} or consider abandoning the patch if it is no longer relevant {%- endif %}? + +{{ documentation }} diff --git a/templates/inactive_revision_needinfo_reviewer.txt b/templates/inactive_revision_needinfo_reviewer.txt new file mode 100644 index 000000000..978beb75c --- /dev/null +++ b/templates/inactive_revision_needinfo_reviewer.txt @@ -0,0 +1,3 @@ +:{{ nicknames }}, you have been asked to review this patch but it is still waiting for your response. Could you please review it {%- if has_old_patch %} or let the author know if it is no longer relevant {%- endif %}? + +{{ documentation }} \ No newline at end of file From ab3df860dcee8d7a1bd946a4b56d741929792b08 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Wed, 3 Jul 2024 16:38:20 -0400 Subject: [PATCH 08/19] Removed old needinfo template --- bugbot/rules/inactive_revision.py | 12 ++++++++---- templates/inactive_revision_needinfo.txt | 19 ------------------- 2 files changed, 8 insertions(+), 23 deletions(-) delete mode 100644 templates/inactive_revision_needinfo.txt diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index b8b4702a4..92dbbacb8 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -36,7 +36,12 @@ def __init__(self, old_patch_months: int = 6): super(InactiveRevision, self).__init__() self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"]) self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab) - self.ni_template = self.get_needinfo_template() + self.ni_author_template = self.load_template( + self.name() + "_needinfo_author.txt" + ) + self.ni_reviewer_template = self.load_template( + self.name() + "_needinfo_reviewer.txt" + ) self.old_patch_limit = ( lmdutils.get_date_ymd("today") - relativedelta(months=old_patch_months) ).timestamp() @@ -68,7 +73,6 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): return bugs def load_template(self, template_filename: str) -> Template: - """Load a template given its filename""" env = Environment(loader=FileSystemLoader("templates")) template = env.get_template(template_filename) return template @@ -85,13 +89,13 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: summary = ( "The last action was by the author, so needinfoing the reviewer." ) - template = self.load_template(self.name() + "_needinfo_reviewer.txt") + template = self.ni_reviewer_template elif last_action_by == "reviewer": ni_mail = revision["author"]["phab_username"] summary = ( "The last action was by the reviewer, so needinfoing the author." ) - template = self.load_template(self.name() + "_needinfo_author.txt") + template = self.ni_author_template else: continue diff --git a/templates/inactive_revision_needinfo.txt b/templates/inactive_revision_needinfo.txt deleted file mode 100644 index 0bfc46dcd..000000000 --- a/templates/inactive_revision_needinfo.txt +++ /dev/null @@ -1,19 +0,0 @@ -The following {{ plural('patch is', revisions, 'patches are') }} waiting for review from {{ reviewers_status_summary }}: - -| ID | Title | Author | Reviewer Status | -| --- | :---- | ------ | :-------------- | -{% for rev in revisions -%} -{% for reviewer in rev['reviewers'] -%} -{% if rev['reviewers'][0] == reviewer -%} -| [D{{ rev['rev_id'] }}](https://phabricator.services.mozilla.com/D{{ rev['rev_id'] }}) | {{ rev['title'] | e }} | [{{ rev['author']['nick'] }}](https://bugzilla.mozilla.org/user_profile?user_id={{ rev['author']['id'] }}) | -{%- else -%} -| | | | -{%- endif -%} - -[{{ reviewer['phab_username'] }}](https://phabricator.services.mozilla.com/p/{{ reviewer['phab_username'] }}/): {{ reviewer["status_note"] }} | -{% endfor -%} -{% endfor %} - -:{{ nicknames }}, could you please find another reviewer {%- if has_old_patch %} or abandon the patch if it is no longer relevant {%- endif %}? - -{{ documentation }} From 89b50accd68df15ee879cb7ab526b73208cc227e Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 9 Jul 2024 15:32:36 -0400 Subject: [PATCH 09/19] Fixed `_get_revisions_with_inactive_action()` to focus on inactive patches rather than inactive reviewers --- bugbot/rules/inactive_revision.py | 166 +++++++++++++----------------- 1 file changed, 72 insertions(+), 94 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 92dbbacb8..c49c526e9 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -3,7 +3,6 @@ # You can obtain one at http://mozilla.org/MPL/2.0/. import re -from datetime import datetime from typing import Dict, List from dateutil.relativedelta import relativedelta @@ -16,22 +15,23 @@ from bugbot import utils from bugbot.bzcleaner import BzCleaner from bugbot.history import History -from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus +from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt") PHAB_TABLE_PAT = re.compile(r"^\|\ \[D([0-9]+)\]\(h", flags=re.M) class InactiveRevision(BzCleaner): - """Bugs with patches that are waiting for review from inactive reviewers""" + """Bugs with inactive patches that are awaiting action from authors or reviewers.""" - def __init__(self, old_patch_months: int = 6): + def __init__(self, old_patch_months: int = 6, patch_activity_months: int = 6): """Constructor Args: old_patch_months: number of months since creation of the patch to be considered old. If the bug has an old patch, we will mention abandon the patch as an option. + patch_activity_months: Number of months since the last activity on the patch. """ super(InactiveRevision, self).__init__() self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"]) @@ -45,9 +45,12 @@ def __init__(self, old_patch_months: int = 6): self.old_patch_limit = ( lmdutils.get_date_ymd("today") - relativedelta(months=old_patch_months) ).timestamp() + self.patch_activity_limit = ( + lmdutils.get_date_ymd("today") - relativedelta(months=patch_activity_months) + ).timestamp() def description(self): - return "Bugs with inactive patch reviewers" + return "Bugs with inactive patches that are awaiting action from authors or reviewers." def columns(self): return ["id", "summary", "revisions"] @@ -90,18 +93,20 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: "The last action was by the author, so needinfoing the reviewer." ) template = self.ni_reviewer_template + nickname = revision["reviewers"][0]["phab_username"] elif last_action_by == "reviewer": ni_mail = revision["author"]["phab_username"] summary = ( "The last action was by the reviewer, so needinfoing the author." ) template = self.ni_author_template + nickname = revision["author"]["phab_username"] else: continue comment = template.render( revisions=[revision], - nicknames=revision["author"]["nick"], + nicknames=nickname, reviewers_status_summary=summary, has_old_patch=has_old_patch, plural=utils.plural, @@ -134,14 +139,7 @@ def _find_last_action(self, revision_id): ] transactions = self._fetch_revision_transactions(revision["phid"]) - - last_transaction = None - for transaction in transactions: - if ( - last_transaction is None - or transaction["dateCreated"] > last_transaction["dateCreated"] - ): - last_transaction = transaction + last_transaction = transactions[0] if transactions else None if last_transaction: last_action_by_phid = last_transaction["authorPHID"] @@ -158,6 +156,7 @@ def _find_last_action(self, revision_id): def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: revisions: List[dict] = [] + for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): for revision in self._fetch_revisions(_rev_ids): if ( @@ -167,36 +166,46 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: ): continue - reviewers = [ - { - "phid": reviewer["reviewerPHID"], - "is_group": reviewer["reviewerPHID"].startswith("PHID-PROJ"), - "is_blocking": reviewer["isBlocking"], - "is_accepted": reviewer["status"] == "accepted", - "is_resigned": reviewer["status"] == "resigned", - } - for reviewer in revision["attachments"]["reviewers"]["reviewers"] - ] - - if any( - reviewer["is_group"] or reviewer["is_accepted"] - for reviewer in reviewers - ) and not any( - not reviewer["is_accepted"] - for reviewer in reviewers - if reviewer["is_blocking"] - ): - continue + _, last_transaction = self._find_last_action(revision["id"]) - revisions.append( - { - "rev_id": revision["id"], - "title": revision["fields"]["title"], - "created_at": revision["fields"]["dateCreated"], - "author_phid": revision["fields"]["authorPHID"], - "reviewers": reviewers, - } - ) + if ( + last_transaction + and last_transaction["dateCreated"] < self.patch_activity_limit + ): + reviewers = [ + { + "phid": reviewer["reviewerPHID"], + "is_group": reviewer["reviewerPHID"].startswith( + "PHID-PROJ" + ), + "is_blocking": reviewer["isBlocking"], + "is_accepted": reviewer["status"] == "accepted", + "is_resigned": reviewer["status"] == "resigned", + } + for reviewer in revision["attachments"]["reviewers"][ + "reviewers" + ] + ] + + if any( + reviewer["is_group"] or reviewer["is_accepted"] + for reviewer in reviewers + ) and not any( + not reviewer["is_accepted"] + for reviewer in reviewers + if reviewer["is_blocking"] + ): + continue + + revisions.append( + { + "rev_id": revision["id"], + "title": revision["fields"]["title"], + "created_at": revision["fields"]["dateCreated"], + "author_phid": revision["fields"]["authorPHID"], + "reviewers": reviewers, + } + ) user_phids = set() for revision in revisions: @@ -210,64 +219,33 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: result: Dict[int, dict] = {} for revision in revisions: - author_info = users[revision["author_phid"]] - if author_info["status"] != UserStatus.ACTIVE: + author_phid = revision["author_phid"] + if author_phid in users: + author_info = users[author_phid] + revision["author"] = author_info + else: continue - revision["author"] = author_info - - inactive_reviewers = [] + reviewers = [] for reviewer in revision["reviewers"]: - if reviewer["is_group"]: + reviewer_phid = reviewer["phid"] + if reviewer_phid in users: + reviewer_info = users[reviewer_phid] + reviewer["info"] = reviewer_info + else: continue + reviewers.append(reviewer) - reviewer_info = users[reviewer["phid"]] - if ( - not reviewer["is_resigned"] - and reviewer_info["status"] == UserStatus.ACTIVE - ): - continue - - reviewer["info"] = reviewer_info - inactive_reviewers.append(reviewer) - - last_action_by, last_transaction = self._find_last_action( - revision["rev_id"] - ) - - if last_action_by in ["author", "reviewer"]: - revision["last_action_by"] = last_action_by - revision["last_transaction"] = last_transaction - revision["reviewers"] = [ - { - "phab_username": reviewer["info"]["phab_username"], - "status_note": self._get_reviewer_status_note(reviewer), - } - for reviewer in inactive_reviewers - ] - result[revision["rev_id"]] = revision + revision["reviewers"] = [ + { + "phab_username": reviewer["info"]["phab_username"], + } + for reviewer in reviewers + ] + result[revision["rev_id"]] = revision return result - @staticmethod - def _get_reviewer_status_note(reviewer: dict) -> str: - if reviewer["is_resigned"]: - return "Resigned from review" - - status = reviewer["info"]["status"] - if status == UserStatus.UNAVAILABLE: - until = reviewer["info"]["unavailable_until"] - if until: - return_date = datetime.fromtimestamp(until).strftime("%b %-d, %Y") - return f"Back {return_date}" - - return "Unavailable" - - if status == UserStatus.DISABLED: - return "Disabled" - - return "Inactive" - @retry( wait=wait_exponential(min=4), stop=stop_after_attempt(5), @@ -347,4 +325,4 @@ def get_bz_params(self, date): if __name__ == "__main__": - InactiveRevision().run() + InactiveRevision(patch_activity_months=0).run() From d7c668983687d667ba4c4e570779e202465c2e3e Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 9 Jul 2024 15:35:02 -0400 Subject: [PATCH 10/19] Removed initialization of `patch_activity_months` arg --- bugbot/rules/inactive_revision.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index c49c526e9..f023dd121 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -325,4 +325,4 @@ def get_bz_params(self, date): if __name__ == "__main__": - InactiveRevision(patch_activity_months=0).run() + InactiveRevision().run() From d7365467c59d4d4651d4015e76680eb71aa30bb3 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 9 Jul 2024 15:40:51 -0400 Subject: [PATCH 11/19] Edited email template --- templates/inactive_revision.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/templates/inactive_revision.html b/templates/inactive_revision.html index b331bf748..4d03b2064 100644 --- a/templates/inactive_revision.html +++ b/templates/inactive_revision.html @@ -1,4 +1,6 @@ -

The following {{ plural('bug has', data, pword='bugs have') }} patches with inactive reviewers:

+

+ The following {{ plural('bug has', data, pword='bugs have') }} inactive patches that need action from either the author or a reviewer: +

From f739f18e1e2300ed69dca06e6efc217e8dce93e4 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 6 Aug 2024 09:30:49 -0400 Subject: [PATCH 12/19] Replaced double negation with `all` --- bugbot/rules/inactive_revision.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index f023dd121..532ff081d 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -190,8 +190,8 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: if any( reviewer["is_group"] or reviewer["is_accepted"] for reviewer in reviewers - ) and not any( - not reviewer["is_accepted"] + ) and all( + reviewer["is_accepted"] for reviewer in reviewers if reviewer["is_blocking"] ): From 9dbb76a1229932716932939f762ff475d56b186c Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 6 Aug 2024 09:52:28 -0400 Subject: [PATCH 13/19] Removed returning "other" case when finding last action --- bugbot/rules/inactive_revision.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 532ff081d..cd1dea9f6 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -139,18 +139,26 @@ def _find_last_action(self, revision_id): ] transactions = self._fetch_revision_transactions(revision["phid"]) - last_transaction = transactions[0] if transactions else None - - if last_transaction: - last_action_by_phid = last_transaction["authorPHID"] - if last_action_by_phid == author_phid: - last_action_by = "author" - elif last_action_by_phid in reviewers: - last_action_by = "reviewer" - else: - last_action_by = "other" + if not transactions: + return "unknown", None + + filtered_transactions = [ + transaction + for transaction in transactions + if transaction["authorPHID"] == author_phid + or transaction["authorPHID"] in reviewers + ] + + if not filtered_transactions: + return "unknown", None + + last_transaction = filtered_transactions[0] + last_action_by_phid = last_transaction["authorPHID"] + + if last_action_by_phid == author_phid: + last_action_by = "author" else: - last_action_by = "unknown" + last_action_by = "reviewer" return last_action_by, last_transaction From 1c07dddaa5281f344b337399b9d3ce16f708450a Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 6 Aug 2024 10:31:49 -0400 Subject: [PATCH 14/19] Added filtering out resigned reviewers --- bugbot/rules/inactive_revision.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index cd1dea9f6..73b3a4eb3 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -205,6 +205,12 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: ): continue + reviewers = [ + reviewer + for reviewer in reviewers + if not reviewer["is_resigned"] + ] + revisions.append( { "rev_id": revision["id"], From 1c83ba3d3617a515e5611eaa4ca0a21f9d74253c Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 6 Aug 2024 16:01:04 -0400 Subject: [PATCH 15/19] Added column for `needinfo_user` in email template --- bugbot/rules/inactive_revision.py | 7 +++++-- templates/inactive_revision.html | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 73b3a4eb3..21eac07d0 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -67,7 +67,8 @@ def get_bugs(self, date="today", bug_ids=[], chunk_size=None): ] if inactive_revs: bug["revisions"] = inactive_revs - self._add_needinfo(bugid, inactive_revs) + needinfo_user = self._add_needinfo(bugid, inactive_revs) + bug["needinfo_user"] = needinfo_user else: del bugs[bugid] @@ -80,7 +81,7 @@ def load_template(self, template_filename: str) -> Template: template = env.get_template(template_filename) return template - def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: + def _add_needinfo(self, bugid: str, inactive_revs: list) -> str: has_old_patch = any( revision["created_at"] < self.old_patch_limit for revision in inactive_revs ) @@ -124,6 +125,8 @@ def _add_needinfo(self, bugid: str, inactive_revs: list) -> None: } ], } + return nickname + return "" def _find_last_action(self, revision_id): details = self._fetch_revisions([revision_id]) diff --git a/templates/inactive_revision.html b/templates/inactive_revision.html index 4d03b2064..ed47fcb46 100644 --- a/templates/inactive_revision.html +++ b/templates/inactive_revision.html @@ -7,10 +7,11 @@ + - {% for i, (bugid, summary, revisions) in enumerate(data) -%} + {% for i, (bugid, summary, revisions, needinfo_user) in enumerate(data) -%} @@ -27,6 +28,7 @@ {% endfor -%} + {% endfor -%} From e34cd1f2fae346e4552eaa9c8bac1f927249633b Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Tue, 6 Aug 2024 18:08:18 -0400 Subject: [PATCH 16/19] Added column for `needinfo_user` --- bugbot/rules/inactive_revision.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 21eac07d0..9f68a6538 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -53,7 +53,7 @@ def description(self): return "Bugs with inactive patches that are awaiting action from authors or reviewers." def columns(self): - return ["id", "summary", "revisions"] + return ["id", "summary", "revisions", "needinfo_user"] def get_bugs(self, date="today", bug_ids=[], chunk_size=None): bugs = super().get_bugs(date, bug_ids, chunk_size) From 3efbfd18685f8e47d0f210efaa88110bbd743b55 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Wed, 7 Aug 2024 14:26:11 -0400 Subject: [PATCH 17/19] Added `max_actions` to the `inactive_revision` rule --- bugbot/rules/inactive_revision.py | 4 ++++ configs/rules.json | 3 +++ 2 files changed, 7 insertions(+) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 9f68a6538..b01a8e7c0 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -48,10 +48,14 @@ def __init__(self, old_patch_months: int = 6, patch_activity_months: int = 6): self.patch_activity_limit = ( lmdutils.get_date_ymd("today") - relativedelta(months=patch_activity_months) ).timestamp() + self.max_actions = utils.get_config(self.name(), "max_actions", 1) def description(self): return "Bugs with inactive patches that are awaiting action from authors or reviewers." + def get_max_actions(self): + return self.max_actions + def columns(self): return ["id", "summary", "revisions", "needinfo_user"] diff --git a/configs/rules.json b/configs/rules.json index 3fca4a8c1..158c37110 100644 --- a/configs/rules.json +++ b/configs/rules.json @@ -463,5 +463,8 @@ "leave_open_sec": { "additional_receivers": "rm", "days_lookup": 2 + }, + "inactive_revision": { + "max_actions": 1 } } From 8172791fb4f47446bc7a7e7959eb873c37b37925 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Fri, 9 Aug 2024 15:10:55 -0400 Subject: [PATCH 18/19] Replaced `Dict` and `List` with `dict` and `list` --- bugbot/rules/inactive_revision.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index b01a8e7c0..3461066f0 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -3,7 +3,6 @@ # You can obtain one at http://mozilla.org/MPL/2.0/. import re -from typing import Dict, List from dateutil.relativedelta import relativedelta from jinja2 import Environment, FileSystemLoader, Template @@ -169,8 +168,8 @@ def _find_last_action(self, revision_id): return last_action_by, last_transaction - def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: - revisions: List[dict] = [] + def _get_revisions_with_inactive_action(self, rev_ids: list) -> dict[int, dict]: + revisions: list[dict] = [] for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE): for revision in self._fetch_revisions(_rev_ids): @@ -238,7 +237,7 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]: list(user_phids), keep_active=True ) - result: Dict[int, dict] = {} + result: dict[int, dict] = {} for revision in revisions: author_phid = revision["author_phid"] if author_phid in users: From 69385c617d228b4395398b17e7363dfc25b78590 Mon Sep 17 00:00:00 2001 From: Benjamin Mah Date: Fri, 9 Aug 2024 15:52:53 -0400 Subject: [PATCH 19/19] Reversed condition and added `continue` --- bugbot/rules/inactive_revision.py | 80 +++++++++++++++---------------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/bugbot/rules/inactive_revision.py b/bugbot/rules/inactive_revision.py index 3461066f0..164ddbab9 100644 --- a/bugbot/rules/inactive_revision.py +++ b/bugbot/rules/inactive_revision.py @@ -183,49 +183,45 @@ def _get_revisions_with_inactive_action(self, rev_ids: list) -> dict[int, dict]: _, last_transaction = self._find_last_action(revision["id"]) if ( - last_transaction - and last_transaction["dateCreated"] < self.patch_activity_limit + not last_transaction + or last_transaction["dateCreated"] >= self.patch_activity_limit ): - reviewers = [ - { - "phid": reviewer["reviewerPHID"], - "is_group": reviewer["reviewerPHID"].startswith( - "PHID-PROJ" - ), - "is_blocking": reviewer["isBlocking"], - "is_accepted": reviewer["status"] == "accepted", - "is_resigned": reviewer["status"] == "resigned", - } - for reviewer in revision["attachments"]["reviewers"][ - "reviewers" - ] - ] - - if any( - reviewer["is_group"] or reviewer["is_accepted"] - for reviewer in reviewers - ) and all( - reviewer["is_accepted"] - for reviewer in reviewers - if reviewer["is_blocking"] - ): - continue - - reviewers = [ - reviewer - for reviewer in reviewers - if not reviewer["is_resigned"] - ] - - revisions.append( - { - "rev_id": revision["id"], - "title": revision["fields"]["title"], - "created_at": revision["fields"]["dateCreated"], - "author_phid": revision["fields"]["authorPHID"], - "reviewers": reviewers, - } - ) + continue + + reviewers = [ + { + "phid": reviewer["reviewerPHID"], + "is_group": reviewer["reviewerPHID"].startswith("PHID-PROJ"), + "is_blocking": reviewer["isBlocking"], + "is_accepted": reviewer["status"] == "accepted", + "is_resigned": reviewer["status"] == "resigned", + } + for reviewer in revision["attachments"]["reviewers"]["reviewers"] + ] + + if any( + reviewer["is_group"] or reviewer["is_accepted"] + for reviewer in reviewers + ) and all( + reviewer["is_accepted"] + for reviewer in reviewers + if reviewer["is_blocking"] + ): + continue + + reviewers = [ + reviewer for reviewer in reviewers if not reviewer["is_resigned"] + ] + + revisions.append( + { + "rev_id": revision["id"], + "title": revision["fields"]["title"], + "created_at": revision["fields"]["dateCreated"], + "author_phid": revision["fields"]["authorPHID"], + "reviewers": reviewers, + } + ) user_phids = set() for revision in revisions:
Bug Summary PatchesNeedinfo Requested
{{ needinfo_user }}