Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: automatically fill closed or merged date in project #344

Merged
merged 1 commit into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions openedx_webhooks/gh_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand All @@ -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}


Expand Down
57 changes: 49 additions & 8 deletions openedx_webhooks/tasks/pr_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 (
Expand All @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the previous version of this would support having multiple desired projects that could be updated but it looks like now we're just hardcoding it so that even thought the setting is a list, we only accept the first item in the list as a valid project? Am I understanding this correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feanil The previous version was also somewhat specific to the OSPR board as the fix_ospr function was only called for OSPRs. But this implementation makes sure that it is only applied to a single OSPR board as the field names are also hardcoded and are only available in OSPR board.

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)
Expand All @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions openedx_webhooks/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 6 additions & 1 deletion tests/fake_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}",
}
Expand All @@ -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):
"""
Expand All @@ -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)
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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"},
]
}
Expand Down
5 changes: 4 additions & 1 deletion tests/test_gh_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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))
Expand Down
13 changes: 9 additions & 4 deletions tests/test_pull_request_closed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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',
}
1 change: 1 addition & 0 deletions tests/test_pull_request_opened.py
Original file line number Diff line number Diff line change
Expand Up @@ -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})"}
Expand Down
3 changes: 3 additions & 0 deletions tests/test_rescan.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down