fix: billing page confirms real paid state after checkout (no stale success banner)#2398
Merged
Merged
Conversation
…banner Stripe redirects the buyer back to /settings/billing?billing=success before the checkout.session.completed webhook lands, so the page showed a green subscription-updated banner while the plan still read Pro Trial and both Upgrade with Stripe buttons stayed visible (double-charge invite). On ?billing=success, poll the subscription summary every 2s up to ~20s: show a Finalizing your subscription state and hide both upgrade buttons until plan_tier flips to paid, then show the real success banner. On timeout or poll error, fall back to a softer payment-received message. The poll timer is cleared on unmount and guarded against setState-after-unmount.
…lling never loads Adversarial review: if every poll of the subscription summary failed (the endpoint erroring during the post-checkout webhook window), status stayed LOADING forever, leaving the membership card on a skeleton while the banner claimed payment received. Track whether any poll loaded a summary; on the cap, show the delayed banner only when a summary did load, otherwise set the error state so the card renders the unable-to-load message. Also strip the ?billing=success query param on read so a refresh no longer re-runs the whole polling cycle.
📝 WalkthroughWalkthroughThe billing page now polls subscription status after checkout success, tracks finalization and delayed outcomes, updates success alerts, hides checkout CTAs during finalization, and adds English messages for finalizing and delayed subscription states. ChangesBilling subscription finalization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BillingPage
participant BrowserHistory
participant subscriptionsApi
BillingPage->>BrowserHistory: Read and clear billing query
BillingPage->>subscriptionsApi: Poll subscription details
subscriptionsApi-->>BillingPage: Return plan summary and status
BillingPage->>BillingPage: Update finalizing or billing result state
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
app/javascript/src/components/Profile/Organization/Billing/index.tsx (2)
162-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate fetch/summary logic between
pollSummaryandfetchSummary.
pollSummaryre-implements the fetch +setSummary+setSeatEstimate(with the sameused_team_seats || 3fallback) already infetchSummary(Lines 73-83). Extracting a shared helper would avoid the two copies drifting out of sync.♻️ Proposed refactor
+ const loadBillingSummary = async () => { + const response = await subscriptionsApi.show(); + setSummary(response.data); + setSeatEstimate(Math.max(response.data.used_team_seats || 3, 3)); + return response.data; + }; + const fetchSummary = async () => { try { setStatus(ApiStatus.LOADING); - const response = await subscriptionsApi.show(); - setSummary(response.data); - setSeatEstimate(Math.max(response.data.used_team_seats || 3, 3)); + await loadBillingSummary(); setStatus(ApiStatus.SUCCESS); } catch { setStatus(ApiStatus.ERROR); } };And inside
pollSummary, replace the fetch/setSummary/setSeatEstimatelines withconst data = await loadBillingSummary();(then usedata.plan_tier).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/javascript/src/components/Profile/Organization/Billing/index.tsx` around lines 162 - 195, Extract the shared subscription fetch, summary state update, and seat-estimate fallback from fetchSummary and pollSummary into a loadBillingSummary helper. Update both callers to await this helper, use its returned data for plan_tier checks, and preserve the existing used_team_seats || 3 behavior.
139-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftNo test coverage for the polling/finalization flow.
This is a timer-driven, multi-branch (paid/delayed/error/unmount-guarded) effect on a critical billing path, but no accompanying test changes are included in this PR's file set.
Want me to draft tests (using fake timers) covering the paid, delayed, and all-attempts-failed branches?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/javascript/src/components/Profile/Organization/Billing/index.tsx` around lines 139 - 203, Add tests for the billing useEffect polling flow covering paid-plan completion, delayed completion after exhausting attempts with a loaded summary, and all-attempts-failed error handling. Use fake timers to advance the 2-second polling schedule and verify finalization, status, billing-result updates, API calls, and unmount protection around pollSummary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/javascript/src/components/Profile/Organization/Billing/index.tsx`:
- Around line 183-192: Update the polling exhaustion handling around the
attempts threshold and the success-banner condition near the billing result
alert so a failed run that never loads a summary cannot retain or display the
`"success"` billing result. Ensure the error path sets a non-success result
while preserving `"delayed"` when a summary was loaded, and make the top alert
render success only for an actually successful status.
---
Nitpick comments:
In `@app/javascript/src/components/Profile/Organization/Billing/index.tsx`:
- Around line 162-195: Extract the shared subscription fetch, summary state
update, and seat-estimate fallback from fetchSummary and pollSummary into a
loadBillingSummary helper. Update both callers to await this helper, use its
returned data for plan_tier checks, and preserve the existing used_team_seats ||
3 behavior.
- Around line 139-203: Add tests for the billing useEffect polling flow covering
paid-plan completion, delayed completion after exhausting attempts with a loaded
summary, and all-attempts-failed error handling. Use fake timers to advance the
2-second polling schedule and verify finalization, status, billing-result
updates, API calls, and unmount protection around pollSummary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05c7a845-827d-4e79-8eb3-d3687626a746
📒 Files selected for processing (2)
app/javascript/src/components/Profile/Organization/Billing/index.tsxapp/javascript/src/i18n/locales/en.ts
Comment on lines
+183
to
+192
| if (attempts >= 10) { | ||
| setFinalizing(false); | ||
| if (loadedSummary) { | ||
| setBillingResult("delayed"); | ||
| } else { | ||
| setStatus(ApiStatus.ERROR); | ||
| } | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Misleading "success" banner still shows when polling fully fails.
When all 10 attempts fail without ever loading a summary, setStatus(ApiStatus.ERROR) is called but billingResult is left as "success". Since the top alert at Line 392 only checks billingResult === "success", it will render "Subscription updated / Your plan was updated in Stripe successfully" at the same time the card below shows "Unable to load billing details" (Line 442-451). This directly contradicts the PR's goal of accurate post-checkout messaging — the user is told the update succeeded and failed simultaneously.
🐛 Proposed fix
if (attempts >= 10) {
setFinalizing(false);
if (loadedSummary) {
setBillingResult("delayed");
} else {
+ setBillingResult(null);
setStatus(ApiStatus.ERROR);
}
return;
}Also applies to: 395-407
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/javascript/src/components/Profile/Organization/Billing/index.tsx` around
lines 183 - 192, Update the polling exhaustion handling around the attempts
threshold and the success-banner condition near the billing result alert so a
failed run that never loads a summary cannot retain or display the `"success"`
billing result. Ensure the error path sets a non-success result while preserving
`"delayed"` when a summary was loaded, and make the top alert render success
only for an actually successful status.
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
1 participant
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.
Conversion-funnel audit, plan 020 (see
plans/). Depends on #2397 (merged).Problem
Stripe redirects the buyer back to
/settings/billing?billing=successthe instant checkout completes — usually before thecheckout.session.completedwebhook is processed. The page fetched its summary once on mount and unconditionally showed a green "subscription updated" banner, while the plan card still read "Pro Trial"/"No plan" and both "Upgrade with Stripe" buttons stayed visible — inviting a second purchase (the double-charge #2397 guards server-side). It only self-healed on manual refresh or the daily 01:00 reconcile job.Fix
On
?billing=success, poll the subscription summary every 2s up to ~20s:plan_tierflips topaid.?billing=successquery param on read so a refresh doesn't re-run the cycle.Verification
Browser-verified end to end with Playwright (zero console errors):
/settings/billing; a reload shows the normal page, no re-trigger.bin/vite buildclean, ESLint clean.Two defects were caught only because I verified in the browser + adversarial review, and fixed in this PR: a second unguarded upgrade button (the plan-table CTA), and a stuck-loading-skeleton when every poll fails during the webhook window. Dual-model adversarial review: CONFIRMED clean.
Deferred (documented in plan 020)
The attempt cap only advances when the request settles; the repo's axios client has no default timeout, so a hung first request is a pre-existing low-probability edge — noted as a follow-up, not blocking (per review).
Summary by CodeRabbit
New Features
Bug Fixes