Skip to content

Add find_all_with to batch Group members lookup and eliminate N+1 in PATCH#164

Merged
pond merged 2 commits into
pond:mainfrom
shuyahonda:feature/fix-n+1-query-in-group-patch
Sep 12, 2025
Merged

Add find_all_with to batch Group members lookup and eliminate N+1 in PATCH#164
pond merged 2 commits into
pond:mainfrom
shuyahonda:feature/fix-n+1-query-in-group-patch

Conversation

@shuyahonda
Copy link
Copy Markdown
Contributor

@shuyahonda shuyahonda commented Sep 8, 2025

Addresses issue #157: Resolve N+1 queries on Group PATCH when resolving members (users)

Approach

  • Batch member resolution via find_all_with: Add a batch-lookup hook to dynamic list mappings that is preferred over per-item find_with. This allows fetching all referenced users in one query for Group PATCH.
  • Backward compatible: If find_all_with is not provided, existing find_with continues to be used.
  • Minimal mock for tests: Introduce a dedicated MockBatchGroupsController/Model that uses find_all_with (users-only) to verify the end-to-end behavior without overcomplicating the mock.

Changes

  • Batch support: Prefer :find_all_with over :find_with in dynamic list mappings during from_scim!/from_scim_patch!.
  • Documentation/comments: Updated inline comments to explain find_all_with and precedence.

@shuyahonda shuyahonda changed the title Add find_all_with to batch Group members lookup and eliminate N+1 in PATCH [WIP] Add find_all_with to batch Group members lookup and eliminate N+1 in PATCH Sep 8, 2025
@shuyahonda shuyahonda marked this pull request as ready for review September 8, 2025 06:43
@shuyahonda shuyahonda changed the title [WIP] Add find_all_with to batch Group members lookup and eliminate N+1 in PATCH Add find_all_with to batch Group members lookup and eliminate N+1 in PATCH Sep 8, 2025
Copy link
Copy Markdown
Owner

@pond pond left a comment

Choose a reason for hiding this comment

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

Awesome patch! Thanks so much 😀

@pond pond merged commit 089ac9a into pond:main Sep 12, 2025
6 checks passed
@alexmingoia
Copy link
Copy Markdown

Thank you so much for this. Just saved me a late night on-call 👍

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.

3 participants