fix(graphql)!: standardize pagination on a single-level shape#723
Merged
Conversation
BREAKING CHANGE: 6 List*Request input types (ListClients, ListTrustedIssuers,
ListSAMLServiceProviders, ListOrganizations, ListOrgDomains, ListOrgMembers)
declared their pagination field as PaginatedRequest, a wrapper type whose
only field is itself named `pagination: PaginationRequest` - forcing every
caller to double-nest (`pagination: { pagination: { limit, page } }`).
3 more operations (_verification_requests, _webhooks, _email_templates)
took PaginatedRequest as their entire top-level params type.
This wrapper never existed on the canonical proto/gRPC surface
(proto/authorizer/v1/pagination.proto and admin.proto - every List RPC
takes PaginationRequest directly, no wrapper), and 4 other GraphQL-only
request types (ListUsers, UserOrganizations, ListWebhookLog, ListAuditLog)
already embedded PaginationRequest directly. The double-nesting was a
GraphQL-only accident, not an intentional API design - and it was
actively broken: OrgDomains.tsx (this repo's own dashboard) sent the
single-nested shape and got GRAPHQL_VALIDATION_FAILED on every call
until a prior fix matched the wrapper, and authorizer-js's TypeScript
types for all 6 List*Request types were already (accidentally) typed as
single-nested PaginationRequest, meaning no strictly-typed SDK consumer
could construct the double-nested shape the schema required either.
Removes the PaginatedRequest type entirely; the 9 affected fields/params
now declare PaginationRequest directly, matching every other GraphQL
endpoint and the proto surface. internal/utils/pagination.go's shared
GetPagination helper now takes *model.PaginationRequest directly,
eliminating the wrap-then-call boilerplate every "already single-nested"
caller needed to reuse it. The gRPC handler layer's duplicate
modelPaginatedRequest/modelPaginationRequest converters collapse into one.
Every consumer in this repo (internal/service, internal/graphql,
internal/grpcsrv/handlers, web/dashboard) is updated to match.
Follows the schema fix (previous commit): Clients.tsx, TrustedIssuers.tsx,
Organizations.tsx, and OrganizationDetail.tsx's org-members fetch drop
their double-nested `pagination: { pagination: {...} } }` calls down to
single-level. OrgDomains.tsx reverts the double-nesting patch from an
earlier fix now that the schema itself no longer requires it, and keeps
the res.error handling that fix added (matching every other mutation
handler in this file - the query error was previously silently
swallowed). Webhooks.tsx and EmailTemplates.tsx drop the `pagination:`
wrapper key entirely, since _webhooks/_email_templates now take
PaginationRequest directly as their whole params type instead of the
removed PaginatedRequest wrapper.
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.
Summary
List*Requestinput types (ListClients,ListTrustedIssuers,ListSAMLServiceProviders,ListOrganizations,ListOrgDomains,ListOrgMembers) declared theirpaginationfield asPaginatedRequest, a wrapper type whose only field is itself namedpagination: PaginationRequest— forcing every caller to double-nest (pagination: { pagination: { limit, page } })_verification_requests,_webhooks,_email_templates) tookPaginatedRequestas their entire top-levelparamstypeproto/authorizer/v1/pagination.protoandadmin.proto— every List RPC takesPaginationRequestdirectly, no wrapper), and 4 other GraphQL-only request types (ListUsers,UserOrganizations,ListWebhookLog,ListAuditLog) already embeddedPaginationRequestdirectlyOrgDomains.tsx(this repo's own dashboard) sent the single-nested shape and gotGRAPHQL_VALIDATION_FAILEDon every call (see fix(dashboard): send double-nested pagination in OrgDomains' list query #721) until a prior fix matched the wrapper, andauthorizer-js's TypeScript types for all 6List*Requesttypes were already (accidentally) typed as single-nestedPaginationRequest, meaning no strictly-typed SDK consumer could construct the double-nested shape the schema required eitherPaginatedRequesttype entirely; the 9 affected fields/params now declarePaginationRequestdirectly, matching every other GraphQL endpoint and the proto surfaceinternal/utils/pagination.go's sharedGetPaginationhelper now takes*model.PaginationRequestdirectly, eliminating the wrap-then-call boilerplate every "already single-nested" caller needed to reuse itmodelPaginatedRequest/modelPaginationRequestconverters collapse into oneinternal/service,internal/graphql,internal/grpcsrv/handlers,web/dashboard) is updated to matchNote: this branch includes
OrgDomains.tsx's pagination-nesting fix and error handling inline (originally #721, still open) since both PRs touch the same lines — whichever merges first, the other will show as a no-op on that overlap.Coordinated SDK fixes (each needs releasing alongside this): authorizer-js#50, authorizer-go#22, authorizer-py#7.
Test plan
go build ./.../go vet ./.../make lint-gocleango test ./internal/...(SQLite) clean, no regressionsTestGetPaginationunit test covering nil/empty/explicit-values casesweb/dashboard:npm run buildclean, full 43-testvitestsuite passes (including the updatedOrgDomains.test.tsxregression test asserting the correct single-nested shape)PaginatedRequestreferences, every Go call site's value-flow, and both distinct dashboard fix shapes (5 endpoints unwrap one level;Webhooks/EmailTemplatesdrop the wrapper key entirely) — approved, zero findings