Skip to content

fix(graphql)!: standardize pagination on a single-level shape#723

Merged
lakhansamani merged 2 commits into
mainfrom
fix/pagination-schema-standardization
Jul 23, 2026
Merged

fix(graphql)!: standardize pagination on a single-level shape#723
lakhansamani merged 2 commits into
mainfrom
fix/pagination-schema-standardization

Conversation

@lakhansamani

Copy link
Copy Markdown
Contributor

Summary

  • 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 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 (see fix(dashboard): send double-nested pagination in OrgDomains' list query #721) 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

Note: 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-go clean
  • Full go test ./internal/... (SQLite) clean, no regressions
  • New TestGetPagination unit test covering nil/empty/explicit-values cases
  • web/dashboard: npm run build clean, full 43-test vitest suite passes (including the updated OrgDomains.test.tsx regression test asserting the correct single-nested shape)
  • Reviewed by an independent pass (opus-tier) verifying the proto-layer claim, a repo-wide grep for zero remaining PaginatedRequest references, every Go call site's value-flow, and both distinct dashboard fix shapes (5 endpoints unwrap one level; Webhooks/EmailTemplates drop the wrapper key entirely) — approved, zero findings

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.
@lakhansamani
lakhansamani merged commit 9c0ecac into main Jul 23, 2026
4 checks passed
@lakhansamani
lakhansamani deleted the fix/pagination-schema-standardization branch July 23, 2026 22:23
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.

1 participant