Migrate storage mappings table to GraphQL - #2031
Conversation
✅ Code HealthRemoves 48 Unused files
66 Unused exports
29 Unused exported types
|
Replace the PostgREST-backed storage mappings settings table with a cursor-paginated GraphQL query (usePaginatedStorageMappings). - Drop the getStorageMappings PostgREST helper and the EntityTable / TableHydrator / selectable-table Zustand plumbing it required. - Render a plain MUI Table with TablePagination driven by useCursorPagination; forward pagination only, no search or column sorting (the storageMappings query supports neither). - Retype rows to StorageMappingTableRow (catalogPrefix / spec) and remove the "Last Updated" column, which the query does not expose. - Invalidate the storageMappings query in the URQL cache on createStorageMapping / updateStorageMapping, replacing the manual useStorageMappingsRefresh mechanism. - Add the storageMappingsTable.error.loadFailed message.
2ebe14d to
bbfa3fb
Compare
The settings table paginates by cursor, and it took `underPrefix` from
`selectedTenant`. The page-level `TenantSelector` can change that tenant while
the table stays mounted, because `Settings` renders the two as siblings and
passes no `key`. A tenant switch therefore sent the old tenant's `after` cursor
against the new prefix. That skips pages, or returns zero edges. A zero-edge
response also hides the pagination footer, so the user loses the Previous
button.
Remove the `by: { underPrefix }` argument and the `pause` gate. The table now
lists every storage mapping the caller can read. The server applies the
caller's catalog-read scope on its own, so an absent filter cannot widen
visibility.
`useStorageMappings`, which feeds the create and update dialogs, keeps its
`underPrefix` scoping.
The pagination footer renders only when the current page has rows, so a zero-row page removes the Previous control and leaves the user with no way to reach populated rows. A page can come back empty when the mapping list shrinks between fetches, or when a cursor points past the end of the list. Watch for a completed fetch that returns no rows above page 0, then call `goToPage(currentPage - 1)`. The `currentPage > 0` guard ends the walk back at the first page. This matches the recovery in `AccessLinksTable` and `RefreshTokenTable`, which share `useCursorPagination`.
`cursorHistory[i]` holds the cursor that starts page `i + 1`, and a forward step appended to the list. Forward navigation after a backwards step therefore revisited a page that already had an entry, and appended a second one. Every later entry then sat one position too high, so a backwards step read the cursor of an earlier page while the label showed the page the user asked for. The sequence `1 -> 2 -> 3 -> 2 -> 3 -> 4 -> 5 -> 4` reaches it: the final page 4 loaded the rows of page 3. Write `nextCursor` at index `page - 1` and drop every entry past it, so the history describes the path to the page now on screen. Four tables share this hook: storage mappings, access links, refresh tokens, and alert history. Add tests for the first page, a forward step, a return to the first page, a forward step carrying no cursor, and the backtracking sequence above.
Tenant scoping is moving into the auth token, so each query no longer carries
its own prefix filter. The server applies the caller's catalog-read scope on
its own, and the schema states that a filter can only narrow that scope, so an
absent filter cannot widen what a caller sees.
Remove `by: { underPrefix }` from `StorageMappingQuery`, along with the
`selectedTenant` reads in `useStorageMappings` and `useStorageMappingService`.
This also removes the last use of the deprecated `by` argument in this file.
The change repairs the edit dialog. The settings table lists every mapping the
caller can read, and a row click asks `UpdateMappingWizard` to resolve that
catalog prefix through this query. A tenant-scoped query held no row from
another prefix, so the dialog took its `notFound` path and closed itself.
`refetchMappings` now runs for every create and update. It previously returned
early whenever no tenant was selected.
Interim note: `PrefixCard` merges these prefixes with `useLiveSpecs`, which is
still scoped to the selected tenant, so its autocomplete draws the two sides
from different scopes until the token change lands.
ccdfb89 to
b6c271a
Compare
…ash, stale e2e assertion - useStorageMappings now walks every page via useAllPages instead of taking the server's silent 50-row default page, so the edit dialog can resolve any clicked row and PrefixCard's duplicate/coverage validators see the complete list. A dedupe by catalogPrefix guards against a graphcache invalidation re-appending the refetched page. - Gate the edit dialog's notFound self-close on the storage mappings loading flag (deep-link race) and on query errors (notFound previously closed the dialog before the error state could render). - Drop refetchMappings from create/update: graphcache already invalidates every storageMappings query on those mutations, and a first-page-only network refetch is misleading now that the query paginates. - Guard row.spec.data_planes in the settings table and type it optional: the server omits the field from the spec JSON when the list is empty (serde skip_serializing_if), which crashed the whole Settings section on such a mapping. - Remove the Playwright assertion on the removed "Last Updated" column. The GraphQL StorageMapping type carries no timestamp field, so the column cannot come back client-side. - gql-types regenerated for the query change only; the DataPlane.id/IdFilter schema drift stays in #2064.
SeanWhelan
left a comment
There was a problem hiding this comment.
Approving. 2 cosmetic things, not blocking:
storageMappingsTable.filterLabeland.message2are orphaned in
AdminPage.tsnow that the old table is gonedataPlane.replace('ops/dp/', '')duplicatesDATA_PLANE_PREFIXfrom
src/settings/dataPlanes.ts
Migrate storage mappings table on the settings page to use the gql list query, and add pagination