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..8712a210 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 @@ -68,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, @@ -225,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:")} @@ -477,6 +472,11 @@ 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 + state = get_pr_state(self.pr) + if state == "closed": + ad_hoc_labels -= GITHUB_CLOSED_PR_OBSOLETE_LABELS + elif state == "merged": + ad_hoc_labels -= GITHUB_MERGED_PR_OBSOLETE_LABELS desired_labels.update(ad_hoc_labels) if desired_labels != self.current.github_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"}