Skip to content
Draft
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
1 change: 1 addition & 0 deletions docs/actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
31 changes: 31 additions & 0 deletions jbi/bugzilla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just realized this now, after checking out what the attachment looks like on this bug - https://bugzilla.mozilla.org/show_bug.cgi?id=2014529

It seems that we may need to account for other GitHub projects too. And I think we need to infer the GitHub repository from the description of the attachment.

Image

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point, I didn't consider that this would be used on multiple repositories. Will make an update!


def is_phabricator_patch(self) -> bool:
"""
Returns True if this attachment is a phabricator patch attachment.
Expand Down
3 changes: 3 additions & 0 deletions jbi/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
3 changes: 3 additions & 0 deletions jbi/jira/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions jbi/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class ActionSteps(BaseModel, frozen=True):
attachment: list[str] = [
"create_comment",
"maybe_add_phabricator_link",
"maybe_add_github_link",
]

@field_validator("*")
Expand Down
47 changes: 47 additions & 0 deletions jbi/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
*,
Expand Down
40 changes: 37 additions & 3 deletions tests/unit/bugzilla/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
39 changes: 37 additions & 2 deletions tests/unit/test_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"maybe_update_issue_status",
"maybe_update_issue_resolution",
"maybe_add_phabricator_link",
"maybe_add_github_link",
],
"comment": [
"create_comment",
Expand Down Expand Up @@ -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
):
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down