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

Portfolio:Initiative Membership Backend #89

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

gbdubs
Copy link
Contributor

@gbdubs gbdubs commented Dec 19, 2023

Also: refactors some naming conventions to make memberships more consistently named.

NOTE - this PR demonstrates the two strategies we'll use for relational object population. On high-cardinality relationships (i.e. Portfolios that are members of an Initiative) we'll populate them only on single-object retrievals, using the get relationships type DB methods. On low cardinality relationships (i.e. Initiatives that are attached to a portfolio), we'll populate the membership as an empty shell object on the entity, and use the populateX resolver helpers to populate the full thing. See that in this PR, despite the relationship being the same one, we use one strategy when looking up an initiative (which may have tens of thousands of portfolios) vs looking up a portfolio (which may have 1-2 initiatives). This allows us to do an appropriate amount of data retrieval in each context.

Hope that makes sense!

@gbdubs gbdubs requested a review from bcspragu December 19, 2023 21:06
Copy link
Collaborator

@bcspragu bcspragu left a comment

Choose a reason for hiding this comment

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

LGTM!

cmd/server/pactasrv/conv/pacta_to_oapi.go Outdated Show resolved Hide resolved
@gbdubs gbdubs enabled auto-merge (squash) December 19, 2023 21:54
@gbdubs gbdubs merged commit dc1f699 into main Dec 19, 2023
2 checks passed
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.

2 participants