Skip to content

Conversation

javirln
Copy link
Member

@javirln javirln commented Jul 10, 2025

This patch eagerly loads the organization of the membership, so it's available and can be used to check roles in the service and business layers.

Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
@javirln javirln requested review from migmartri and jiparis July 10, 2025 09:16
@javirln javirln self-assigned this Jul 10, 2025
Copy link
Member

@jiparis jiparis left a comment

Choose a reason for hiding this comment

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

Thanks.
I'd rather just get the Organization ID from the foreign key instead of an additional query. But accepting since this fixes the issue.

@javirln
Copy link
Member Author

javirln commented Jul 10, 2025

Thanks. I'd rather just get the Organization ID from the foreign key instead of an additional query. But accepting since this fixes the issue.

That is how we are doing it on the direct roles of the users. We can do instead:

diff --git a/app/controlplane/pkg/data/membership.go b/app/controlplane/pkg/data/membership.go
index ef78cec0..aa89b7ca 100644
--- a/app/controlplane/pkg/data/membership.go
+++ b/app/controlplane/pkg/data/membership.go
@@ -284,7 +284,9 @@ func (r *MembershipRepo) ListGroupMembershipsByUser(ctx context.Context, userID
 		groupRoleMemberships, err := r.data.DB.Membership.Query().Where(
 			membership.MembershipTypeEQ(authz.MembershipTypeGroup),
 			membership.MemberIDIn(groupIDs...),
-		).WithOrganization().All(ctx)
+		).WithOrganization(func(q *ent.OrganizationQuery) {
+			q.Select(organization.FieldID)
+		}).All(ctx)
 
 		if err != nil {
 			return nil, fmt.Errorf("failed to query group role memberships: %w", err)

But that will break the mapping for the rest of the queries

@javirln javirln merged commit 9bfdcd7 into chainloop-dev:main Jul 10, 2025
13 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