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

Sync external references to Jira tracker links #865

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

dmonzonis
Copy link
Contributor

Before this commit, when filing Jira trackers, external/upstream references were created as links in the Jira issue, but this was only done during tracker creation, and no further changes in the references on OSIDB side were reflected in already created trackers.

With this commit, changes to references on OSIDB will create/update links in existing Jira trackers. Links are kept unique by their URL, so if a reference with a given URL changes its description, it will change the Jira link description as well, but if the URL changes, it will create a new Jira link.

Closes OSIDB-3733.

@dmonzonis dmonzonis self-assigned this Dec 24, 2024
@dmonzonis dmonzonis force-pushed the tracker-sync-references branch from 0169990 to fa7f15e Compare December 24, 2024 14:20
@dmonzonis dmonzonis requested a review from a team December 27, 2024 13:44
Copy link
Contributor

@osoukup osoukup left a comment

Choose a reason for hiding this comment

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

LGTM

if not tracker.is_closed and tracker.type == Tracker.TrackerType.JIRA:
tracker.sync_reference(
jira_token=self.get_jira_token(), reference=new_ref
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not want to over-complicate this right now but I am not convinced that the serializer module is a good place for this on-save logic and we should rather continuously migrate it to the model layer. I know that we already backed a lot of it into the serializer for various reasons in the past and it is not always trivial to change so I am just leaving a comment and not insisting on the change.

Copy link
Contributor

@JakubFrejlach JakubFrejlach left a comment

Choose a reason for hiding this comment

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

I agree with @osoukup regarding putting things into serializers, but won't insist on changing that right now.

Test that service is able to update remote links in Jira issues.
"""
url = "https://www.redhat.com"
flaw = FlawFactory(embargoed=False, uuid="c581c407-5bc3-4a84-83ce-e3688e8fe87c")
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for directly specifying the uuid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No real reason other than it being done in the rest of the tests in that module. But you're right it's not really necessary so might remove them all

@@ -220,6 +226,58 @@ def add_link(self, issue_key, url, title) -> Response:
)
return Response(data=response_content, status=e.status_code)

def update_link(self, issue_key, link_id, url, title) -> Response:
"""Update a remote link to a task by its internal ID."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Tracker instead of task? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or even better, the more generic Jira issue since this is not limited to trackers but can be used for a flaw's task as well.

@dmonzonis dmonzonis force-pushed the tracker-sync-references branch from fa7f15e to 8168bce Compare January 3, 2025 11:38
@dmonzonis
Copy link
Contributor Author

@osoukup @JakubFrejlach I moved the logic to the model layer since in this case it was easy, if you want to re-review

@osoukup
Copy link
Contributor

osoukup commented Jan 3, 2025

@osoukup @JakubFrejlach I moved the logic to the model layer since in this case it was easy, if you want to re-review

I would consider the eventual end-goal that serializers do not do more than just serialization while there is still a bit more being done in the overridden create/update but this is definitely closer to that.

@dmonzonis dmonzonis force-pushed the tracker-sync-references branch 3 times, most recently from 1303bee to 4e88dcf Compare January 7, 2025 09:47
Update the jira package in the requirements to be able to use some of
its newer functionalities, namely the ability to delete remote links
with a dedicated method instead of having to send the http query
directly.
Before this commit, when filing Jira trackers, external/upstream
references were created as links in the Jira issue, but this was only
done during tracker creation, and no further changes in the references
on OSIDB side were reflected in already created trackers.

With this commit, changes to references on OSIDB will create/update
links in existing Jira trackers. Links are kept unique by their URL, so
if a reference with a given URL changes its description, it will change
the Jira link description as well, but if the URL changes, it will
create a new Jira link.

Closes OSIDB-3733.
@dmonzonis dmonzonis force-pushed the tracker-sync-references branch from 4e88dcf to d7682c4 Compare January 8, 2025 11:32
@dmonzonis dmonzonis added this pull request to the merge queue Jan 8, 2025
Merged via the queue into master with commit 0005c87 Jan 8, 2025
11 checks passed
@dmonzonis dmonzonis deleted the tracker-sync-references branch January 8, 2025 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants