fix(dashboard): send double-nested pagination in OrgDomains' list query#721
Closed
lakhansamani wants to merge 1 commit into
Closed
fix(dashboard): send double-nested pagination in OrgDomains' list query#721lakhansamani wants to merge 1 commit into
lakhansamani wants to merge 1 commit into
Conversation
ListOrgDomainsRequest.pagination is PaginatedRequest, which itself
wraps a `pagination: PaginationRequest` field
(internal/graph/schema.graphqls) - fetchDomains sent the single-nested
`pagination: { limit: 100 }`, which the server rejected on every call
with GRAPHQL_VALIDATION_FAILED. The error was silently swallowed
(`res.data?._org_domains?.org_domains || []` never checked res.error),
so the verified-domains table always rendered "No verified domains
yet." even when domains existed - this feature (added in #707) has
never worked since it shipped.
Also adds the missing res.error handling fetchDomains lacked, matching
every other mutation handler in this file, and a regression test
asserting the exact query variable shape sent (the existing test
file's generic client mock never validated variable shape against a
real schema, which is why this went unnoticed by the pre-existing
unit tests).
Found while building an e2e spec to drive this UI through a real
browser and backend.
5 tasks
Contributor
Author
|
Superseded by #723 (pagination schema standardization), which was built directly on top of this fix and includes it inline — both PRs touched the exact same lines in OrgDomains.tsx/OrgDomains.test.tsx. #723 has now merged, so this branch's diff is fully redundant (and now conflicts, since main already contains equivalent content in a different shape). Closing rather than force-resolving conflicts for a no-op. |
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
web/dashboard/src/components/OrgDomains.tsx'sfetchDomains()sentpagination: { limit: 100 }, butListOrgDomainsRequest.paginationisPaginatedRequest, which itself wraps apagination: PaginationRequestfield (internal/graph/schema.graphqls) — the correct shape is double-nested:pagination: { pagination: { limit: 100 } } }_org_domainsquery failed server-side withGRAPHQL_VALIDATION_FAILED— silently swallowed (res.data?._org_domains?.org_domains || []never checkedres.error), so the verified-domains table always rendered "No verified domains yet." even when domains existedres.errorhandlingfetchDomainslacked, matching the exact pattern already used by every other mutation handler in this file (RequestOrgDomain/VerifyOrgDomain/AddVerifiedOrgDomain/DeleteOrgDomain)Found while building an e2e spec that drives this UI through a real browser and backend (not the mocked client the existing unit tests use).
Test plan
npx vitest run src/components/OrgDomains.test.tsx— 6/6 pass (5 pre-existing + 1 new regression test asserting the correct double-nested shape)npx vitest run(web/dashboard) — 43/43 pass, no regressionsnpm run buildclean