refactor: Added unpaginated list methods to the Organizations plugin. - #108
Merged
Merged
Conversation
List methods returned a page, not a collection, so fetching every member of an organization meant looping until pagination.has_more went false — N round-trips and N SELECT COUNT(*) queries for data one unbounded query returns. The plugin API had no way to ask for a whole collection at all. Every list method now comes as a pair, and the names say which is which: - ListAll… takes pagination.Params and returns the List…Response envelope. These are the existing methods, renamed. - GetAll… takes no pagination and returns a plain slice. These are new, and reuse the names the paginated methods used to occupy. Because GetAll… keeps its old name against a new signature, every call site that was not updated is a compile error rather than a silent behaviour change. The pairs exist at the repository, service and plugin API layers. HTTP routes, query parameters and response bodies are unchanged; handlers, their structs and the usecase layer are renamed only. - Organizations get both an actor-scoped GetAllOrganizations and an unfiltered GetAllOrganizationsUnscoped, backed by new OrganizationRepository.GetAll. - Invitations get pairs for both collections they expose: by organization, and pending by email. - Unconstrained queries reuse the same predicates, joins, row mappers and ORDER BY as their paginated twins, so both return rows in the same order. The raw-SQL joins also skip the SELECT COUNT(*), making them a single round-trip where the paginated path takes two, and closing the torn-read window between the count and the page. GetAllPendingByEmail drops the MaxPendingInvitationsPerBatch = 500 cap, which fixes a bug in AcceptPendingOrganizationInvitationsForEmail. On saturation it logged "remaining invitations will be accepted on the next call" and returned, but nothing re-invokes it, so a user with more than 500 pending invitations silently never had the rest accepted. Note that acceptOrganizationInvitations still processes the batch in a single transaction, which is now unbounded. Corrects the five list endpoint descriptions in the OpenAPI spec, which have documented a "hard maximum of 100" on limit since 8f08c48 removed MaxLimit, and regenerates openapi.json. Tests cover ordering, parent scoping, empty-slice-not-nil, relation hydration, repository error propagation and each authorization failure path, plus SQL-backed cases proving the unconstrained methods ignore pagination.DefaultLimit and that more than 500 pending invitations are all accepted. Known gap, pre-existing and unchanged: api.go bypasses the usecase layer where authorizeOrgAccess lives. The member, team and team-member services authorize themselves, but GetAllInvitations checks only for a non-empty organization ID, so a caller holding *API can read invitations for any organization. This change makes that cheaper to reach rather than newly possible. GetAllOrganizationsUnscoped is documented as administrative-only but is not enforced. BREAKING CHANGE: the Organizations plugin's exported Go API is renamed. On *API, the service interfaces and the repository interfaces, paginated GetAll… methods are now ListAll…, and the GetAll… names now belong to unpaginated methods returning a plain slice. Embedders calling these must rename their call sites; a call left unchanged will fail to compile rather than silently return a single page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
List methods returned a page, not a collection, so fetching every member of an organization meant looping until pagination.has_more went false — N round-trips and N SELECT COUNT(*) queries for data one unbounded query returns. The plugin API had no way to ask for a whole collection at all.
Every list method now comes as a pair, and the names say which is which:
Because GetAll… keeps its old name against a new signature, every call site that was not updated is a compile error rather than a silent behaviour change.
The pairs exist at the repository, service and plugin API layers. HTTP routes, query parameters and response bodies are unchanged; handlers, their structs and the usecase layer are renamed only.
GetAllPendingByEmail drops the MaxPendingInvitationsPerBatch = 500 cap, which fixes a bug in AcceptPendingOrganizationInvitationsForEmail. On saturation it logged "remaining invitations will be accepted on the next call" and returned, but nothing re-invokes it, so a user with more than 500 pending invitations silently never had the rest accepted. Note that acceptOrganizationInvitations still processes the batch in a single transaction, which is now unbounded.
Corrects the five list endpoint descriptions in the OpenAPI spec, which have documented a "hard maximum of 100" on limit since 8f08c48 removed MaxLimit, and regenerates openapi.json.
Tests cover ordering, parent scoping, empty-slice-not-nil, relation hydration, repository error propagation and each authorization failure path, plus SQL-backed cases proving the unconstrained methods ignore pagination.DefaultLimit and that more than 500 pending invitations are all accepted.
Known gap, pre-existing and unchanged: api.go bypasses the usecase layer where authorizeOrgAccess lives. The member, team and team-member services authorize themselves, but GetAllInvitations checks only for a non-empty organization ID, so a caller holding *API can read invitations for any organization. This change makes that cheaper to reach rather than newly possible. GetAllOrganizationsUnscoped is documented as administrative-only but is not enforced.
BREAKING CHANGE: the Organizations plugin's exported Go API is renamed. On *API, the service interfaces and the repository interfaces, paginated GetAll… methods are now ListAll…, and the GetAll… names now belong to unpaginated methods returning a plain slice. Embedders calling these must rename their call sites; a call left unchanged will fail to compile rather than silently return a single page.