Update DataPlaneTable in admin view to use GQL instead of Supabase - #2067
Update DataPlaneTable in admin view to use GQL instead of Supabase#2067adrian-estuary wants to merge 13 commits into
Conversation
⚪ Code HealthNo change to the dead-code surface. 48 Unused files
66 Unused exports
30 Unused exported types
14 Unused exported enum members
5 Unused dependencies
3 Unused devDependencies
|
| // The region is parsed out of the data plane name rather than read off the | ||
| // `region` field so it stays in sync with the displayed name. They differ | ||
| // for the local data plane: `ops/dp/public/local-flow` parses to `flow`, | ||
| // while the field reports `local`. | ||
| const { region } = parseDataPlaneName(dataPlane.name, scope); |
There was a problem hiding this comment.
Not sure if this is just a local environment quirk and if this logic is really needed for production. Thoughts?
There was a problem hiding this comment.
I'm not following here - region is in fact parsed from the name in the gql resolver so i wouldn't expect this logic to change anything in the UI.
I sometimes wonder if we could just get rid of the "local" region and call it GCP or whatever, would simplify a bunch of little annoying things.
| <ToggleDataPlaneScope | ||
| scope={dataPlaneScope} | ||
| onChange={handleDataPlaneScopeChange} | ||
| /> |
There was a problem hiding this comment.
Updated the ToggleDataPlaneScope component to take in the scope and handle changes via a callback. There was some wonky interaction with it directly updating the context that caused a render loop.
c03a576 to
942ace8
Compare
|
left a few low-conviction comments so far - i'll get to the stuff that actually matters with fresh eyes tomorrow. |
53680d0 to
2a5bbf4
Compare
b6d1284 to
5227bb8
Compare
Update useDataPlanes.ts
- remove zustand store interaction - remove intl
- memoize data to fix render loop - remove toggle callback in favor of a set callback
5227bb8 to
140e0b0
Compare
| const { rowsPerPage, setRowsPerPage } = useTableState( | ||
| TablePrefixes.dataPlanes, | ||
| 'name', | ||
| 'asc' | ||
| ); |
There was a problem hiding this comment.
we can go even further and delete this (does stuff like stores the rows-per-page selection in the url, like admin/settings?dpt-r=25 IMO not necessary)
There was a problem hiding this comment.
and just declare const PAGE_SIZE = 10 to replace rowsPerPage
| count={-1} | ||
| page={currentPage} | ||
| rowsPerPage={rowsPerPage} | ||
| onPageChange={handlePageChange} |
There was a problem hiding this comment.
| onPageChange={handlePageChange} | |
| rowsPerPageOptions={[PAGE_SIZE]} // hides the rows-per-page selector (which i wish was just off by default - who actually ever touches that? | |
| onPageChange={handlePageChange} |
| const slotProps = useMemo(() => { | ||
| const previousButtonDisabled = !pageInfo?.hasPreviousPage; | ||
| const nextButtonDisabled = !pageInfo?.hasNextPage; | ||
| return { | ||
| actions: { | ||
| previousButton: { | ||
| disabled: previousButtonDisabled, | ||
| }, | ||
| nextButton: { | ||
| disabled: nextButtonDisabled, | ||
| }, | ||
| }, | ||
| }; | ||
| }, [pageInfo?.hasNextPage, pageInfo?.hasPreviousPage]); |
There was a problem hiding this comment.
the <TablePagination /> component isn't memoized, so stabilizing slotProps (and many of its siblings) does nothing. Could probably inline some of them in the component props
| const slotProps = useMemo(() => { | |
| const previousButtonDisabled = !pageInfo?.hasPreviousPage; | |
| const nextButtonDisabled = !pageInfo?.hasNextPage; | |
| return { | |
| actions: { | |
| previousButton: { | |
| disabled: previousButtonDisabled, | |
| }, | |
| nextButton: { | |
| disabled: nextButtonDisabled, | |
| }, | |
| }, | |
| }; | |
| }, [pageInfo?.hasNextPage, pageInfo?.hasPreviousPage]); | |
| const slotProps = { | |
| actions: { | |
| previousButton: { | |
| disabled: !pageInfo?.hasPreviousPage, | |
| }, | |
| nextButton: { | |
| disabled: !pageInfo?.hasNextPage, | |
| }, | |
| }, | |
| }; |
Issues
#2065
Changes
Requires landing control-plane-api: Add public/private filter option to DataPlanes query flow#3403 first, as it utilizes the newDone!publicfilter.tenantfilter.DataPlanesTablecomponent to use the GQL hook.ToggleDataPlaneScopeto simplify scope selection.EnityTablecomponent.DataPlanesTable.Tests
Test plan
npm run formatnpm run typechecknpm run lintnpm run test(310 passed)Screenshots
Public data planes (default 1 local plane)

Private data planes (none in local)

Many data planes - paginated (seeded data)

Private data planes (only 1 page, no pagination controls show up)

Dialog

GQL query in network tab

If applicable - please include some screenshots of the new UI