fix(admin): stop duplicate API calls across admin tables - #1869
fix(admin): stop duplicate API calls across admin tables#1869Shreyag02 wants to merge 27 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds shared server-table query and pagination hooks, moves organization member data into a dedicated query hook, updates query cache freshness and priming, and revises organization billing and invitation loading conditions. ChangesAdmin query and organization data updates
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The PR reduces duplicate admin requests and prevents billing actions from appearing usable when required account data is unavailable. One bounded merge-readiness risk remains: organization data resolved from a slug may refresh a separate ID-based cache entry’s freshness timestamp, potentially delaying visibility of later organization changes; mergeable with explicit owner awareness or follow-up. Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Shreyag02
added
the
Do not merge
Shreyag02
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a49db67e-1c52-40d1-9e9f-dff6cca3a8b0
📒 Files selected for processing (21)
web/apps/admin/src/contexts/ConnectProvider.tsxweb/apps/admin/src/pages/organizations/details/index.tsxweb/sdk/admin/hooks/useOrgMembersMap.tsweb/sdk/admin/hooks/useServerTableQuery.tsweb/sdk/admin/views/admins/columns.tsxweb/sdk/admin/views/audit-logs/index.tsxweb/sdk/admin/views/audit-logs/navbar.tsxweb/sdk/admin/views/audit-logs/util.tsweb/sdk/admin/views/invoices/index.tsxweb/sdk/admin/views/organizations/details/apis/index.tsxweb/sdk/admin/views/organizations/details/contexts/organization-context.tsxweb/sdk/admin/views/organizations/details/index.tsxweb/sdk/admin/views/organizations/details/invoices/index.tsxweb/sdk/admin/views/organizations/details/members/index.tsxweb/sdk/admin/views/organizations/details/pat/index.tsxweb/sdk/admin/views/organizations/details/projects/index.tsxweb/sdk/admin/views/organizations/details/projects/members/index.tsxweb/sdk/admin/views/organizations/details/projects/use-add-project-members.tsxweb/sdk/admin/views/organizations/details/tokens/index.tsxweb/sdk/admin/views/organizations/list/index.tsxweb/sdk/admin/views/users/list/list.tsx
💤 Files with no reviewable changes (2)
- web/sdk/admin/views/audit-logs/util.ts
- web/sdk/admin/views/organizations/details/contexts/organization-context.tsx
| const { organization } = useContext(OrganizationContext); | ||
| const { data: orgMembersMap = {} } = useOrgMembersMap(organization?.id); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include organization member loading in the returned loading state.
If listProjectUsers resolves before useOrgMembersMap, eligibleMembers is empty while isLoading is false. The member picker can display an incorrect empty state.
Proposed fix
- const { data: orgMembersMap = {} } = useOrgMembersMap(organization?.id);
+ const {
+ data: orgMembersMap = {},
+ isLoading: isOrgMembersMapLoading,
+ } = useOrgMembersMap(organization?.id);
...
- isLoading,
+ isLoading: isLoading || isOrgMembersMapLoading,There was a problem hiding this comment.
Real, but not blocking — leaving it for follow-up rather than fixing here.
The window is narrower than it looks. The projects tab calls useOrgMembersMap itself on mount (projects/index.tsx:94), and useAddProjectMembers runs per row from columns.tsx:118, so those observers mount together and share one in-flight fetch. Hitting the wrong empty state needs the dropdown opened inside the gap where listProjectUsers has resolved but listOrganizationUsers has not — so a click, on a cold cache, on a slow connection.
54759942 also gave useOrgMembersMap a staleTime, so anything that has already loaded the projects tab serves from cache and the window closes entirely.
Queuing it on fix/admin-ui-followups instead of growing a PR that is already carrying a refactor.
There was a problem hiding this comment.
@Shreyag02, acknowledged. The shared in-flight query and staleTime reduce the exposure window.
The loading-state gap remains valid on a cold cache. Deferring it keeps this refactor scoped appropriately.
Would you like me to open a GitHub follow-up issue for fix/admin-ui-followups?
You are interacting with an AI system.
Coverage Report for CI Build 32706398253Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.08%) to 48.886%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Every server-mode DataTable fired two requests for its first page.
INITIAL_QUERY carried no sort while defaultSort was passed as a prop.
DataTable seeds its internal query from getDefaultTableQuery(defaultSort,
query) and its mount effect emits unconditionally, since oldQueryRef
starts null. The emitted query therefore differs from the one the parent
already had in state by exactly the sort field.
connect-query builds its cache key with createMessageKey, which omits
unset fields, so sort: [] and sort: [{...}] hash to different keys. The
key changed, a second request went out, and the first was aborted
mid-flight once its observer was dropped.
Seeding the initial sort makes the mount emit structurally identical to
the query already in state, so the key is unchanged and no refetch is
triggered.
The project members dialog passed defaultSort={{ name: "", order: "desc" }},
which sent an RQL sort with an empty field name on every mount and
guaranteed the key change that caused a duplicate request.
The sort was never applied: ProjectUsersRepository.prepareDataQuery builds
its statement from search, offset and limit only, and ignores sort
entirely. No column in this table is sortable either — title sets
enableSorting: false and the rest are unsorted.
Removing the prop leaves both the initial and emitted query at sort: [],
so ordering is unchanged and the mount no longer refetches.
The layout renders a spinner in place of its children while isLoading is true, and isLoading included isBillingAccountLoading. That query is gated on firstBillingAccountId, which arrives from a separate listBillingAccounts call that was not itself in the gate. A disabled query reports isLoading false, so once the org and role queries settled the gate opened, the tab mounted and its tables fetched. When listBillingAccounts then resolved, the billing query enabled, isLoading went true again and the whole tab unmounted, only to remount and refetch once billing settled. Gating only on queries that are enabled from the first render makes the transition monotonic, so the tab mounts once. The side panel already renders its own skeletons while billing resolves.
VirtualizedContent calls loadMoreData() from its scroll handler, guarded only by the isLoading value captured in that render. Scroll events fire per frame, while isFetchingNextPage only becomes true after react-query notifies and React re-renders, so several events can pass the guard for the same page. fetchNextPage defaults to cancelRefetch: true, so each of those calls aborts and restarts the previous one: three calls in a frame issue three requests and advance by a single page. Guard on hasNextPage and isFetchingNextPage at the call site, matching what the members table already does.
The invalidation key was built with an empty input. react-query matches query keys partially, and an empty object matches vacuously, so every cached searchOrganizationUsers entry was invalidated regardless of which org it belonged to. Updating a role in one org refetched the member list of every other org still held in cache. Keying on the org id scopes the match to that org, while leaving `query` unset so its filter and sort variants are still covered.
The QueryClient set only retry and refetchOnWindowFocus, leaving staleTime at its default of 0. Combined with refetchOnMount, every mount of every component refetched, so reference data such as roles, plans and products was re-requested on each navigation. Four views had worked around this locally with staleTime: Infinity, which left the same key refetching or not depending on which page it was reached from. A 30s default covers navigation without holding data long enough to look stale. Mutations invalidate their own keys and the two panels that need immediate freshness call refetch(), which ignores staleTime, so writes are still reflected at once. The search-backed tables keep their explicit staleTime: 0.
Cold-loading an org from a slug URL fetched the same organization twice. The page resolves the URL segment with getOrganization, and the view then fetches by id: connect-query keys on the request message, so the slug and the id are different keys and both went to the server. In-app navigation was unaffected because it carries the id in router state and skips the resolve, so this only hit deep links and refreshes. Seed the id-keyed entry with the org already resolved. This is done during render rather than in an effect: the view mounts in the same commit and child effects run first, so an effect would seed the cache after the request had already gone out. Depends on a non-zero default staleTime; with staleTime 0 the seeded entry is immediately stale and the view refetches regardless.
The details context fetched listOrganizationUsers — the full, unpaginated member list — for every organization page, on every tab. The result was only ever read by the projects tab: its columns render project member avatars from it, and the add-members dropdown filters against it. Move it behind a useOrgMembersMap hook called by those two consumers. react-query dedupes the request between them, so the projects tab still issues one, and the members, tokens, API, security, invoices and PAT tabs no longer issue it at all. The select is defined at module scope so its identity is stable and react-query can memoize the derived map instead of rebuilding it on every render.
The invite trigger lives in the users page navbar, so the dialog component mounts with the page. Neither of the queries backing its fields was gated, so searchOrganizations and listRoles ran on every visit to the users list whether or not anyone opened the dialog. Gate both on the dialog's open state, as the PAT details dialog already does.
The earlier guard covered the tables rendering VirtualizedContent, whose scroll handler fires per frame. These three only checked hasNextPage, so a second call could still land while the previous page was in flight — and fetchNextPage cancels the in-flight page by default, turning that into an aborted request for no gain. All 11 server tables now check both hasNextPage and isFetchingNextPage before paging.
Cut each block back to the non-obvious point, and drop the load-more comment: it was repeated verbatim in three files and the guard reads clearly without it.
a5c2307 to
cec34fa
Compare
The previous guard read hasNextPage and isFetchingNextPage, but both are last-render values and react-query notifies its observers on a macrotask. VirtualizedContent calls onLoadMore straight from onScroll, so a burst of scroll events clears the guard several times before React re-renders. fetchNextPage defaults to cancelRefetch: true, so each of those extra calls aborted the in-flight page and re-issued it — exactly the redundant requests the guard was meant to prevent. Add a ref that flips synchronously, so only the first call in a burst gets through. All 11 server tables now share the shape, and the four that had the nested conditional were inverted to the same early return.
The account id comes from listBillingAccounts, and getBillingAccount stays disabled until it arrives — so on its own it reports "not loading" while its consumers are still waiting. Keeping the detail tab mounted through the billing load made that window visible for the first time: the side panel showed N/A for the billing name, email and address, and 0 / Prepaid under tokens, as though those were settled values rather than pending ones. OR the two legs together in the context, and have the side-panel sections wait on that combined flag instead of their own queries, which are disabled for the same reason. The same window left Edit billing and Add tokens submittable before the ids existed, so guard both submits and disable the Add button while billing loads. An org with no billing account still settles to false: the list resolves empty and getBillingAccount never enables.
The slug resolve and the view's fetch are separate cache entries, and an org edit invalidates only the id-keyed one. Seeding unconditionally meant a remount could write the slug copy — up to its five minute staleTime old — over fresher data, and the new default staleTime then kept it there with nothing to trigger a correction. Seed only when the id key holds nothing, and skip it entirely for a UUID URL, where the resolve already owns that key and the write was a no-op that just pushed dataUpdatedAt forward. Also records why the resolve tolerates disabled orgs: GetOrganization returns them to superusers only, the org-state gate answers everyone else with FailedPrecondition, and the console is superuser-only.
Drop the notes that restated the code beneath them and cut the rest to the one fact a reader needs in order not to undo the change. Also corrects the QueryClient note: plans and products already set staleTime: Infinity, so navigating never refetched them. Roles was the case the default actually fixes.
Shreyag02
removed
the
Do not merge
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b1080dc-9c22-45e9-bb04-474ed5412152
📒 Files selected for processing (21)
web/apps/admin/src/contexts/ConnectProvider.tsxweb/apps/admin/src/pages/organizations/details/index.tsxweb/sdk/admin/hooks/useOrgMembersMap.tsweb/sdk/admin/views/audit-logs/index.tsxweb/sdk/admin/views/invoices/index.tsxweb/sdk/admin/views/organizations/details/apis/index.tsxweb/sdk/admin/views/organizations/details/contexts/organization-context.tsxweb/sdk/admin/views/organizations/details/edit/billing.tsxweb/sdk/admin/views/organizations/details/index.tsxweb/sdk/admin/views/organizations/details/invoices/index.tsxweb/sdk/admin/views/organizations/details/layout/add-tokens-dialog.tsxweb/sdk/admin/views/organizations/details/members/index.tsxweb/sdk/admin/views/organizations/details/pat/index.tsxweb/sdk/admin/views/organizations/details/projects/index.tsxweb/sdk/admin/views/organizations/details/projects/members/index.tsxweb/sdk/admin/views/organizations/details/side-panel/billing-details-section.tsxweb/sdk/admin/views/organizations/details/side-panel/tokens-details-section.tsxweb/sdk/admin/views/organizations/details/tokens/index.tsxweb/sdk/admin/views/organizations/list/index.tsxweb/sdk/admin/views/users/list/invite-users.tsxweb/sdk/admin/views/users/list/list.tsx
💤 Files with no reviewable changes (1)
- web/sdk/admin/views/organizations/details/contexts/organization-context.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- web/sdk/admin/hooks/useOrgMembersMap.ts
- web/apps/admin/src/contexts/ConnectProvider.tsx
- web/apps/admin/src/pages/organizations/details/index.tsx
- web/sdk/admin/views/organizations/details/projects/index.tsx
- web/sdk/admin/views/organizations/details/index.tsx
- web/sdk/admin/views/organizations/list/index.tsx
Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review.
The submit guard added earlier returns silently when the org has no billing account, but isBillingAccountLoading settles to false in that case, so the Add button stays enabled. The click did nothing at all: the dialog stayed open with no toast and no console output. Hoist the guard condition into canCheckout and feed it to the button's disabled prop, so the state the guard rejects is no longer reachable.
Moving listOrganizationUsers out of the details provider and into useOrgMembersMap dropped the console.error that went with it. A failed fetch then left the projects tab rendering raw user ids in place of member avatars with no signal anywhere, not even in the console. Read error off the hook in the projects view and log it there, keeping the hook a plain query wrapper and the message in line with its siblings in details/index.tsx.
The one conflict was organizations/details/index.tsx, where both sides reworked the same block of the provider. - roles: take main's useOrganizationRoles hook and drop our two inline role queries. The hook logs both role errors itself, so removing them from the provider's error effect loses no logging. - members: keep our removal. main still fetched listOrganizationUsers here; that query and its error log now live in useOrgMembersMap and the projects view, which are its only readers. - isLoading: main's single isRolesLoading flag, still without the billing leg we dropped to stop the tab remounting mid-load.
rohilsurana
left a comment
rohilsurana
left a comment
There was a problem hiding this comment.
Review against main. Solid fix and well tested. Two broader risks worth a look (staleTime, render-phase cache write) and three smaller notes.
| queries: { | ||
| retry: false, | ||
| refetchOnWindowFocus: false, | ||
| staleTime: 30 * 1000, |
There was a problem hiding this comment.
A 30s app-wide staleTime means every read that does not opt out serves cached data for up to 30s after navigation, so correctness now rides on every mutation invalidating or setting the keys it touches. That is easy to miss. For example, creating an invite from the users list does not invalidate an org's listOrganizationInvitations, so it can show stale for 30s. Worth stating that convention clearly, or scoping the longer staleTime to just the queries that want it.
There was a problem hiding this comment.
Are we ok with this in long term?
There was a problem hiding this comment.
No, we weren't — you were right, and it's gone. ConnectProvider.tsx is back to exactly what's on main, no client-wide staleTime.
It's opt-in per query now, via SHARED_QUERY_STALE_TIME, applied to the four reads that were actually relying on the default:
getOrganizationby id — the slug-resolve seed is worthless without it (seeded +staleTime: 0is still 1 request); edit and block/unblock both invalidate that key- both legs of
useOrganizationRoles— reference data, and nothing in the console writes roles useOrgMembersMap— one observer per project row plus the project members page
Everything else keeps refetchOnMount, so your listOrganizationInvitations example can't go stale.
The convention is stated where you'd read it before using it, in constants.ts: only safe where every writer invalidates the key, so each query opts in and says why. That precondition wasn't holding for the member map — nothing invalidated it — so the members tab now invalidates listOrganizationUsers alongside its own table. Without that, a member removed there would have shown for 30s in the projects tab.
| cardinality: 'finite', | ||
| }); | ||
| if (queryClient.getQueryData(orgKey) === undefined) { | ||
| queryClient.setQueryData( |
There was a problem hiding this comment.
This seeds the cache with setQueryData in the render body rather than in an effect. A write during render can trigger React's "Cannot update a component while rendering a different component" warning if another mounted component already observes that query, and a discarded concurrent render still mutates the global cache. Moving it into a useEffect keeps it out of the render path.
There was a problem hiding this comment.
Moved out of render — but to a layout effect rather than a passive one, because a plain useEffect lands too late to be worth anything.
The view mounts in this same commit, and react-query subscribes from useSyncExternalStore (useBaseQuery.js:56), which React runs as a passive effect. Passive effects for a commit run after all layout effects, so:
| Seed runs in | Child fetches |
|---|---|
| render (what you flagged) | 0 |
| layout effect | 0 |
| passive effect | 1 |
Measured against real React 19.2.4 + react-query 5.90.21, identical under StrictMode. So a useEffect here would have quietly given back the GetOrganization this was saving.
The layout effect answers both halves of your comment: it's out of the render path, and layout effects only run on committed renders, so a discarded concurrent render no longer mutates the cache. The useRef latch went with it — the dep array plus the existing empty-key check cover it.
Confirmed end to end since: a cold load on a slug URL now issues exactly one GetOrganization.
| schema: AdminServiceQueries.searchOrganizationUsers, | ||
| transport, | ||
| input: {}, | ||
| input: { id: organizationId }, |
There was a problem hiding this comment.
If organizationId is ever an empty string, createConnectQueryKey drops the empty id field and the input collapses back to {}, which partial-matches and re-invalidates every org's member list, the exact bug this line fixes. It is guarded by the member-action preconditions today, but a non-empty-id guard here would make it safe by construction.
There was a problem hiding this comment.
Guarded — invalidateMembersQuery now returns early on an empty id.
It stopped being optional: scoping the staleTime meant useOrgMembersMap needed a writer, so this function gained a second invalidateQueries for listOrganizationUsers. Adding a second partial-match invalidation while relying on "the member-action preconditions hold today" would have doubled exactly the hazard you named.
Confirmed the mechanism rather than assuming it — message-key.js says it outright: "Default values are omitted (both implicit and explicit field presence)". "" is the proto3 default for a string, so { id: "" } really does collapse to input: {} and prefix-matches every org.
Both invalidations sit behind the one guard, and updateMember / removeMember are the only callers, so it's safe by construction now.
Four other sites build org-scoped keys the same way without a guard — remove-invite-dialog.tsx, invite-users-dialog.tsx, domains-list.tsx, sessions/index.tsx. The last two are worse in principle, since they write || "" explicitly. None is touched by this PR, so I've left them rather than fix in passing.
| data-test-id="add-tokens-invite-button" | ||
| type="submit" | ||
| loaderText="Adding..." | ||
| disabled={isBillingAccountLoading || !canCheckout || isSubmitting} |
There was a problem hiding this comment.
isBillingAccountLoading is redundant here. While billing loads, billingAccountId is empty so canCheckout is already false and the button is disabled. The extra term does not change the outcome.
There was a problem hiding this comment.
Agreed, dropped. disabled={!canCheckout || isSubmitting}.
isLoading is isPending && isFetching, so it's only true when there's no data at all — which means billingAccount is undefined, billingAccountId is "", and !canCheckout has already disabled the button. There's no state where the term flipped the result: a cached-data refetch surfaces as isFetching, not isLoading, so it can't produce a truthy flag alongside a real account either.
It was this file's only reader of the flag, so it came off the context destructure too. The flag itself stays — edit/billing.tsx and both side-panel sections still use it.
Related, from the same review pass: Edit billing had the mirror-image bug and is now fixed the same way. The description had claimed that panel was unreachable without a billing account; it isn't — Edit… → Billing… (layout/navbar.tsx:157) is ungated, so Save sat live over a guard that returns silently. Both panels now disable on the condition their guard rejects.
A client-wide default made every read serve cached data for 30s unless it opted out, so correctness rode on every mutation invalidating the keys it touches. That is easy to miss, and wrong for reads that move quickly — an org's invitations among them. Drop the default and opt in per query, for the four that relied on it: - getOrganization by id, so the page's slug-resolve seed stands instead of being refetched; edit and block/unblock both invalidate that key - both legs of useOrganizationRoles, read from the members, projects and invite surfaces, and nothing in the console writes roles - useOrgMembersMap, one observer per project row plus the project members page The member map had no writer invalidating it, so the members tab now invalidates that key alongside its own table, guarded against an empty org id that would otherwise partial-match every org. Everything else keeps react-query's refetch-on-mount.
isLoading is only true while there is no data, so billingAccount is undefined in that window and canCheckout is already false. The extra term never changed whether the button was disabled. It was this file's only reader of the flag, so drop it from the context destructure too.
Writing to the query cache in the render body notifies observers from a phase that is not allowed to, and a render React discards still mutates the global cache. A layout effect fixes both and keeps the request saving: the view mounts in this same commit and subscribes to that key from a passive effect, which runs after every layout effect, so the seed is still in place by the time it would otherwise refetch. A passive effect here would land too late and cost the extra GetOrganization back. The ref latch goes with it — the dependency array and the existing empty-key check already cover repeats.
The load-more latch was copied into all eleven server tables and the copies had already drifted: six guarded on isError but swallowed the failure, five logged it but retried on every further scroll. useLoadMore takes the union, so neither half regresses. useServerTableQuery comes from fix/admin-ui-followups unchanged, so this is the hook that branch already carries rather than a second one. It absorbs three different query-state shapes: - six org tabs held state, a memo and a debounce by hand - the org, user and invoice lists debounced the table state itself, so the grid lagged behind typing; only the request is debounced now, and the invoice list gains the debounce it never had - audit logs and project members kept query and rqlRequest in one object; audit logs still publishes its request for the navbar's CSV export, now from an effect, and project members drops sort through mapQuery The apis tab also starts resetting offset when the query changes, which the other ten already did. Eleven views, 223 lines lighter.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/apps/admin/src/pages/organizations/details/index.tsx (1)
91-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the source query timestamp when priming the ID key.
setQueryDataassigns the current time todataUpdatedAtby default. Pass the slug query’sdataUpdatedAtthrough theupdatedAtoption so the ID-keyed query does not extend stale data for 30 seconds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: be19e777-2ef0-4d3f-be3c-b9371cb106df
📒 Files selected for processing (19)
web/apps/admin/src/pages/organizations/details/index.tsxweb/sdk/admin/hooks/useLoadMore.tsweb/sdk/admin/hooks/useOrgMembersMap.tsweb/sdk/admin/hooks/useOrganizationRoles.tsweb/sdk/admin/hooks/useServerTableQuery.tsweb/sdk/admin/utils/constants.tsweb/sdk/admin/views/audit-logs/index.tsxweb/sdk/admin/views/invoices/index.tsxweb/sdk/admin/views/organizations/details/apis/index.tsxweb/sdk/admin/views/organizations/details/index.tsxweb/sdk/admin/views/organizations/details/invoices/index.tsxweb/sdk/admin/views/organizations/details/layout/add-tokens-dialog.tsxweb/sdk/admin/views/organizations/details/members/index.tsxweb/sdk/admin/views/organizations/details/pat/index.tsxweb/sdk/admin/views/organizations/details/projects/index.tsxweb/sdk/admin/views/organizations/details/projects/members/index.tsxweb/sdk/admin/views/organizations/details/tokens/index.tsxweb/sdk/admin/views/organizations/list/index.tsxweb/sdk/admin/views/users/list/list.tsx
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.
The panel opens from the navbar's Edit menu, which is ungated, so an org with no billing account can reach it. Both ids come from billingAccount, so the submit guard returned silently while Save still looked live — the same dead click already fixed on Add tokens. Disable on the condition the guard rejects, and name it so the two panels read the same way.
Left behind when the shared hook replaced this file's INITIAL_QUERY; the type had no other reference here.
3 participants
Summary
Every server-mode table fetched its first page twice, and the org detail page could fetch its active tab up to four times — visible on staging as
(canceled)requests.Our tables pass
defaultSorttoDataTablebut leavesortout of the initial query.DataTablemergesdefaultSortin and emits it on mount, changing the connect-query cache key and triggering a second request; the first is aborted mid-flight, after the server has already done the work.Scoped to duplicate and redundant requests only.
Changes
sortdefaultSorton project membersdetails/index.tsxisBillingAccountLoadinglistBillingAccountsleg, so the side panel showed empty fallbacks as settledstaleTimestaleTime: 0+refetchOnMountrefetched reference data on every navigationgetOrganization, roles, org member mapuseServerTableQuery,useLoadMoreopenconsole.errorWhy
createMessageKeyomits unset fields, sosort: []andsort: [{name: "created_at", …}]hash differently — one query in our heads, two cache entries. Seeding the initial sort makes the mount emit structurally identical to state, so the key never changes.isBillingAccountLoadingbut not thelistBillingAccountscall that enables it, and a disabled query reportsisLoading: false. So it rantrue→false(tab mounts, tables fetch) →true(unmounts) →false(remounts, tables refetch). It now only includes queries enabled from the first render, so it flips once.false, so the buttons enabled and the guards swallowed the click. Both now disable on the condition their guard rejects.hasNextPage/isFetchingNextPageare last-render values and react-query notifies on a macrotask, butVirtualizedContentcallsonLoadMorestraight fromonScroll— a burst clears the guard before React re-renders, andfetchNextPagedefaults tocancelRefetch: true, aborting the in-flight page. Only a synchronously-flipped ref closes it.staleTimeonly work togetherstaleTime: 0→ 1. Seeded + 30s → 0. The seed writes only into an empty key: slug- and id-keyed entries are separate and an edit invalidates only the id-keyed one, so an unconditional seed could overwrite fresher data.staleTimeis opt-inSHARED_QUERY_STALE_TIMEis applied to the four queries that need it; everything else keepsrefetchOnMount. The member map had no writer, so the members tab now invalidateslistOrganizationUserstoo.useEffectcosts the extraGetOrganizationback.offseton query change, as the other ten did. Audit logs still publishes its request for the CSV export, now from an effect.useLoadMoretakes the union of the two drifted latches, so neither half regresses.Merged
mainOne conflict in
details/index.tsx:mainreplaced the inline role queries withuseOrganizationRoles, this branch removed the member map. Both kept,isLoadingcombined asisOrganizationLoading || isRolesLoading.main's new invites table needs nothing here — it'smode="client", so Apsara skips the mount emit, and it has no load-more path.Test Plan
pnpm buildinweb/sdkandweb/apps/admintsc --noEmitweb/apps/admincleango build ./...,go vetafter the mergeeslinton all changed filesmode="server"table auditedweb/sdk/admin, all covered; each pairs seededsortwithdefaultSort, project members correctly has neitherStrictModeuseLoadMore, real hook in jsdomProduction bundle in headless Chrome — every page issued each RPC exactly once, none cancelled:
/organizationscoldconfigs= 5, none cancelled (was 6, with a cancelledSearchOrganizations)/users,/audit-logs,/invoicesGetOrganization, confirming the seedSearchOrganizations, none cancelledRan against a stub backend, so tables were empty: real data, latencies and error responses aren't covered, and browser-level load-more wasn't exercised — the hook test above covers the latch instead.
Still worth a click-through:
Out of scope
GetOrganization403 on the admins listapp/organization#getgrants superusers access only viaplatform->superuser, which needs the org'splatformrelation tuple — written once at creation, no backfill.OrgCellissues one per service-user row (views/admins/columns.tsx:74-78); the row falls back to the raw id with its button disabled, so nothing breaks.