#5135

Committed 03 Jul 2024 08:23PM CUT coverage: 21.491% (-0.03%) from 21.518%

Build # #5135

Build Type

push

coveralls-python

Committed by

benjaminmah

Commit Message

Added separate needinfo comments depending on who is being needinfo'ed

Run Details

716 of 3685 branches covered (19.43%)

0 of 8 new or added lines in 1 file covered. (0.0%)

1 existing line in 1 file now uncovered.

1932 of 8990 relevant lines covered (21.49%)

0.21 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

0.0

/bugbot/rules/inactive_revision.py

# 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 datetime import datetime
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
from tenacity import retry, stop_after_attempt, wait_exponential

from bugbot import utils
from bugbot.bzcleaner import BzCleaner
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)


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

        for revision in inactive_revs:
            last_action_by, _ = self._find_last_action(revision["rev_id"])
            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."
                )
                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 = template.render(
                revisions=[revision],
                nicknames=revision["author"]["nick"],
                reviewers_status_summary=summary,
                has_old_patch=has_old_patch,
                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):
        details = self._fetch_revisions([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

    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 (
                    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"]
                ]

                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: Dict[int, dict] = {}
        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

    @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": 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


if __name__ == "__main__":
    InactiveRevision().run()

1	# This Source Code Form is subject to the terms of the Mozilla Public
2	# License, v. 2.0. If a copy of the MPL was not distributed with this file,
3	# You can obtain one at http://mozilla.org/MPL/2.0/.
4
5	import re	×
6	from datetime import datetime	×
7	from typing import Dict, List	×
8
9	from dateutil.relativedelta import relativedelta	×
NEW 10	from jinja2 import Environment, FileSystemLoader, Template	×
11	from libmozdata import utils as lmdutils	×
12	from libmozdata.connection import Connection	×
13	from libmozdata.phabricator import PhabricatorAPI	×
14	from tenacity import retry, stop_after_attempt, wait_exponential	×
15
16	from bugbot import utils	×
17	from bugbot.bzcleaner import BzCleaner	×
18	from bugbot.history import History	×
19	from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus	×
20
21	PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt")	×
22	PHAB_TABLE_PAT = re.compile(r"^\\|\ \[D([0-9]+)\]\(h", flags=re.M)	×
23
24
25	class InactiveRevision(BzCleaner):	×
26	"""Bugs with patches that are waiting for review from inactive reviewers"""
27
28	def __init__(self, old_patch_months: int = 6):	×
29	"""Constructor
30
31	Args:
32	old_patch_months: number of months since creation of the patch to be
33	considered old. If the bug has an old patch, we will mention
34	abandon the patch as an option.
35	"""
36	super(InactiveRevision, self).__init__()	×
37	self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"])	×
38	self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab)	×
39	self.ni_template = self.get_needinfo_template()	×
40	self.old_patch_limit = (	×
41	lmdutils.get_date_ymd("today") - relativedelta(months=old_patch_months)
42	).timestamp()
43
44	def description(self):	×
45	return "Bugs with inactive patch reviewers"	×
46
47	def columns(self):	×
48	return ["id", "summary", "revisions"]	×
49
50	def get_bugs(self, date="today", bug_ids=[], chunk_size=None):	×
51	bugs = super().get_bugs(date, bug_ids, chunk_size)	×
52
53	rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]}	×
54	revisions = self._get_revisions_with_inactive_action(list(rev_ids))	×
55
56	for bugid, bug in list(bugs.items()):	×
57	inactive_revs = [	×
58	revisions[rev_id] for rev_id in bug["rev_ids"] if rev_id in revisions
59	]
60	if inactive_revs:	×
61	bug["revisions"] = inactive_revs	×
62	self._add_needinfo(bugid, inactive_revs)	×
63	else:
64	del bugs[bugid]	×
65
66	self.query_url = utils.get_bz_search_url({"bug_id": ",".join(bugs.keys())})	×
67
68	return bugs	×
69
NEW 70	def load_template(self, template_filename: str) -> Template:	×
71	"""Load a template given its filename"""
NEW 72	env = Environment(loader=FileSystemLoader("templates"))	×
NEW 73	template = env.get_template(template_filename)	×
NEW 74	return template	×
75
76	def _add_needinfo(self, bugid: str, inactive_revs: list) -> None:	×
77	has_old_patch = any(	×
78	revision["created_at"] < self.old_patch_limit for revision in inactive_revs
79	)
80
81	for revision in inactive_revs:	×
82	last_action_by, _ = self._find_last_action(revision["rev_id"])	×
83	if last_action_by == "author" and revision["reviewers"]:	×
84	ni_mail = revision["reviewers"][0]["phab_username"]	×
85	summary = (	×
86	"The last action was by the author, so needinfoing the reviewer."
87	)
NEW 88	template = self.load_template(self.name() + "_needinfo_reviewer.txt")	×
89	elif last_action_by == "reviewer":	×
90	ni_mail = revision["author"]["phab_username"]	×
91	summary = (	×
92	"The last action was by the reviewer, so needinfoing the author."
93	)
NEW 94	template = self.load_template(self.name() + "_needinfo_author.txt")	×
95	else:
96	continue	×
97
NEW 98	comment = template.render(	×
99	revisions=[revision],
100	nicknames=revision["author"]["nick"],
101	reviewers_status_summary=summary,
102	has_old_patch=has_old_patch,
103	plural=utils.plural,
104	documentation=self.get_documentation(),
105	)
106
107	self.autofix_changes[bugid] = {	×
108	"comment": {"body": comment},
109	"flags": [
110	{
111	"name": "needinfo",
112	"requestee": ni_mail,
113	"status": "?",
114	"new": "true",
115	}
116	],
117	}
118
119	def _find_last_action(self, revision_id):	×
120	details = self._fetch_revisions([revision_id])	×
121
122	if not details:	×
123	return None, None	×
124
125	revision = details[0]	×
126	author_phid = revision["fields"]["authorPHID"]	×
127	reviewers = [	×
128	reviewer["reviewerPHID"]
129	for reviewer in revision["attachments"]["reviewers"]["reviewers"]
130	]
131
132	transactions = self._fetch_revision_transactions(revision["phid"])	×
133
134	last_transaction = None	×
135	for transaction in transactions:	×
136	if (	×
137	last_transaction is None
138	or transaction["dateCreated"] > last_transaction["dateCreated"]
139	):
140	last_transaction = transaction	×
141
142	if last_transaction:	×
143	last_action_by_phid = last_transaction["authorPHID"]	×
144	if last_action_by_phid == author_phid:	×
145	last_action_by = "author"	×
146	elif last_action_by_phid in reviewers:	×
147	last_action_by = "reviewer"	×
148	else:
149	last_action_by = "other"	×
150	else:
151	last_action_by = "unknown"	×
152
153	return last_action_by, last_transaction	×
154
155	def _get_revisions_with_inactive_action(self, rev_ids: list) -> Dict[int, dict]:	×
156	revisions: List[dict] = []	×
157	for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE):	×
158	for revision in self._fetch_revisions(_rev_ids):	×
159	if (	×
160	len(revision["attachments"]["reviewers"]["reviewers"]) == 0
161	or revision["fields"]["status"]["value"] != "needs-review"
162	or revision["fields"]["isDraft"]
163	):
164	continue	×
165
166	reviewers = [	×
167	{
168	"phid": reviewer["reviewerPHID"],
169	"is_group": reviewer["reviewerPHID"].startswith("PHID-PROJ"),
170	"is_blocking": reviewer["isBlocking"],
171	"is_accepted": reviewer["status"] == "accepted",
172	"is_resigned": reviewer["status"] == "resigned",
173	}
174	for reviewer in revision["attachments"]["reviewers"]["reviewers"]
175	]
176
177	if any(	×
178	reviewer["is_group"] or reviewer["is_accepted"]
179	for reviewer in reviewers
180	) and not any(
181	not reviewer["is_accepted"]
182	for reviewer in reviewers
183	if reviewer["is_blocking"]
184	):
185	continue	×
186
187	revisions.append(	×
188	{
189	"rev_id": revision["id"],
190	"title": revision["fields"]["title"],
191	"created_at": revision["fields"]["dateCreated"],
192	"author_phid": revision["fields"]["authorPHID"],
193	"reviewers": reviewers,
194	}
195	)
196
197	user_phids = set()	×
198	for revision in revisions:	×
199	user_phids.add(revision["author_phid"])	×
200	for reviewer in revision["reviewers"]:	×
201	user_phids.add(reviewer["phid"])	×
202
203	users = self.user_activity.get_phab_users_with_status(	×
204	list(user_phids), keep_active=True
205	)
206
207	result: Dict[int, dict] = {}	×
208	for revision in revisions:	×
209	author_info = users[revision["author_phid"]]	×
210	if author_info["status"] != UserStatus.ACTIVE:	×
211	continue	×
212
213	revision["author"] = author_info	×
214
215	inactive_reviewers = []	×
216	for reviewer in revision["reviewers"]:	×
217	if reviewer["is_group"]:	×
218	continue	×
219
220	reviewer_info = users[reviewer["phid"]]	×
221	if (	×
222	not reviewer["is_resigned"]
223	and reviewer_info["status"] == UserStatus.ACTIVE
224	):
225	continue	×
226
227	reviewer["info"] = reviewer_info	×
228	inactive_reviewers.append(reviewer)	×
229
230	last_action_by, last_transaction = self._find_last_action(	×
231	revision["rev_id"]
232	)
233
234	if last_action_by in ["author", "reviewer"]:	×
235	revision["last_action_by"] = last_action_by	×
236	revision["last_transaction"] = last_transaction	×
237	revision["reviewers"] = [	×
238	{
239	"phab_username": reviewer["info"]["phab_username"],
240	"status_note": self._get_reviewer_status_note(reviewer),
241	}
242	for reviewer in inactive_reviewers
243	]
244	result[revision["rev_id"]] = revision	×
245
246	return result	×
247
248	@staticmethod	×
249	def _get_reviewer_status_note(reviewer: dict) -> str:	×
250	if reviewer["is_resigned"]:	×
251	return "Resigned from review"	×
252
253	status = reviewer["info"]["status"]	×
254	if status == UserStatus.UNAVAILABLE:	×
255	until = reviewer["info"]["unavailable_until"]	×
256	if until:	×
257	return_date = datetime.fromtimestamp(until).strftime("%b %-d, %Y")	×
258	return f"Back {return_date}"	×
259
260	return "Unavailable"	×
261
262	if status == UserStatus.DISABLED:	×
263	return "Disabled"	×
264
265	return "Inactive"	×
266
267	@retry(	×
268	wait=wait_exponential(min=4),
269	stop=stop_after_attempt(5),
270	)
271	def _fetch_revisions(self, ids: list):	×
272	return self.phab.request(	×
273	"differential.revision.search",
274	constraints={"ids": ids},
275	attachments={"reviewers": True},
276	)["data"]
277
278	def _fetch_revision_transactions(self, revision_phid):	×
279	response = self.phab.request(	×
280	"transaction.search", objectIdentifier=revision_phid
281	)
282	return response["data"]	×
283
284	def handle_bug(self, bug, data):	×
285	rev_ids = [	×
286	# To avoid loading the attachment content (which can be very large),
287	# we extract the revision id from the file name, which is in the
288	# format of "phabricator-D{revision_id}-url.txt".
289	# len("phabricator-D") == 13
290	# len("-url.txt") == 8
291	int(attachment["file_name"][13:-8])
292	for attachment in bug["attachments"]
293	if attachment["content_type"] == "text/x-phabricator-request"
294	and PHAB_FILE_NAME_PAT.match(attachment["file_name"])
295	and not attachment["is_obsolete"]
296	]
297
298	if not rev_ids:	×
299	return	×
300
301	# We should not comment about the same patch more than once.
302	rev_ids_with_ni = set()	×
303	for comment in bug["comments"]:	×
304	if comment["creator"] == History.BOT and comment["raw_text"].startswith(	×
305	"The following patch"
306	):
307	rev_ids_with_ni.update(	×
308	int(id) for id in PHAB_TABLE_PAT.findall(comment["raw_text"])
309	)
310
311	if rev_ids_with_ni:	×
312	rev_ids = [id for id in rev_ids if id not in rev_ids_with_ni]	×
313
314	if not rev_ids:	×
315	return	×
316
317	# It will be nicer to show a sorted list of patches
318	rev_ids.sort()	×
319
320	bugid = str(bug["id"])	×
321	data[bugid] = {	×
322	"rev_ids": rev_ids,
323	}
324	return bug	×
325
326	def get_bz_params(self, date):	×
327	fields = [	×
328	"comments.raw_text",
329	"comments.creator",
330	"attachments.file_name",
331	"attachments.content_type",
332	"attachments.is_obsolete",
333	]
334	params = {	×
335	"include_fields": fields,
336	"resolution": "---",
337	"f1": "attachments.mimetype",
338	"o1": "equals",
339	"v1": "text/x-phabricator-request",
340	}
341
342	return params	×
343
344
345	if __name__ == "__main__":	×
346	InactiveRevision().run()	×

mozilla / relman-auto-nag / #5135

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous