fix(admin): stop duplicate API calls across admin tables - #1869
Draft
Shreyag02 wants to merge 13 commits into
Draft
fix(admin): stop duplicate API calls across admin tables#1869Shreyag02 wants to merge 13 commits into
Shreyag02 wants to merge 13 commits into
Conversation
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.
OrgCell ignored the query state and rendered
{org?.title || org?.name || orgId}, so a failed lookup silently printed
the raw org id in the organization column with a disabled button. A
denied or unreachable org was indistinguishable from one whose title
happens to be missing.
Render a skeleton while the lookup is in flight, and on failure show
"Unavailable" with the error and the id behind a tooltip.
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.
It wrote to [schema, {id}], which is not the shape connect-query uses for
its keys — those are ["connect-query", {serviceName, methodName, transport,
cardinality, input}]. The write therefore landed on an entry no query ever
reads and updated nothing.
Nothing called it: the edit panel invalidates the org query directly. Rather
than fix a key for a method with no callers, remove it and leave
invalidation as the one way the org is refreshed.
The current table query was written into the react-query cache under ["audit-logs", "table-query"] so the download button could read it back. No query owns that key, so the entry is garbage collected once idle for the default gcTime — after which the export ran with an undefined query and downloaded the unfiltered log. The value is plain component state already held one level up, so pass it down instead of routing it through the cache.
The organization tabs each rebuilt the same query state by hand: table state, a memo transforming it to RQL, a debounce, and a change handler. Four variations of that had drifted across the views, which is why the duplicate-request-on-mount bug existed in all of them and had been fixed in none. useServerTableQuery owns that shape, including the initial sort that has to agree with DataTable's defaultSort. Views that need to adjust the query before it becomes a request pass mapQuery; the organization invoices table uses it to convert its amount filter to cents. Field mappings and mapQuery are read through refs so an inline object at the call site cannot restart the debounce on every render.
The organization and user lists debounced their whole table state, so the
grid lagged behind typing; audit logs and project members carried a
{query, rqlRequest} pair through state; the invoices list had no debounce
at all. All four shapes did the same job as the tabs already migrated.
They now use useServerTableQuery, which debounces the request rather than
the table state, so the grid stays responsive while the RPC still waits
for typing to settle.
Project members keeps its behaviour through mapQuery: that endpoint
ignores sort, so the request drops it as the hand-written handler did.
All 11 mode="server" tables now share one implementation, and the initial
sort agreeing with defaultSort is a property of the hook rather than
something each view has to remember.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR centralizes server-side table query state and RQL generation across admin views. It adds organization member lookup through a dedicated hook, removes related context fields, primes organization caches, improves organization lookup states, and configures a 30-second default query freshness period. ChangesAdmin query and organization data refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
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
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 31441247261Coverage remained the same at 48.097%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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 fixes that and the other duplicate-fetch issues found while tracing it, then moves all 11 server tables onto one shared hook so the bug can't come back one view at a time.
Changes
Duplicate requests
sortDataTable's mount emit changed the request, so every table fetched page 1 twicedetails/index.tsxhasNextPage/isFetchingNextPagestaleTimeof 30sstaleTime: 0+refetchOnMountrefetched reference data on every navigationCorrectness and cleanup
defaultSorton project membersupdateOrganizationcontext methodRefactor
useServerTableQuery, migrate all 11 server tablesTechnical 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.
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 hook.
transformOptionsandmapQueryread through refsmapQueryescape hatchOut of scope, worth knowing. Staging also shows a
GetOrganizationreturning 403. 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. Needs the failing org id to confirm; raising separately with whoever owns the authz work.Test Plan
pnpm buildinweb/sdktsc --noEmitstaleTimestaleTime: 0→ 1 request; seeded + 30s → 0 requestsNot yet manually tested — what to look at:
/frontier-connect, prod build(canceled)entries on org list, users, audit logs, invoicesGetOrganization; tab does not flashSQL Safety (if your PR touches
*_repository.goorgoqu.*)Not applicable — frontend only, no Go or query-building changes. One backend file was read while confirming that
project_users_repository.goignoressort, but nothing underinternal/is modified.