feat(sdk): guard the delete organization flow - #1881
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughOrganization deletion now checks server-reported blockers and token balances. The UI disables deletion while checks load or blockers exist. The dialog displays processed precondition errors and token transfer guidance. ChangesOrganization deletion validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The organization-delete preflight can still enable the delete control before invoice status is available, allowing users to attempt deletion without complete blocker guidance. The server continues to enforce blocked deletions, so the impact is limited and the change is mergeable with explicit owner awareness or follow-up. 🚥 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 |
![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
🧹 Nitpick comments (1)
web/sdk/client/views/general/components/delete-organization-dialog.tsx (1)
48-48: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDefer the balance query until the dialog opens.
GeneralView mounts
DeleteOrganizationDialogeven whenopenis false. CallinguseTokens()starts the billing query for every permitted General view. A failed request can also show the “Unable to fetch balance” toast before the user opens the dialog.Add an
enabled: openoption touseTokens, or mount the dialog only while it is open.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e51689d-663d-4f44-83ac-03667d03f265
📒 Files selected for processing (2)
web/sdk/client/views/general/components/delete-organization-dialog.tsxweb/sdk/client/views/general/general-view.tsx
Coverage Report for CI Build 32711636426Warning No base build found for commit Coverage: 49.131%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
rohilsurana
left a comment
rohilsurana
left a comment
There was a problem hiding this comment.
Review against main. The guard follows the existing payment-issue pattern well. A few correctness and UX notes; the token-transfer wording is the one I would fix before this ships.
f15c922 to
4bf5ef9
Compare
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: 1f32f0d8-36f5-4f90-a760-824945d49de1
📒 Files selected for processing (1)
web/sdk/client/views/general/components/delete-organization-dialog.tsx
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.
| // fetched only while the dialog is open; the confirm button waits for the | ||
| // answer so the forfeit warning cannot be skipped by a slow response | ||
| const { tokenBalance, isTokensLoading } = useTokens({ enabled: open }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
fd -HI -t f \
'package\.json|package-lock\.json|npm-shrinkwrap\.json|yarn\.lock|pnpm-lock\.yaml' . \
-E node_modules \
-x sh -c 'printf "\n%s\n" "$1"; rg -n -C 2 "\"(`@tanstack/react-query`|react-query)\"" "$1" || true' sh {}Repository: raystack/frontier
Length of output: 882
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file="web/sdk/client/views/general/components/delete-organization-dialog.tsx"
printf '%s\n' '--- target file outline ---'
ast-grep outline "$file" --lang tsx || true
printf '%s\n' '--- target file ---'
cat -n "$file"
printf '%s\n' '--- useTokens definitions and usages ---'
rg -n -C 6 'useTokens|isTokensLoading|tokenBalance|isFetching' web/sdk/client web/sdk \
-g '*.{ts,tsx}' -g '!node_modules' | head -n 500
printf '%s\n' '--- related tests ---'
rg -n -C 5 'delete-organization-dialog|DeleteOrganization|forfeit|token balance|tokenBalance' web/sdk/client \
-g '*.{test,spec}.{ts,tsx}' -g '!node_modules' || trueRepository: raystack/frontier
Length of output: 41441
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- useTokens.ts ---'
cat -n web/sdk/client/hooks/useTokens.ts
printf '%s\n' '--- query option patterns ---'
rg -n -C 8 'useQuery\(' web/sdk/client/hooks web/sdk/client/views \
-g '*.{ts,tsx}' -g '!node_modules' | head -n 800
printf '%s\n' '--- lockfile package versions ---'
rg -n -C 3 '`@tanstack/`(react-query|query-core)|connect-query' web/pnpm-lock.yaml \
web/sdk/client/package.json web/sdk/package.json web/package.jsonRepository: raystack/frontier
Length of output: 50373
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- locked query packages ---'
python3 - <<'PY'
from pathlib import Path
p = Path("web/pnpm-lock.yaml")
lines = p.read_text().splitlines()
for i, line in enumerate(lines):
if any(x in line for x in ("`@tanstack/react-query`", "`@tanstack/query-core`", "`@connectrpc/connect-query`")):
print("\n".join(lines[max(0, i-3):i+8]))
print("---")
PY
printf '%s\n' '--- package declarations ---'
rg -n -C 2 '"(`@connectrpc/connect-query`|`@tanstack/react-query`)"' \
web/package.json web/sdk/package.json web/sdk/client/package.json web/apps/admin/package.json
printf '%s\n' '--- TanStack state semantics from repository documentation/comments ---'
rg -n -C 3 'isLoading.*isFetching|isFetching.*isLoading|refetchOnMount|staleTime' \
. -g '!node_modules' -g '!pnpm-lock.yaml' | head -n 300 || true
printf '%s\n' '--- standalone state probe ---'
node - <<'JS'
const cases = [
{ name: 'initial enabled request', hasData: false, isFetching: true },
{ name: 'cached data with background refetch', hasData: true, isFetching: true },
{ name: 'cached fresh data', hasData: true, isFetching: false },
];
for (const c of cases) {
// TanStack Query v5 derives isLoading from pending && fetching.
const status = c.hasData ? 'success' : 'pending';
const isPending = status === 'pending';
const isLoading = isPending && c.isFetching;
console.log(JSON.stringify({ ...c, status, isPending, isLoading }));
}
JSRepository: raystack/frontier
Length of output: 44836
Disable deletion during balance refetches.
isLoading is false when cached data exists during a background refetch. Expose isFetching from useTokens and include it in the delete-button guard. Add a regression test for reopening the dialog after the balance changes from zero to positive.
The delete button greys out while the org has open invoices with a non-zero amount, and its hover tooltip says to pay them from the billing page first — the server refuses the delete in that state anyway, this stops the user before the failed call. The delete dialog warns when tokens remain on the billing account: deleting forfeits them, and support can transfer the amount to the user's bank account. Confirming the dialog is the user's consent. The confirm button also disables while the request is in flight, so repeated clicks cannot fire the delete twice, and a failed_precondition response shows the server's reasons instead of a generic error.
The token balance is only fetched while the delete dialog is open, so the General page stops firing a balance request (and a failure toast on accounts without balance support) on every visit. The delete button now stays disabled while the unpaid-invoice answer is still loading instead of briefly allowing a delete the server would refuse, and it trusts the server-filtered result instead of re-checking only the state. The token warning stops promising a bank transfer for the whole balance — only a purchased amount is transferable, matching the notice email. The failed_precondition toast uses err.message like the other handlers, and the open-invoice filters live in one shared helper used by both the delete guard (one row, no sort — existence is all it needs) and the billing page's payment-issue banner.
4bf5ef9 to
d8b1b73
Compare
The delete button now asks the server directly whether a delete would go through (CheckOrganizationDelete, added in #1894) instead of inferring one blocker from the invoice list. The button greys out for every blocker the real delete would refuse with — a paid subscription to downgrade, unpaid invoices, a token debt — and the tooltip shows one short instruction per kind of blocker, with counts for invoices and a fallback to the server's message for kinds this version does not know. While the check loads the button stays disabled; a failed check fails open since the server refuses independently. @raystack/proton moves to the proton main commit that ships the RPC. The shared open-invoice filters stay for the billing page's payment-issue banner.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
Part of #1837 — the client half of the delete pre-flight. The server stack (#1865–#1894) is merged; this PR now contains all the SDK changes in one place.
Guards in the client SDK's General settings view:
CheckOrganizationDeleteRPC (shipped in feat(deleter): serve CheckOrganizationDelete for delete eligibility #1894,@raystack/protonbumped to the proton main commit that carries it). The button greys out for every blocker the real delete would refuse with — a paid subscription to downgrade, unpaid invoices, a token debt — and the hover tooltip shows one short instruction per kind of blocker ("Downgrade the subscription to the standard plan", "Pay the 2 open invoices from the billing page", "Contact support to settle the token balance"), with a fallback to the server's own message for kinds this version does not know. While the check loads the button stays disabled; a failed check fails open, since the server refuses a blocked delete independently.failed_preconditionresponse still surfaces the server's reasons in the error toast for anyone who reaches the server anyway.client/utils/invoice-queries.ts).🤖 Generated with Claude Code