chore: remove GetRelationDetailsForUnit #19328
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Aflynn50
force-pushed
the
remove-get-unit-relation-details
branch
from
8e6d9c8 to
a931fd9
Compare
gfouillet
reviewed
Apr 1, 2025
Contributor
gfouillet
left a comment
gfouillet
left a comment
There was a problem hiding this comment.
Looks good overall, yet I have a small concern before approval.
| // - [relationerrors.RelationNotFound] is returned if the relation UUID | ||
| // is not found. | ||
| GetRelationDetails(ctx context.Context, relationID int) (relation.RelationDetailsResult, error) | ||
| GetRelationDetails(ctx context.Context, relationUUID corerelation.UUID) (relation.RelationDetailsResult, error) |
Aflynn50
force-pushed
the
remove-get-unit-relation-details
branch
from
a931fd9 to
d56b40c
Compare
hmlanigan
approved these changes
Apr 1, 2025
Aflynn50
force-pushed
the
remove-get-unit-relation-details
branch
from
d56b40c to
f5b2dad
Compare
Aflynn50
force-pushed
the
remove-get-unit-relation-details
branch
from
f5b2dad to
0f09795
Compare
Contributor
Author
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove the GetRelationDetailsForUnit method. Callsites of this method can be replaced by a call to
GetRelationDetailswhich will return the same contents. This method was added to replace the 3.6 functiongetRelationUnitin the uniter facade which fetched the unit and relation separately. In the callsites replaced in this PR, the unit information was not used anyway, so theGetRelationDetailsfunction can be swapped in.The only hitch was that
GetRelationDetailstook a relation ID rather than a relation UUID, whereasGetRelationDetailsForUnittook a unit UUID and a relation UUID. Since it would take an extra facade call to get the relation ID For this reason I changedGetRelationDetailsto use a UUID instead of an ID. This means that the existing facade calls in the uniter facade now need to resolve their relation ID into a relation UUID. I settled on this solution rather than resolving the UUID into an ID because we are generally moving towards using the UUIDs in the facades.Checklist
QA steps
Unit tests pass
Links
Jira card: JUJU-7701