diff --git a/docs/actions.md b/docs/actions.md index ab59013d..983ec5c3 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -166,3 +166,4 @@ linked Jira issue status to "Closed". If the bug changes to a status not listed Syncs the Bugzilla whitboard tags field to the Jira labels field. - `maybe_update_components`: looks at the component that's set on the bug (if any) and any components added to the project configuration with the `jira_components` parameter (see above). If those components are available on the Jira side as well, they're added to the Jira issue - `maybe_add_phabricator_link`: looks at an attachment and if it is a phabricator attachment, it gets added as a link or updated if the attachment was previously added. +- `maybe_add_github_link`: looks at an attachment and if it is a Github attachment, it gets added as a link or updated if the attachment was previously added. diff --git a/jbi/bugzilla/models.py b/jbi/bugzilla/models.py index a474c255..23fda9a2 100644 --- a/jbi/bugzilla/models.py +++ b/jbi/bugzilla/models.py @@ -103,6 +103,37 @@ class WebhookAttachment(BaseModel, frozen=True): is_patch: bool is_private: bool + def is_github_pull_request(self) -> bool: + """ + Returns True if this attachment is a github pull request attachment. + + We identify an attachment as a pull request if the content type contains "github-pull-request" + """ + return "github-pull-request" in self.content_type + + def github_url(self, base_url: str) -> str | None: + """ + Returns the github patch URL from the file name if the attachment is a patch, otherwise, it returns None. + """ + if not self.is_github_pull_request(): + return None + + match = re.search(r"\d+", self.file_name) + if not match: + logger.info( + "Expected that attachment with name %s is a pull request, but we couldn't extract the pull request id", + self.file_name, + extra={ + "bug": { + "id": self.id, + } + }, + ) + return None + + revision_id = match.group(0) + return f"{base_url}/{revision_id}" + def is_phabricator_patch(self) -> bool: """ Returns True if this attachment is a phabricator patch attachment. diff --git a/jbi/environment.py b/jbi/environment.py index 16a25d3f..1aef5fe1 100644 --- a/jbi/environment.py +++ b/jbi/environment.py @@ -40,6 +40,9 @@ class Settings(BaseSettings): bugzilla_base_url: str = "https://bugzilla-dev.allizom.org" bugzilla_api_key: str + # Github + github_base_url: str = "https://github.com/mozilla-firefox/firefox/pull" + # Phabricator phabricator_base_url: str = "https://phabricator.services.mozilla.com" diff --git a/jbi/jira/service.py b/jbi/jira/service.py index 3dd171fb..3fa5973d 100644 --- a/jbi/jira/service.py +++ b/jbi/jira/service.py @@ -137,6 +137,9 @@ def add_jira_comment(self, context: ActionContext): ): formatted_comment += f"\n*Phabricator URL*: {phabricator_url}" + if github_url := att.github_url(base_url=settings.github_base_url): + formatted_comment += f"\n*Github URL*: {github_url}" + else: comment = context.bug.comment assert comment # See jbi.steps.create_comment() diff --git a/jbi/models.py b/jbi/models.py index a3463186..81b0062e 100644 --- a/jbi/models.py +++ b/jbi/models.py @@ -46,6 +46,7 @@ class ActionSteps(BaseModel, frozen=True): attachment: list[str] = [ "create_comment", "maybe_add_phabricator_link", + "maybe_add_github_link", ] @field_validator("*") diff --git a/jbi/steps.py b/jbi/steps.py index 3bce543e..6f7cebac 100644 --- a/jbi/steps.py +++ b/jbi/steps.py @@ -119,6 +119,53 @@ def add_link_to_bugzilla( return (StepStatus.SUCCESS, context) +def maybe_add_github_link( + context: ActionContext, + *, + jira_service: JiraService, +) -> StepResult: + """Add a Github link to the Jira issue if an attachment is a Github attachment""" + if context.event.target != "attachment" or not context.bug.attachment: + return (StepStatus.NOOP, context) + + attachment = context.bug.attachment + + settings = get_settings() + github_url = attachment.github_url(base_url=settings.github_base_url) + + if not github_url: + return (StepStatus.NOOP, context) + + description = attachment.description + if attachment.is_obsolete: + description = f"{0} - {1}".format("Abandoned", attachment.description) + + issue_key = context.jira.issue + + jira_response = jira_service.client.create_or_update_issue_remote_links( + issue_key=issue_key, + global_id=f"{context.bug.id}-{attachment.id}", + link_url=github_url, + title=description, + ) + + if jira_response: + logger.info( + "Github pull request added or updated in Jira issue %s", + issue_key, + extra=context.update(operation=Operation.LINK).model_dump(), + ) + context = context.append_responses(jira_response) + return (StepStatus.SUCCESS, context) + else: + logger.info( + "Failed to add or update github url in Jira issue %s", + issue_key, + ) + + return (StepStatus.NOOP, context) + + def maybe_add_phabricator_link( context: ActionContext, *, diff --git a/tests/unit/bugzilla/test_models.py b/tests/unit/bugzilla/test_models.py index ad61a786..216e8085 100644 --- a/tests/unit/bugzilla/test_models.py +++ b/tests/unit/bugzilla/test_models.py @@ -7,13 +7,47 @@ def test_attachments_flags(bug_factory): ) bug_factory.build( with_attachment=True, - attachment__flags=[ - {"id": None, "name": "approval-mozilla-beta", "value": "?"} - ], + attachment__flags=[{"id": None, "name": "approval-mozilla-beta", "value": "?"}], ) # not raising is a success +def test_pull_request_attachment_returns_valid_github_url(bug_factory): + bug = bug_factory.build( + with_attachment=True, + attachment__is_pull_request=True, + attachment__file_name="github-53-url.txt", + attachment__content_type="text/x-github-pull-request", + ) + base_url = "https://github.com/mozilla-firefox/firefox/pull" + assert ( + bug.attachment.github_url(base_url=base_url) + == "https://github.com/mozilla-firefox/firefox/pull/53" + ) + + +def test_non_pull_request_attachment_returns_no_github_url(bug_factory): + bug = bug_factory.build( + with_attachment=True, + attachment__file_name="screenshot.png", + attachment__content_type="image/png", + ) + base_url = "https://github.com/mozilla-firefox/firefox/pull" + assert bug.attachment.github_url(base_url=base_url) is None + + +def test_pull_request_attachment_with_unexpected_file_name_returns_no_github_url( + bug_factory, +): + bug = bug_factory.build( + with_attachment=True, + attachment__file_name="github-no-number-url.txt", # github-53-url.txt + attachment__content_type="text/x-github-pull-request", + ) + base_url = "https://github.com/mozilla-firefox/firefox/pull" + assert bug.attachment.github_url(base_url=base_url) is None + + def test_patch_attachment_returns_valid_phabricator_url(bug_factory): bug = bug_factory.build( with_attachment=True, diff --git a/tests/unit/test_steps.py b/tests/unit/test_steps.py index 73968aef..cf1f9a98 100644 --- a/tests/unit/test_steps.py +++ b/tests/unit/test_steps.py @@ -26,6 +26,7 @@ "maybe_update_issue_status", "maybe_update_issue_resolution", "maybe_add_phabricator_link", + "maybe_add_github_link", ], "comment": [ "create_comment", @@ -250,6 +251,36 @@ def test_added_attachment( ) +def test_added_github_attachment( + action_context_factory, mocked_jira, action_params_factory +): + github_attachment_context = action_context_factory( + operation=Operation.ATTACHMENT, + bug__with_attachment=True, + bug__id=5555, + bug__attachment__is_patch=True, + bug__attachment__is_obsolete=False, + bug__attachment__id=1981576, + bug__attachment__file_name="github-53-url.txt", + bug__attachment__description="[mozilla-firefox/firefox] Bug 1981576 - Permanently make exceptions for certificate errors for normal tabs (#53)", + bug__attachment__content_type="text/x-github-pull-request", + event__target="attachment", + jira__issue="JBI-234", + ) + callable_object = Executor( + action_params_factory(jira_project_key=github_attachment_context.jira.project) + ) + + callable_object(context=github_attachment_context) + + mocked_jira.create_or_update_issue_remote_links.assert_called_once_with( + issue_key="JBI-234", + link_url="https://github.com/mozilla-firefox/firefox/pull/53", + title="[mozilla-firefox/firefox] Bug 1981576 - Permanently make exceptions for certificate errors for normal tabs (#53)", + global_id="5555-123456", + ) + + def test_added_phabricator_attachment( action_context_factory, mocked_jira, action_params_factory ): @@ -1403,13 +1434,16 @@ def test_maybe_update_components_create_components_prefix_component( jira_components=JiraComponents( create_components=True, use_bug_component_with_product_prefix=True, - use_bug_component=False + use_bug_component=False, ), ) mocked_jira.get_project_components.return_value = [ {"id": 1, "name": "Firefox::ExistingComponent"}, ] - mocked_jira.create_component.return_value = {"id": "42", "name": "Firefox::NewComponent"} + mocked_jira.create_component.return_value = { + "id": "42", + "name": "Firefox::NewComponent", + } steps.maybe_update_components( action_context, @@ -1425,6 +1459,7 @@ def test_maybe_update_components_create_components_prefix_component( fields={"components": [{"id": "42"}]}, ) + def test_sync_whiteboard_labels( action_context_factory, mocked_jira,