From 72ba8d6cd50752bc1e91ae1b7c9b2c82ff5c4939 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 20 Jan 2025 19:16:11 +0530 Subject: [PATCH 1/2] feat: remove obsolete labels in closed/merged PRs --- openedx_webhooks/labels.py | 21 +++++++++++++++++++++ openedx_webhooks/tasks/pr_tracking.py | 6 ++++++ 2 files changed, 27 insertions(+) diff --git a/openedx_webhooks/labels.py b/openedx_webhooks/labels.py index e5a53ac8..4792de54 100644 --- a/openedx_webhooks/labels.py +++ b/openedx_webhooks/labels.py @@ -14,3 +14,24 @@ "blended", "open-source-contribution", } + +GITHUB_MERGED_PR_OBSOLETE_LABELS = { + "blocked by other work", + "changes requested", + "inactive", + "needs maintainer attention", + "needs more product information", + "needs rescoping", + "needs reviewer assigned", + "needs test run", + "waiting for eng review", + "waiting on author", +} + +GITHUB_CLOSED_PR_OBSOLETE_LABELS = { + "needs maintainer attention", + "needs reviewer assigned", + "needs test run", + "waiting for eng review", + "waiting on author", +} diff --git a/openedx_webhooks/tasks/pr_tracking.py b/openedx_webhooks/tasks/pr_tracking.py index bc8f579b..8927410c 100644 --- a/openedx_webhooks/tasks/pr_tracking.py +++ b/openedx_webhooks/tasks/pr_tracking.py @@ -59,6 +59,8 @@ ) from openedx_webhooks.labels import ( GITHUB_CATEGORY_LABELS, + GITHUB_CLOSED_PR_OBSOLETE_LABELS, + GITHUB_MERGED_PR_OBSOLETE_LABELS, GITHUB_STATUS_LABELS, ) from openedx_webhooks import settings @@ -477,6 +479,10 @@ def _fix_github_labels(self) -> None: """ desired_labels = set(self.desired.github_labels) ad_hoc_labels = self.current.github_labels - GITHUB_CATEGORY_LABELS - GITHUB_STATUS_LABELS + if self.pr["state"] == "closed": + ad_hoc_labels -= GITHUB_CLOSED_PR_OBSOLETE_LABELS + elif self.pr["state"] == "merged": + ad_hoc_labels -= GITHUB_MERGED_PR_OBSOLETE_LABELS desired_labels.update(ad_hoc_labels) if desired_labels != self.current.github_labels: From 4401cf39ff97b8803c79c78f5137f1bc684a984f Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 21 Jan 2025 13:00:08 +0530 Subject: [PATCH 2/2] test: removing obsolete labels --- openedx_webhooks/tasks/pr_tracking.py | 16 +++++----------- openedx_webhooks/utils.py | 17 ++++++++++++++++- tests/fake_github.py | 1 + tests/test_pull_request_closed.py | 17 +++++++++++++++++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/openedx_webhooks/tasks/pr_tracking.py b/openedx_webhooks/tasks/pr_tracking.py index 8927410c..8712a210 100644 --- a/openedx_webhooks/tasks/pr_tracking.py +++ b/openedx_webhooks/tasks/pr_tracking.py @@ -70,6 +70,7 @@ ) from openedx_webhooks.types import GhProject, JiraId, PrDict, PrId from openedx_webhooks.utils import ( + get_pr_state, log_check_response, sentry_extra_context, text_summary, @@ -227,15 +228,7 @@ def desired_support_state(pr: PrDict) -> PrDesiredInfo: else: desired.is_ospr = True - if pr.get("hook_action") == "reopened": - state = "reopened" - elif pr["state"] == "open": - state = "open" - elif pr["merged"]: - state = "merged" - else: - state = "closed" - + state = get_pr_state(pr) # A label of jira:xyz means we want a Jira issue in the xyz Jira. desired.jira_nicks = {name.partition(":")[-1] for name in label_names if name.startswith("jira:")} @@ -479,9 +472,10 @@ def _fix_github_labels(self) -> None: """ desired_labels = set(self.desired.github_labels) ad_hoc_labels = self.current.github_labels - GITHUB_CATEGORY_LABELS - GITHUB_STATUS_LABELS - if self.pr["state"] == "closed": + state = get_pr_state(self.pr) + if state == "closed": ad_hoc_labels -= GITHUB_CLOSED_PR_OBSOLETE_LABELS - elif self.pr["state"] == "merged": + elif state == "merged": ad_hoc_labels -= GITHUB_MERGED_PR_OBSOLETE_LABELS desired_labels.update(ad_hoc_labels) diff --git a/openedx_webhooks/utils.py b/openedx_webhooks/utils.py index 897331f6..5bee2883 100644 --- a/openedx_webhooks/utils.py +++ b/openedx_webhooks/utils.py @@ -19,7 +19,7 @@ from openedx_webhooks import logger from openedx_webhooks.auth import get_github_session, get_jira_session -from openedx_webhooks.types import JiraDict +from openedx_webhooks.types import JiraDict, PrDict def environ_get(name: str, default=None) -> str: @@ -345,3 +345,18 @@ def jira_get(jira_nick, *args, **kwargs): if resp.content: return resp return get_jira_session(jira_nick).get(*args, **kwargs) + + +def get_pr_state(pr: PrDict): + """ + Get gthub pull request state. + """ + if pr.get("hook_action") == "reopened": + state = "reopened" + elif pr["state"] == "open": + state = "open" + elif pr["merged"]: + state = "merged" + else: + state = "closed" + return state diff --git a/tests/fake_github.py b/tests/fake_github.py index 05a75c5b..a8ffa1ed 100644 --- a/tests/fake_github.py +++ b/tests/fake_github.py @@ -385,6 +385,7 @@ def _patch_issues(self, match, request, _context) -> Dict: patch = request.json() if "labels" in patch: pr.set_labels(patch["labels"]) + r.pull_requests[pr.number] = pr return pr.as_json() # Commmits diff --git a/tests/test_pull_request_closed.py b/tests/test_pull_request_closed.py index 1e19e68d..da2a9e33 100644 --- a/tests/test_pull_request_closed.py +++ b/tests/test_pull_request_closed.py @@ -122,3 +122,20 @@ def test_pr_closed_after_employee_leaves(org, is_merged, fake_github, mocker): assert pr.status(CLA_CONTEXT) == CLA_STATUS_GOOD assert pr.labels == set() assert pull_request_projects(pr.as_json()) == set() + + +def test_pr_closed_labels(fake_github, is_merged): + """ + Test whether obsolete labels are removed on closing merge requests + """ + pr = fake_github.make_pull_request( + user="newuser", + owner="openedx", + repo="edx-platform", + body=None, + ) + pr.set_labels({"open-source-contribution", "waiting on author", "needs test run", "custom label 1"}) + + pr.close(merge=is_merged) + pull_request_changed(pr.as_json()) + assert pr.labels == {"open-source-contribution", "custom label 1"}