Merge pull request #344 from open-craft/navin/merged-closed-date

feat: automatically fill closed or merged date in project

Author: feanil

openedx_webhooks/gh_projects.py

@@ -7,7 +7,7 @@
 from glom import glom
 
 from openedx_webhooks.tasks import logger
-from openedx_webhooks.types import GhPrMetaDict, GhProject, PrDict, PrId
+from openedx_webhooks.types import GhPrMetaDict, GhProject, PrDict, PrGhProject, PrId
 from openedx_webhooks.utils import graphql_query, memoize_timed, value_graphql_type
 
 # The name of the query is used by FakeGitHub while testing.
@@ -39,12 +39,10 @@
 """
 
 
-def pull_request_projects(pr: PrDict) -> Set[GhProject]:
-    """Return the projects this PR is in.
-
-    The projects are expressed as sets of tuples with owning org and number:
-    {("openedx", 19)}
+def pull_request_projects_info(pr: PrDict) -> list[PrGhProject]:
+    """Return the projects info this PR is in.
 
+    example: [{"id": "some-id", "org": "login_id", "number": "1"}]
     """
     variables = glom(pr, {
         "owner": "base.repo.owner.login",
@@ -58,11 +56,20 @@ def pull_request_projects(pr: PrDict) -> Set[GhProject]:
         (
             "repository.pullRequest.projectItems.nodes",
             [
-                {"org": "project.owner.login", "number": "project.number"}
+                {"id": "id", "org": "project.owner.login", "number": "project.number"}
             ]
         )
     )
-    # I can't figure out how to get glom to make a tuple directly...
+    return projects
+
+
+def pull_request_projects(pr: PrDict, projects: list[PrGhProject] | None = None) -> Set[GhProject]:
+    """
+    Helper method for expressing projects info as sets of tuples with owning org and number:
+    {("openedx", 19)}
+    """
+    if projects is None:
+        projects = pull_request_projects_info(pr)
     return {(p["org"], p["number"]) for p in projects}
 
 

openedx_webhooks/tasks/pr_tracking.py

@@ -11,6 +11,7 @@
 from dataclasses import dataclass, field
 from typing import Dict, List, Optional, Set, cast
 
+from openedx_webhooks import settings
 from openedx_webhooks.auth import get_github_session, get_jira_session
 from openedx_webhooks.bot_comments import (
     BOT_COMMENT_INDICATORS,
@@ -39,6 +40,7 @@
 from openedx_webhooks.gh_projects import (
     add_pull_request_to_project,
     pull_request_projects,
+    pull_request_projects_info,
     update_project_pr_custom_field,
 )
 from openedx_webhooks.info import (
@@ -63,12 +65,11 @@
     GITHUB_MERGED_PR_OBSOLETE_LABELS,
     GITHUB_STATUS_LABELS,
 )
-from openedx_webhooks import settings
 from openedx_webhooks.tasks import logger
 from openedx_webhooks.tasks.jira_work import (
     update_jira_issue,
 )
-from openedx_webhooks.types import GhProject, JiraId, PrDict, PrId
+from openedx_webhooks.types import GhProject, JiraId, PrDict, PrGhProject, PrId
 from openedx_webhooks.utils import (
     get_pr_state,
     log_check_response,
@@ -125,6 +126,9 @@ class PrCurrentInfo:
     # The GitHub projects the PR is in.
     github_projects: Set[GhProject] = field(default_factory=set)
 
+    # The GitHub projects the PR is in.
+    github_projects_info: list[PrGhProject] = field(default_factory=list)
+
     # The status of the cla check.
     cla_check: Optional[Dict[str, str]] = None
 
@@ -190,7 +194,8 @@ def current_support_state(pr: PrDict) -> PrCurrentInfo:
         current.bot_data.update(extract_data_from_comment(body))
 
     current.github_labels = set(lbl["name"] for lbl in pr["labels"])
-    current.github_projects = set(pull_request_projects(pr))
+    current.github_projects_info = pull_request_projects_info(pr)
+    current.github_projects = pull_request_projects(pr, current.github_projects_info)
     current.cla_check = cla_status_on_pr(pr)
 
     return current
@@ -402,6 +407,9 @@ def _fix_ospr(self) -> None:
         # Check the GitHub projects.
         self._fix_projects()
 
+        # Update fields in project for this PR
+        self._fix_project_node_fields()
+
     def _fix_projects(self) -> None:
         """
         Update projects for pr.
@@ -410,19 +418,38 @@ def _fix_projects(self) -> None:
             project_item_id = self.actions.add_pull_request_to_project(
                 pr_node_id=self.pr["node_id"], project=project
             )
-            if not project_item_id:
-                continue
+            self.current.github_projects_info.append({
+                "id": project_item_id,
+                "org": project[0],
+                "number": project[1],
+            })
+
+    def _fix_project_node_fields(self) -> None:
+        """
+        Update pr fields in OSPR project board.
+        """
+        for project in self.current.github_projects_info:
+            if (
+                project["org"] == settings.GITHUB_OSPR_PROJECT[0]
+                and project["number"] == settings.GITHUB_OSPR_PROJECT[1]
+            ):
+                project_item_id = project["id"]
+                break
+        else:
+            return
+        state = get_pr_state(self.pr)
+        if state == "open":
             self.actions.update_project_pr_custom_field(
                 field_name="Date opened",
                 field_value=self.pr["created_at"],
                 item_id=project_item_id,
-                project=project
+                project=settings.GITHUB_OSPR_PROJECT
             )
             # get base repo owner info
             repo_spec = get_repo_spec(self.pr["base"]["repo"]["full_name"])
             owner = repo_spec.owner
             if not owner:
-                continue
+                return
             # get user info if owner is an individual
             if repo_spec.is_owner_individual:
                 owner_info = get_github_user_info(owner)
@@ -432,7 +459,21 @@ def _fix_projects(self) -> None:
                 field_name="Repo Owner / Owning Team",
                 field_value=owner,
                 item_id=project_item_id,
-                project=project
+                project=settings.GITHUB_OSPR_PROJECT
+            )
+        elif state == "merged":
+            self.actions.update_project_pr_custom_field(
+                field_name="Date merged/closed",
+                field_value=self.pr["merged_at"],
+                item_id=project_item_id,
+                project=settings.GITHUB_OSPR_PROJECT
+            )
+        elif state == "closed":
+            self.actions.update_project_pr_custom_field(
+                field_name="Date merged/closed",
+                field_value=self.pr["closed_at"],
+                item_id=project_item_id,
+                project=settings.GITHUB_OSPR_PROJECT
             )
 
     def _make_jira_issue(self, jira_nick) -> None:

openedx_webhooks/types.py

@@ -17,6 +17,9 @@
 # A GitHub project: org name, and number.
 GhProject = Tuple[str, int]
 
+# A GitHub project info: org name, number and pr node id in project.
+PrGhProject = Dict
+
 # A GitHub project metadata json object.
 GhPrMetaDict = Dict
 

tests/fake_github.py

@@ -113,6 +113,7 @@ class PullRequest:
     node_id: str = field(default_factory=fake_node_id)
     created_at: datetime.datetime = field(default_factory=patchable_now)
     closed_at: Optional[datetime.datetime] = None
+    merged_at: Optional[datetime.datetime] = None
     comments: List[int] = field(default_factory=list)
     labels: Set[str] = field(default_factory=set)
     state: str = "open"
@@ -140,6 +141,7 @@ def as_json(self, brief=False) -> Dict:
             },
             "created_at": self.created_at.strftime("%Y-%m-%dT%H:%M:%SZ"),
             "closed_at": self.closed_at.strftime("%Y-%m-%dT%H:%M:%SZ") if self.closed_at else None,
+            "merged_at": self.merged_at.strftime("%Y-%m-%dT%H:%M:%SZ") if self.merged_at else None,
             "url": f"{self.repo.github.host}/repos/{self.repo.full_name}/pulls/{self.number}",
             "html_url": f"https://github.com/{self.repo.full_name}/pull/{self.number}",
         }
@@ -154,6 +156,7 @@ def close(self, merge=False):
         self.state = "closed"
         self.merged = merge
         self.closed_at = datetime.datetime.now()
+        self.merged_at = datetime.datetime.now()
 
     def reopen(self):
         """
@@ -162,6 +165,7 @@ def reopen(self):
         self.state = "open"
         self.merged = False
         self.closed_at = None
+        self.merged_at = None
 
     def add_comment(self, user="someone", **kwargs) -> Comment:
         comment = self.repo.make_comment(user, **kwargs)
@@ -461,7 +465,7 @@ def _graphql_ProjectsForPr(self, owner: str, name: str, number: int) -> Dict:
         for node_id in project_node_ids:
             org, num = self.project_nodes[node_id]
             nodes.append(
-                {"project": {"owner": {"login": org}, "number": num}}
+                {"project": {"owner": {"login": org}, "number": num}, "id": node_id}
             )
         return {
             "data": {
@@ -518,6 +522,7 @@ def _graphql_OrgProjectMetadata(self, orgname: str, number: int) -> dict:
                             "nodes": [
                                 {"name": "Name", "id": "name-id", "dataType": "text"},
                                 {"name": "Date opened", "id": "date-opened-id", "dataType": "date"},
+                                {"name": "Date merged/closed", "id": "date-closed-id", "dataType": "date"},
                                 {"name": "Repo Owner / Owning Team", "id": "repo-owner-id", "dataType": "text"},
                             ]
                         }

tests/test_gh_projects.py

@@ -3,6 +3,7 @@
 from openedx_webhooks.gh_projects import (
     add_pull_request_to_project,
     pull_request_projects,
+    pull_request_projects_info,
 )
 from openedx_webhooks.types import PrId
 
@@ -16,7 +17,9 @@ def test_adding_pr_to_project(fake_github):
     assert not pr.is_in_project(("myorg", 23))
 
     add_pull_request_to_project(prid, pr.node_id, ("myorg", 23))
-    projects = set(pull_request_projects(prj))
+    project_info = pull_request_projects_info(prj)
+    assert project_info == [{'id': 'PROJECT:myorg.23', 'org': 'myorg', 'number': 23}]
+    projects = set(pull_request_projects(prj, project_info))
     assert projects == {("myorg", 23)}
     assert pr.is_in_project(("myorg", 23))
     assert not pr.is_in_project(("anotherorg", 27))

tests/test_pull_request_closed.py

@@ -2,16 +2,13 @@
 
 import pytest
 
-from openedx_webhooks.bot_comments import (
-    BotComment,
-)
 from openedx_webhooks.cla_check import (
     CLA_CONTEXT,
     CLA_STATUS_GOOD,
-    CLA_STATUS_NO_CONTRIBUTIONS,
 )
 from openedx_webhooks.gh_projects import pull_request_projects
 from openedx_webhooks.tasks.github import pull_request_changed
+
 from .helpers import check_issue_link_in_markdown, random_text
 
 # These tests should run when we want to test flaky GitHub behavior.
@@ -139,3 +136,11 @@ def test_pr_closed_labels(fake_github, is_merged):
     pr.close(merge=is_merged)
     pull_request_changed(pr.as_json())
     assert pr.labels == {"open-source-contribution", "custom label 1"}
+
+
+def test_pr_closed_date_on_close(closed_pull_request):
+    pr = closed_pull_request
+    pull_request_changed(pr.as_json())
+    assert pr.repo.github.project_items['date-closed-id'] == {
+        pr.closed_at.isoformat(timespec='seconds') + 'Z',
+    }

tests/test_pull_request_opened.py

@@ -499,6 +499,7 @@ def test_pr_project_fields_data(fake_github, mocker, owner):
     pr = fake_github.make_pull_request(owner="openedx", repo="edx-platform", created_at=created_at)
     pull_request_changed(pr.as_json())
     assert pr.repo.github.project_items['date-opened-id'] == {created_at.isoformat() + 'Z'}
+    assert pr.repo.github.project_items['date-closed-id'] == set()
     owner_type, owner_name = owner.split(":")
     if owner_type == "user":
         assert pr.repo.github.project_items['repo-owner-id'] == {f"{owner_name.title()} (@{owner_name})"}

tests/test_rescan.py

@@ -111,16 +111,19 @@ def test_rescan_repository_dry_run(rescannable_repo, fake_github, fake_jira, pul
             "update_labels_on_pull_request",    # ["open-source-contribution"]
             "add_comment_to_pull_request",      # "Thanks for the pull request, @tusbar!"
             "add_pull_request_to_project",
+            "update_project_pr_custom_field",
         ],
         106: [
             "initial_state",
             "set_cla_status",                   # "The author is authorized to contribute"
             "update_labels_on_pull_request",    # ["open-source-contribution"]
             "add_comment_to_pull_request",      # "Thanks for the pull request, @tusbar!"
             "add_pull_request_to_project",
+            "update_project_pr_custom_field",
         ],
         108: [
             "initial_state",
+            "update_project_pr_custom_field",
         ],
         110: [
             "initial_state",