- 
                Notifications
    
You must be signed in to change notification settings  - Fork 61
 
[SCIM 4.1/4] List users and groups in a single query #9325
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
6f4977f    to
    1c5ddcd      
    Compare
  
    | 
               | 
          ||
| async move { self.list_users_in_txn(&conn, err, filter).await } | ||
| }) | ||
| .list_users_with_groups(&conn, err.clone(), filter) | 
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.
hmm I think doing this in a txn is load bearing, is that right @jmpesp ?
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.
I thought it was only because there were 50 queries in there. It can easily be put back.
This test verifies that listing groups via the SCIM API correctly includes member information for each group, including groups with multiple members, groups with a single member, and groups with no members. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Rewrites list_groups_in_txn as list_groups_with_members to use a single query that fetches both groups and their members in one database roundtrip. This uses a LEFT JOIN to the silo_group_membership table and aggregates the results in application code, similar to the list_users_with_groups implementation. The transaction wrapper is removed since it's no longer needed - the function now performs a single read-only query. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1c5ddcd    to
    fc8cd1d      
    Compare
  
    
Added integration test for existing implementation, change implementation, test continues to pass. Let's definitely give the logic a close look, I did not exactly write it.
Assembling the users and groups into the final shape is done in Rust code. I don't know if there's any way around that.