fix(admin): stop duplicate API calls across admin tables - #1869
Open
Shreyag02 wants to merge 16 commits into
Open
fix(admin): stop duplicate API calls across admin tables#1869Shreyag02 wants to merge 16 commits into
Shreyag02 wants to merge 16 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR updates React Query freshness and organization cache priming, moves member-map loading into a dedicated hook, prevents overlapping table pagination requests, aligns initial table sorting, and improves billing and invitation query loading behavior. ChangesAdmin query and organization data updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to Billing actions may appear available before the required account identifiers exist, then silently do nothing when submitted. This is a bounded correctness issue that should have explicit owner awareness or follow-up before or after merge. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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
marked this pull request as draft
August 10, 2026 23:10
Shreyag02
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
| /** Search owned outside the table, e.g. the organization page's shared box. */ | ||
| search?: string; | ||
| /** Adjust the query before it becomes a request, e.g. converting units. */ | ||
| mapQuery?: (query: DataTableQuery) => DataTableQuery; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 'no-unused-vars|argsIgnorePattern|varsIgnorePattern' \
-g 'eslint.config.*' -g '.eslintrc*' -g 'package.json' .Repository: raystack/frontier
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target file ---'
sed -n '1,100p' web/sdk/admin/hooks/useServerTableQuery.ts
printf '%s\n' '--- repository lint/config files ---'
git ls-files | rg '(^|/)(eslint\.config\.[^/]+|\.eslintrc[^/]*|package\.json|.*lint.*)$' | head -200
printf '%s\n' '--- unused-argument conventions ---'
rg -n -S 'argsIgnorePattern|varsIgnorePattern|no-unused-vars|unused.*(param|arg)|^ *[_$][A-Za-z0-9_]*[,:)]' \
web package.json .github 2>/dev/null | head -300Repository: raystack/frontier
Length of output: 4286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ESLint configuration ---'
cat -n web/.eslintrc.js
printf '%s\n' '--- shared ESLint configuration ---'
cat -n web/tools/eslint-config/index.js
printf '%s\n' '--- relevant package scripts and dependencies ---'
node - <<'JS'
const fs = require("fs");
for (const file of ["web/package.json", "web/sdk/package.json", "web/apps/admin/package.json"]) {
if (!fs.existsSync(file)) continue;
const pkg = JSON.parse(fs.readFileSync(file, "utf8"));
console.log(`--- ${file} ---`);
console.log(JSON.stringify({
scripts: pkg.scripts,
eslintConfig: pkg.eslintConfig,
devDependencies: pkg.devDependencies,
dependencies: pkg.dependencies
}, null, 2));
}
JS
printf '%s\n' '--- existing declaration-only parameter suppressions ---'
cat -n web/sdk/admin/components/PageHeader.tsx
rg -n -C 3 'eslint-disable.*no-unused-vars|callback param name|type documentation' web --glob '*.{js,jsx,ts,tsx}'Repository: raystack/frontier
Length of output: 10156
Suppress the unused declaration parameter at line 20.
Add a targeted no-unused-vars suppression, consistent with web/sdk/admin/components/PageHeader.tsx. The query parameter at line 31 is used and does not need a suppression.
🧰 Tools
🪛 GitHub Check: JS SDK Lint
[warning] 20-20:
'query' is defined but never used
Source: Linters/SAST tools
Comment on lines
+19
to
+20
| const { organization } = useContext(OrganizationContext); | ||
| const { data: orgMembersMap = {} } = useOrgMembersMap(organization?.id); |
Contributor
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,
Coverage Report for CI Build 31486182655Coverage remained the same at 48.097%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.
Shreyag02
force-pushed
the
fix/admin-duplicate-api-calls
branch
from
August 11, 2026 11:20
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
Shreyag02
marked this pull request as ready for review
August 17, 2026 16:26
Contributor
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.
| ); | ||
|
|
||
| const onSubmit = async (data: BillingDetailsForm) => { | ||
| if (!organizationId || !billingId) return; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align billing controls with missing-identifier guards.
Both handlers silently no-op when required identifiers are unavailable. Keep the corresponding controls disabled, or show an explicit unavailable-account state.
web/sdk/admin/views/organizations/details/edit/billing.tsx#L129-L129: include missingorganizationIdandbillingIdin the Save disabled condition.web/sdk/admin/views/organizations/details/layout/add-tokens-dialog.tsx#L88-L88: retain the guard fororganisationIdandbillingAccountId.web/sdk/admin/views/organizations/details/layout/add-tokens-dialog.tsx#L161-L161: include missing identifiers in the Add disabled condition.
📍 Affects 2 files
web/sdk/admin/views/organizations/details/edit/billing.tsx#L129-L129(this comment)web/sdk/admin/views/organizations/details/layout/add-tokens-dialog.tsx#L88-L88web/sdk/admin/views/organizations/details/layout/add-tokens-dialog.tsx#L161-L161
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
2 participants
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
Every server-mode table in the admin UI was fetching its first page twice, and the organization detail page could fetch its active tab up to four times. Both were visible on staging as
(canceled)requests in the network tab.The root cause is small: our tables pass
defaultSorttoDataTablebut leavesortout of the initial query.DataTablemergesdefaultSortin and emits it on mount, which changes the connect-query cache key and triggers a second request. The first one is then aborted mid-flight — after the server has already done the work.This PR is scoped to duplicate and redundant requests only. The shared-hook refactor and four unrelated fixes found along the way moved to
fix/admin-ui-followups.Measured on a production build: the organization list now issues 5 requests with no cancellation, against 6-with-a-cancelled-
SearchOrganizationson the deployed build. Numbers below.Changes
sortDataTable's mount emit changed the request, so every table fetched page 1 twicedefaultSorton project membersdetails/index.tsxisBillingAccountLoadinglistBillingAccountsleg, so the side panel rendered empty fallbacks as settled valuesstaleTimeof 30sstaleTime: 0+refetchOnMountrefetched reference data on every navigationopensearchOrganizations+listRolesran on every visit to the users listTechnical Details
Why the key changes. connect-query builds cache keys with
createMessageKey, which omits unset fields.sort: []andsort: [{name: "created_at", …}]therefore hash differently — same query in our heads, two cache entries in practice. Seeding the initial sort makes the mount emit structurally identical to what's already in state, so the key never changes.Why the tab remounted. The layout's
isLoadingincludedisBillingAccountLoading, but thelistBillingAccountscall that enables that query wasn't in the gate. A disabled query reportsisLoading: false, so:truefalselistBillingAccountsresolves, billing query enablestruefalseThe gate now only includes queries enabled from the first render, so it can flip once and stay there.
What narrowing the gate exposed. The same "disabled query reports not-loading" quirk that caused the remount also made
isBillingAccountLoadingwrong for its own consumers: it coversgetBillingAccount, which stays disabled untillistBillingAccountsyields an id. While the spinner hid that window nobody noticed. With the tab mounted throughout, the side panel rendered N/A for billing name/email/address and 0 / Prepaid for tokens as though they were settled, and Edit billing / Add tokens became submittable before the ids existed. The context now ORs both legs, the side-panel sections wait on the combined flag rather than their own disabled queries, and both submits are guarded. An org with genuinely no billing account still settles tofalse— the list resolves empty andgetBillingAccountnever enables.Why a guard wasn't enough for load-more.
hasNextPageandisFetchingNextPageare last-render values, and react-query notifies observers on a macrotask (notifyManager's default scheduler issetTimeout(…, 0)).VirtualizedContentcallsonLoadMorestraight fromonScroll, so a burst of scroll events clears the guard several times before React re-renders — andfetchNextPagedefaults tocancelRefetch: true, aborting the in-flight page and re-issuing it. A ref that flips synchronously is what actually closes it; the guard stays as a cheap first filter.Two changes that only work together. Seeding the resolved org into the cache does nothing while
staleTimeis 0 — the entry is stale on arrival and the view refetches anyway. Please don't land one without the other:staleTime30sstaleTime0staleTime30sThe seed writes only into an empty key. The slug-keyed and id-keyed entries are separate, and an org edit invalidates only the id-keyed one, so an unconditional seed could put the slug copy — up to its five minute
staleTimeold — over fresher data, where the new default would then keep it. A UUID URL skips the seed entirely: the resolve already owns that key.Follow-up branch
fix/admin-ui-followupsbranches off this one and carries what isn't about call volume:useServerTableQuery+ all 11 tables migratedupdateOrganizationOut of scope, worth knowing. Staging also shows a
GetOrganizationreturning 403, and it reproduces on a local production build. It isn't a duplicate-call artifact —app/organization#getgrants superusers access only viaplatform->superuser, which needs the org'splatformrelation tuple. That tuple is written once byAttachToPlatformat creation, with no backfill or reconcile path if it's missing. It surfaces on the admins list, whereOrgCellissues oneGetOrganizationper service-user row (views/admins/columns.tsx:74-78); the affected row falls back to the raw id with its button disabled, so nothing breaks. Raising separately with whoever owns the authz work.Test Plan
pnpm buildinweb/sdk,pnpm buildinweb/apps/admintsc --noEmiteslinton all changed filesstaleTimestaleTime: 0→ 1 request; seeded + 30s → 0 requestsMeasured on a production build (
vite preview; dev is unusable for this —React.StrictModedouble-invokes mounts and connect-query aborts and refires each query, so dev numbers roughly double):/organizationscold loadGetCurrentUser,configs,GetCurrentAdminUser,SearchOrganizations,ListPlans— 5, all 200, none cancelledSearchOrganizationsGetOrganization, then the detail fan-out; oneSearchOrganizationUsers, no remountStill to check:
/organizations/<slug>/securityGetOrganization, not two. In-app nav carriesstate.orgIdand skips the resolve, so it does not exercise the seed