From 992479d1994063216be313e03ff90d5766e378ef Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 27 Sep 2023 14:08:14 -0400 Subject: [PATCH] feat: adding a jira:xyz label creates a Jira issue --- .../20230927_140851_nedbat_label_for_jira.rst | 5 ++ openedx_webhooks/auth.py | 5 +- openedx_webhooks/bot_comments.py | 14 ++- openedx_webhooks/github_views.py | 1 + openedx_webhooks/tasks/jira_work.py | 5 +- openedx_webhooks/tasks/pr_tracking.py | 88 ++++++++++++------- .../templates/jira_issue_comment.md.j2 | 4 + openedx_webhooks/types.py | 2 +- openedx_webhooks/utils.py | 26 +++--- tests/conftest.py | 8 ++ tests/helpers.py | 16 ++-- tests/test_pull_request_opened.py | 66 ++++++++++++++ 12 files changed, 180 insertions(+), 60 deletions(-) create mode 100644 changelog.d/20230927_140851_nedbat_label_for_jira.rst create mode 100644 openedx_webhooks/templates/jira_issue_comment.md.j2 diff --git a/changelog.d/20230927_140851_nedbat_label_for_jira.rst b/changelog.d/20230927_140851_nedbat_label_for_jira.rst new file mode 100644 index 00000000..17a96d1d --- /dev/null +++ b/changelog.d/20230927_140851_nedbat_label_for_jira.rst @@ -0,0 +1,5 @@ +.. A new scriv changelog fragment. + +- Adding a label like ``jira:xyz`` to a pull request will look in a private + registry of Jira servers for one nicknamed ``xyz``, and then create a Jira + issue there to correspond to the pull request. diff --git a/openedx_webhooks/auth.py b/openedx_webhooks/auth.py index da66a55d..035a6b6a 100644 --- a/openedx_webhooks/auth.py +++ b/openedx_webhooks/auth.py @@ -33,10 +33,9 @@ def get_jira_session(jira_nick): `jira_nick` is a nickname for one of our configured Jira servers. """ # Avoid a circular import. - from openedx_webhooks.info import get_jira_info + from openedx_webhooks.info import get_jira_server_info - jira_info = get_jira_info() - jira_server = jira_info[jira_nick.lower()] + jira_server = get_jira_server_info(jira_nick) session = BaseUrlSession(base_url=jira_server.server) session.auth = (jira_server.email, jira_server.token) session.trust_env = False # prevent reading the local .netrc diff --git a/openedx_webhooks/bot_comments.py b/openedx_webhooks/bot_comments.py index d601ed84..a2fbec27 100644 --- a/openedx_webhooks/bot_comments.py +++ b/openedx_webhooks/bot_comments.py @@ -13,10 +13,11 @@ from flask import render_template from openedx_webhooks.info import ( + get_jira_server_info, is_draft_pull_request, pull_request_has_cla, ) -from openedx_webhooks.types import PrDict +from openedx_webhooks.types import JiraId, PrDict class BotComment(Enum): @@ -152,6 +153,17 @@ def github_end_survey_comment(pull_request: PrDict) -> str: ) +def jira_issue_comment(pull_request: PrDict, jira_id: JiraId) -> str: + """Render a comment about making a new Jira issue.""" + jira_server = get_jira_server_info(jira_id.nick) + return render_template( + "jira_issue_comment.md.j2", + server_url=jira_server.server, + server_description=jira_server.description, + key=jira_id.key, + ) + + def no_contributions_thanks(pull_request: PrDict) -> str: # pylint: disable=unused-argument """ Create a "no contributions" comment. diff --git a/openedx_webhooks/github_views.py b/openedx_webhooks/github_views.py index b24e1220..7776c344 100644 --- a/openedx_webhooks/github_views.py +++ b/openedx_webhooks/github_views.py @@ -91,6 +91,7 @@ def hook_receiver(): "converted_to_draft", "reopened", "enqueued", + "labeled", } def handle_pull_request_event(event): diff --git a/openedx_webhooks/tasks/jira_work.py b/openedx_webhooks/tasks/jira_work.py index 3d0fe84c..bdc1f32e 100644 --- a/openedx_webhooks/tasks/jira_work.py +++ b/openedx_webhooks/tasks/jira_work.py @@ -11,11 +11,12 @@ def update_jira_issue( + jira_nick: str, issue_key: str, summary: Optional[str]=None, description: Optional[str]=None, labels: Optional[List[str]]=None, - ): + ) -> None: """ Update some fields on a Jira issue. """ @@ -34,5 +35,5 @@ def update_jira_issue( # Contrary to the docs, if the bot is not an admin, the setting isn't ignored, # the request fails. url = f"/rest/api/2/issue/{issue_key}?notifyUsers={notify}" - resp = get_jira_session().put(url, json={"fields": fields}) + resp = get_jira_session(jira_nick).put(url, json={"fields": fields}) log_check_response(resp) diff --git a/openedx_webhooks/tasks/pr_tracking.py b/openedx_webhooks/tasks/pr_tracking.py index b765d755..9fc99bff 100644 --- a/openedx_webhooks/tasks/pr_tracking.py +++ b/openedx_webhooks/tasks/pr_tracking.py @@ -21,6 +21,7 @@ github_community_pr_comment, github_community_pr_comment_closed, github_end_survey_comment, + jira_issue_comment, no_contributions_thanks, ) from openedx_webhooks.cla_check import ( @@ -70,25 +71,19 @@ @dataclass class BotData: """ - The data we store hidden on the first bot comment, to track our work. + The data we store hidden in bot comments, to track our work. """ # Is this a draft pull request? draft: bool = False # The Jira issues associated with the pull request. jira_issues: Set[JiraId] = field(default_factory=set) - def asdict(self): - return { - "draft": self.draft, - "jira_issues": [ji.asdict() for ji in self.jira_issues], - } - - @classmethod - def fromdict(cls, d): - return cls( - draft=d.get("draft", False), - jira_issues=set(JiraId(**jd) for jd in d.get("jira_issues", [])), - ) + def update(self, data: dict) -> None: + """Add data from `data` to this BotData.""" + if "draft" in data: + self.draft = data["draft"] + if "jira_issues" in data: + self.jira_issues.update(JiraId(**jd) for jd in data["jira_issues"]) @dataclass @@ -137,6 +132,9 @@ class PrDesiredInfo: jira_title: Optional[str] = None jira_description: Optional[str] = None + # The Jira instances we want to have issues on. + jira_nicks: Set[str] = field(default_factory=set) + # The Jira status to start a new issue at. jira_initial_status: Optional[str] = None @@ -178,7 +176,7 @@ def current_support_state(pr: PrDict) -> PrCurrentInfo: full_bot_comments = list(get_bot_comments(prid)) if full_bot_comments: current.bot_comment0_text = cast(str, full_bot_comments[0]["body"]) - current.bot_data = BotData.fromdict(extract_data_from_comment(current.bot_comment0_text)) + current.bot_data.update(extract_data_from_comment(current.bot_comment0_text)) for comment in full_bot_comments: body = comment["body"] for comment_id, snips in BOT_COMMENT_INDICATORS.items(): @@ -186,6 +184,7 @@ def current_support_state(pr: PrDict) -> PrCurrentInfo: current.bot_comments.add(comment_id) if comment_id == BotComment.SURVEY: current.bot_survey_comment_id = comment["id"] + 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)) @@ -227,6 +226,10 @@ def desired_support_state(pr: PrDict) -> PrDesiredInfo: else: state = "closed" + # A label of jira:xyz means we want a Jira issue in the xyz Jira. + label_names = set(lbl["name"] for lbl in pr["labels"]) + desired.jira_nicks = {name.partition(":")[-1] for name in label_names if name.startswith("jira:")} + desired.jira_initial_status = "Needs Triage" desired.jira_title = pr["title"] desired.jira_description = pr["body"] or "" @@ -325,6 +328,13 @@ def fix(self) -> None: """ The main routine for making needed changes. """ + self.actions.initial_state( + current=json_safe_dict(self.current), + desired=json_safe_dict(self.desired), + ) + + self.fix_result.jira_issues = set(self.current.bot_data.jira_issues) + if self.desired.cla_check != self.current.cla_check: assert self.desired.cla_check is not None self.actions.set_cla_status(status=self.desired.cla_check) @@ -335,6 +345,12 @@ def fix(self) -> None: if self.desired.is_refused: self.fix_comments() + # Make needed Jira issues. + current_jira_nicks = {ji.nick for ji in self.current.bot_data.jira_issues} + for jira_nick in self.desired.jira_nicks: + if jira_nick not in current_jira_nicks: + self._make_jira_issue(jira_nick) + def fix_comments(self) -> None: fix_comment = True if self.pr["state"] == "closed" and self.current.bot_comments: @@ -346,11 +362,6 @@ def fix_comments(self) -> None: self._add_bot_comments() def fix_ospr(self) -> None: - self.actions.initial_state( - current=json_safe_dict(self.current), - desired=json_safe_dict(self.desired), - ) - # Draftiness self.bot_data.draft = is_draft_pull_request(self.pr) @@ -366,22 +377,32 @@ def fix_ospr(self) -> None: pr_node_id=self.pr["node_id"], project=project ) - def _make_jira_issue(self) -> Dict: + def _make_jira_issue(self, jira_nick) -> None: """ - Make our desired Jira issue. + Make a Jira issue in a particular Jira server. """ - assert self.desired.jira_project is not None user_name, institution = get_name_and_institution_for_pr(self.pr) - issue_data = self.actions.create_ospr_issue( + issue_data = self.actions.create_jira_issue( + jira_nick=jira_nick, pr_url=self.pr["html_url"], - project=self.desired.jira_project, + project="TODOXXX", # TODO: get the real project summary=self.desired.jira_title, description=self.desired.jira_description, labels=list(self.desired.jira_labels), user_name=user_name, institution=institution, ) - return issue_data + + jira_id = JiraId(jira_nick, issue_data["key"]) + self.current.bot_data.jira_issues.add(jira_id) + self.fix_result.jira_issues.add(jira_id) + self.fix_result.changed_jira_issues.add(jira_id) + + comment_body = jira_issue_comment(self.pr, jira_id) + comment_body += format_data_for_comment({ + "jira_issues": [jira_id.asdict()], + }) + self.actions.add_comment_to_pull_request(comment_body=comment_body) def _fix_github_labels(self) -> None: """ @@ -437,7 +458,9 @@ def _fix_bot_comment(self) -> None: needed_comments.remove(BotComment.END_OF_WIP) # BTW, we never have WELCOME_CLOSED in desired.bot_comments - comment_body += format_data_for_comment(self.bot_data.asdict()) + comment_body += format_data_for_comment({ + "draft": is_draft_pull_request(self.pr) + }) if comment_body != self.current.bot_comment0_text: # If there are current-state comments, then we need to edit the @@ -507,9 +530,9 @@ class DryRunFixingActions: def __init__(self): self.action_calls = [] - def create_ospr_issue(self, **kwargs): + def create_jira_issue(self, **kwargs): # This needs a special override because it has to return a Jira key. - self.action_calls.append(("create_ospr_issue", kwargs)) + self.action_calls.append(("create_jira_issue", kwargs)) return { "key": f"OSPR-{next(self.jira_ids)}", "fields": { @@ -543,8 +566,9 @@ def initial_state(self, *, current: Dict, desired: Dict) -> None: Does nothing when really fixing, but captures information for dry runs. """ - def create_ospr_issue( + def create_jira_issue( self, *, + jira_nick: str, pr_url: str, project: str, summary: Optional[str], @@ -554,12 +578,12 @@ def create_ospr_issue( institution: Optional[str], ) -> Dict: """ - Create a new OSPR or OSPR-like issue for a pull request. + Create a new Jira issue for a pull request. Returns the JSON describing the issue. """ - custom_fields = get_jira_custom_fields() + custom_fields = get_jira_custom_fields(jira_nick) new_issue = { "fields": { "project": { @@ -582,7 +606,7 @@ def create_ospr_issue( sentry_extra_context({"new_issue": new_issue}) logger.info(f"Creating new JIRA issue for PR {self.prid}...") - resp = get_jira_session().post("/rest/api/2/issue", json=new_issue) + resp = get_jira_session(jira_nick).post("/rest/api/2/issue", json=new_issue) log_check_response(resp) # Jira only sends the key. Put it into the JSON we started with, and diff --git a/openedx_webhooks/templates/jira_issue_comment.md.j2 b/openedx_webhooks/templates/jira_issue_comment.md.j2 new file mode 100644 index 00000000..af41eede --- /dev/null +++ b/openedx_webhooks/templates/jira_issue_comment.md.j2 @@ -0,0 +1,4 @@ +{% filter replace("\n", " ")|trim %} +{# The bot makes this comment when it creates a label-triggered Jira issue. #} +I've created issue [{{ key }}]({{ server_url }}/browse/{{ key }}) in {{ server_description }}. +{% endfilter %} diff --git a/openedx_webhooks/types.py b/openedx_webhooks/types.py index e0fbe57d..15d2d64d 100644 --- a/openedx_webhooks/types.py +++ b/openedx_webhooks/types.py @@ -20,7 +20,7 @@ @dataclasses.dataclass(frozen=True) class PrId: - """An id of a pull request, with three parts used by GitHub.""" + """An id of a pull request, with a repo full_name and an id.""" full_name: str number: int diff --git a/openedx_webhooks/utils.py b/openedx_webhooks/utils.py index eb9d2aee..c66b9ebd 100644 --- a/openedx_webhooks/utils.py +++ b/openedx_webhooks/utils.py @@ -19,7 +19,6 @@ from openedx_webhooks import logger from openedx_webhooks.auth import get_github_session, get_jira_session -from openedx_webhooks.settings import settings from openedx_webhooks.types import JiraDict @@ -306,21 +305,18 @@ def sentry_extra_context(data_dict): @memoize_timed(minutes=30) -def get_jira_custom_fields(): +def get_jira_custom_fields(jira_nick: str): """ Return a name-to-id mapping for the custom fields on JIRA. """ - if settings.JIRA_SERVER: - session = get_jira_session() - field_resp = session.get("/rest/api/2/field") - field_resp.raise_for_status() - fields = field_resp.json() - return {f["name"]: f["id"] for f in fields if f["custom"]} - else: - return {} + session = get_jira_session(jira_nick) + field_resp = session.get("/rest/api/2/field") + field_resp.raise_for_status() + fields = field_resp.json() + return {f["name"]: f["id"] for f in fields if f["custom"]} -def get_jira_issue(key: str, missing_ok: bool = False) -> Optional[JiraDict]: +def get_jira_issue(jira_nick: str, key: str, missing_ok: bool = False) -> Optional[JiraDict]: """ Get the dictionary for a Jira issue, from its key. @@ -333,20 +329,20 @@ def get_jira_issue(key: str, missing_ok: bool = False) -> Optional[JiraDict]: is missing. """ - resp = jira_get("/rest/api/2/issue/{key}".format(key=key)) + resp = jira_get(jira_nick, "/rest/api/2/issue/{key}".format(key=key)) if resp.status_code == 404 and missing_ok: return None log_check_response(resp) return resp.json() -def jira_get(*args, **kwargs): +def jira_get(jira_nick, *args, **kwargs): """ JIRA sometimes returns an empty response to a perfectly valid GET request, so this will retry it a few times if that happens. """ for _ in range(3): - resp = get_jira_session().get(*args, **kwargs) + resp = get_jira_session(jira_nick).get(*args, **kwargs) if resp.content: return resp - return get_jira_session().get(*args, **kwargs) + return get_jira_session(jira_nick).get(*args, **kwargs) diff --git a/tests/conftest.py b/tests/conftest.py index c5145ee2..42a4fa4b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -98,10 +98,18 @@ def fake_github(pytestconfig, mocker, requests_mocker, fake_repo_data): @pytest.fixture def fake_jira(requests_mocker, fake_repo_data): + """A FakeJira for the first server configured in our jira-info.yaml.""" the_fake_jira = FakeJira("https://test.atlassian.net") the_fake_jira.install_mocks(requests_mocker) return the_fake_jira +@pytest.fixture +def fake_jira2(requests_mocker, fake_repo_data): + """A FakeJira for the second server configured in our jira-info.yaml.""" + the_fake_jira = FakeJira("https://anothertest.atlassian.net") + the_fake_jira.install_mocks(requests_mocker) + return the_fake_jira + @pytest.fixture(autouse=True) def configure_flask_app(): diff --git a/tests/helpers.py b/tests/helpers.py index ac608b63..a153c526 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -3,6 +3,9 @@ import random import re +from openedx_webhooks.info import get_jira_server_info +from openedx_webhooks.types import JiraId + def check_good_markdown(text: str) -> None: """ @@ -33,21 +36,22 @@ def check_good_markdown(text: str) -> None: raise ValueError(f"Markdown has a link to a None url: {text!r}") -def check_issue_link_in_markdown(text: str, issue_id: str|None) -> None: +def check_issue_link_in_markdown(text: str, jira_id: JiraId|None) -> None: """ - Check that `text` has properly links to `issue_id`. + Check that `text` has properly formatted links to `jira_id`. Args: text: Markdown text. - issue_id: A JIRA issue id, which can be None. + jira_id: A JiraId (nick, key) pair, which can be None. Returns: Nothing. Will raise an exception with a failure message if something is wrong. """ - if issue_id is not None: - jira_link = "[{id}](https://test.atlassian.net/browse/{id})".format(id=issue_id) - assert jira_link in text, f"Markdown is missing a link to {issue_id}" + if jira_id is not None: + jira_server = get_jira_server_info(jira_id.nick) + jira_link = f"[{jira_id.key}]({jira_server.server}/browse/{jira_id.key})" + assert jira_link in text, f"Markdown is missing a link to {jira_id}" else: assert "/browse/" not in text, "Markdown links to JIRA when we have no issue id" diff --git a/tests/test_pull_request_opened.py b/tests/test_pull_request_opened.py index 8a94498c..43eaa6e7 100644 --- a/tests/test_pull_request_opened.py +++ b/tests/test_pull_request_opened.py @@ -332,3 +332,69 @@ def test_add_to_multiple_projects(fake_github): assert pull_request_projects(pr.as_json()) == { settings.GITHUB_OSPR_PROJECT, ("openedx", 23), ("anotherorg", 17), } + + +def test_jira_labelling(fake_github, fake_jira, fake_jira2): + # A PR with a "jira:" label makes a Jira issue. + pr = fake_github.make_pull_request("openedx", user="nedbat", title="Ned's PR") + pr.set_labels(["jira:test1"]) + assert len(pr.list_comments()) == 0 + + result = pull_request_changed(pr.as_json()) + assert len(result.jira_issues) == 1 + assert len(result.changed_jira_issues) == 1 + assert len(fake_jira.issues) == 1 + assert len(fake_jira2.issues) == 0 + pr_comments = pr.list_comments() + assert len(pr_comments) == 1 + body = pr_comments[-1].body + jira_id = result.changed_jira_issues.pop() + check_issue_link_in_markdown(body, jira_id) + assert "in the private Test1 Jira" in body + jira_issue = fake_jira.issues[jira_id.key] + assert jira_issue.summary == "Ned's PR" + + # Processing the pull request again won't make another issue. + result = pull_request_changed(pr.as_json()) + assert len(result.jira_issues) == 1 + assert len(result.changed_jira_issues) == 0 + assert len(fake_jira.issues) == 1 + assert len(fake_jira2.issues) == 0 + assert len(pr.list_comments()) == 1 + + +def test_jira_labelling_later(fake_github, fake_jira, fake_jira2): + # You can add the label later, and get a Jira issue. + # At first, no labels, so no Jira issues: + pr = fake_github.make_pull_request("openedx", user="nedbat", title="Yet another PR") + result = pull_request_changed(pr.as_json()) + assert len(result.jira_issues) == 0 + assert len(result.changed_jira_issues) == 0 + assert len(fake_jira.issues) == 0 + assert len(fake_jira2.issues) == 0 + + # Make a label, get an issue: + pr.set_labels(["jira:test1"]) + result = pull_request_changed(pr.as_json()) + assert len(result.jira_issues) == 1 + assert len(result.changed_jira_issues) == 1 + assert len(fake_jira.issues) == 1 + assert len(fake_jira2.issues) == 0 + pr_comments = pr.list_comments() + assert len(pr_comments) == 1 + + # You can add a second label for another Jira server. + pr.set_labels(["jira:AnotherTest"]) + result = pull_request_changed(pr.as_json()) + assert len(result.jira_issues) == 2 + assert len(result.changed_jira_issues) == 1 + assert len(fake_jira.issues) == 1 + assert len(fake_jira2.issues) == 1 + pr_comments = pr.list_comments() + assert len(pr_comments) == 2 + body = pr_comments[-1].body + jira_id = result.changed_jira_issues.pop() + check_issue_link_in_markdown(body, jira_id) + assert "in the Another Test Jira" in body + jira_issue = fake_jira2.issues[jira_id.key] + assert jira_issue.summary == "Yet another PR"