-
Notifications
You must be signed in to change notification settings - Fork 672
SMQ-2704 - Update listing of group hierarchy #2710
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2710 +/- ##
==========================================
- Coverage 42.13% 41.48% -0.65%
==========================================
Files 351 24 -327
Lines 48109 5093 -43016
==========================================
- Hits 20269 2113 -18156
+ Misses 25631 2814 -22817
+ Partials 2209 166 -2043 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
deb1e34
to
19abcf6
Compare
19abcf6
to
057f724
Compare
Signed-off-by: Felix Gateru <felix.gateru@gmail.com>
057f724
to
f1a7a63
Compare
return HierarchyPage{}, errors.Wrap(svcerr.ErrViewEntity, err) | ||
} | ||
hids := svc.getGroupIDs(hp.Groups) | ||
ids, err := svc.filterAllowedGroupIDsOfUserID(ctx, session.DomainUserID, "read_permission", hids) | ||
ids, err := svc.filterAllowedGroupIDsOfUserID(ctx, session.DomainID, session.UserID, hids) | ||
if err != nil { | ||
return HierarchyPage{}, errors.Wrap(svcerr.ErrViewEntity, err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use different approach, This is legacy way which is inefficient and will not scale.
We should follow the approach used for listing groups.
I will come up with SQL, or idea to approach this..
What type of PR is this?
This is a refactor as it updates the list groups hierarchy fucntionality to follow the new listing methodology
What does this do?
This pr refactors listing groups for a user from SpiceDB to the groups repo method.
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Yes, I have updated tests for my changes.
Did you document any new/modified feature?
No
Notes
None