feat(billing): /upgrade naar Stripe + Mijn Ribba met portal-endpoint - #32
Conversation
Bedrading van de bewezen Stripe-keten naar de webclient — geen nieuwe billinglogica: - /upgrade roept nu de edge function stripe-create-checkout aan (attempt_id per bewuste poging, hergebruik bij retry van hetzelfde plan, dubbelklik-blokkade in controller én knop, Nederlandse fouten); - /upgrade/success toont het echte plan (URL-param voor Mollie, sessionStorage voor Stripe; onbekend -> neutrale tekst, nooit stil 'Premium'); - POST /api/portal maakt met Ribba-auth een verse Stripe Customer Portal-sessie op de bestaande stripe_customers-koppeling (fail-closed keyprefix, objectbewijs op session.livemode, no_customer -> 409 met duidelijke vervolgstap); - /mijn-ribba: minimale permanente klantportaalpagina met één actie. Mollie-paden (/api/checkout, /api/cancel-subscription, mollie-webhook) volledig onaangeroerd; bestaande Mollie-klanten blijven ongewijzigd werken. Tests: 136/136 groen (23 nieuw). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds an authenticated Mijn Ribba portal page, introduces Stripe portal and checkout Edge Function helpers with concurrency and retry handling, updates the upgrade flow to use those helpers, and makes the success page plan-neutral. Tests cover request wiring, validation, errors, and gating. ChangesStripe billing flows
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)Upgrade checkoutsequenceDiagram
participant UpgradePage
participant CheckoutController
participant startStripeCheckout
participant StripeCheckoutFunction
UpgradePage->>CheckoutController: Begin plan attempt
UpgradePage->>startStripeCheckout: Send checkout request
startStripeCheckout->>StripeCheckoutFunction: POST with Bearer token
StripeCheckoutFunction-->>startStripeCheckout: Return checkoutUrl or error
startStripeCheckout-->>UpgradePage: Return checkout result
Mijn Ribba portalsequenceDiagram
participant Browser
participant MijnRibbaPage
participant SupabaseAuth
participant openStripePortal
participant StripePortalFunction
Browser->>MijnRibbaPage: Open portal page
MijnRibbaPage->>SupabaseAuth: Read session
MijnRibbaPage->>openStripePortal: Send school_id and access token
openStripePortal->>StripePortalFunction: POST portal request
StripePortalFunction-->>openStripePortal: Return portal URL or error
openStripePortal-->>MijnRibbaPage: Return portal result
MijnRibbaPage->>Browser: Navigate to portal URL
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/upgrade/page.tsx (1)
163-213: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
handleCheckoutnever releases the controller'sinFlightlock outside the explicit failure branch.Two exit paths leave
checkoutControllerRef.currentpermanently locked for this component instance:
- Lines 185-188: if
tokenis missing, the function returns without callingfail(plan).- Lines 206-213: on success, nothing releases
inFlightbefore/while redirecting viawindow.location.href.Because this is a full-page
window.location.hrefnavigation (often cross-origin, to Stripe), the browser's back-forward cache can restore this exact component/ref state when the user returns via the back button — at which pointbegin()silently returnsnullforever, and clicking any plan button does nothing with no visible error. See the consolidated comment (anchored onlib/stripe-upgrade.ts) for the suggestedcomplete()API and the corresponding call-site fix here.🤖 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/upgrade/page.tsx` around lines 163 - 213, Update handleCheckout to release checkoutControllerRef.current’s inFlight lock on every exit after begin(plan), including the missing-token branch and the successful checkout path before window.location.href navigation. Use the controller’s suggested complete() API for successful attempts and retain fail(plan) for failed attempts, ensuring back-forward-cache restoration allows a new checkout attempt.
🧹 Nitpick comments (5)
app/api/portal/route.ts (1)
78-88: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the Stripe Billing Portal call.
The
fetchto Stripe has no timeout, so a slow/hung upstream ties up the request until the platform limit. Wrap it withAbortSignal.timeout(...)and handle the abort in the existing catch.♻️ Suggested timeout guard
const stripeRes = await fetch('https://api.stripe.com/v1/billing_portal/sessions', { method: 'POST', headers: { Authorization: `Bearer ${stripeKey}`, 'Content-Type': 'application/x-www-form-urlencoded', }, body: new URLSearchParams({ customer: decision.stripeCustomerId, return_url: returnUrl, }), + signal: AbortSignal.timeout(10_000), });🤖 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/api/portal/route.ts` around lines 78 - 88, Add a timeout to the Stripe Billing Portal request in the fetch call by passing an AbortSignal.timeout value, and ensure the existing catch handles timeout aborts consistently with other request failures. Keep the current request payload and response handling unchanged.app/upgrade/page.tsx (1)
166-166: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor:
createCheckoutController()is invoked on every render.
useRef(createCheckoutController())evaluates the argument on every re-render even thoughuseRefonly keeps the first value. The controller is cheap to construct, so this is negligible, but a lazy pattern avoids the redundant allocation.🤖 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/upgrade/page.tsx` at line 166, Update the checkoutControllerRef initialization in the upgrade page to lazily create the controller only when the ref has no current value, rather than evaluating createCheckoutController() on every render. Preserve the existing controller instance across subsequent renders.app/upgrade/success/page.tsx (1)
15-22: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueStale plan label on repeat visits.
UPGRADE_PLAN_STORAGE_KEYis read but never cleared, so if this success page is revisited later in the same tab session without a fresh?plan=, it could still show the previous purchase's plan label. Low priority sinceurlPlanwins when present (Mollie flow) and sessionStorage is tab-scoped, but considersessionStorage.removeItem(UPGRADE_PLAN_STORAGE_KEY)after reading to avoid staleness on a second visit within the Stripe flow.🤖 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/upgrade/success/page.tsx` around lines 15 - 22, Clear UPGRADE_PLAN_STORAGE_KEY from sessionStorage immediately after reading it in the success-page effect, while preserving the existing error handling for blocked storage and the successPlanLabel call using the retrieved value.lib/stripe-upgrade.ts (1)
70-111: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout/abort on the checkout fetch.
If the edge function hangs,
startStripeCheckoutwill wait indefinitely; combined with the controller-lock issue above, the button stays disabled/"Bezig..." with no escape until the user reloads. Consider wiring anAbortController-based timeout (a few seconds) so a hang degrades to a retryable error instead of an indefinite spinner.🤖 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 `@lib/stripe-upgrade.ts` around lines 70 - 111, The startStripeCheckout fetch can hang indefinitely and leave the checkout flow stuck. Add an AbortController-based timeout around the doFetch call in startStripeCheckout, pass its signal to the request, and convert timeout aborts into the existing retryable checkout error while preserving current network and response handling.tests/stripe-upgrade.test.mjs (1)
21-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo test covers the success-then-reattempt / stuck-
inFlightscenario.Given the gap flagged in
lib/stripe-upgrade.ts(no way to releaseinFlightafter a successfulbegin()), consider adding a test asserting that after a successful checkout flow, a subsequentbegin()for the same or a different plan is not permanently blocked. This would lock in the fix oncecomplete()/reset is added.🤖 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 `@tests/stripe-upgrade.test.mjs` around lines 21 - 48, The checkout controller tests cover duplicate attempts and failure retries but not releasing inFlight after success. Add a test using createCheckoutController that begins a plan, completes or resets the successful attempt through the exposed API, then verifies a subsequent begin for the same or another plan is accepted and produces the expected attempt_id.
🤖 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/mijn-ribba/page.tsx`:
- Around line 32-52: Update the getSession promise chain in the page component
to add a rejection handler that calls setLoading(false) and surfaces the
authentication/session error using the component’s existing error-handling
mechanism. Keep the inner try/finally behavior unchanged for successful session
retrieval and API failures.
- Line 123: Replace the raw anchor for the internal /upgrade navigation in the
page component with Next.js’s Link component, importing next/link as needed
while preserving the existing link text and surrounding markup.
In `@lib/stripe-upgrade.ts`:
- Around line 16-48: Extend the CheckoutController API and
createCheckoutController with a completion/reset method that clears inFlight and
removes the plan’s cached attempt ID. Update app/upgrade/page.tsx’s
handleCheckout to invoke this method both when checkout succeeds before
redirecting and when the access token is missing before the network request,
while retaining fail() for request errors.
---
Outside diff comments:
In `@app/upgrade/page.tsx`:
- Around line 163-213: Update handleCheckout to release
checkoutControllerRef.current’s inFlight lock on every exit after begin(plan),
including the missing-token branch and the successful checkout path before
window.location.href navigation. Use the controller’s suggested complete() API
for successful attempts and retain fail(plan) for failed attempts, ensuring
back-forward-cache restoration allows a new checkout attempt.
---
Nitpick comments:
In `@app/api/portal/route.ts`:
- Around line 78-88: Add a timeout to the Stripe Billing Portal request in the
fetch call by passing an AbortSignal.timeout value, and ensure the existing
catch handles timeout aborts consistently with other request failures. Keep the
current request payload and response handling unchanged.
In `@app/upgrade/page.tsx`:
- Line 166: Update the checkoutControllerRef initialization in the upgrade page
to lazily create the controller only when the ref has no current value, rather
than evaluating createCheckoutController() on every render. Preserve the
existing controller instance across subsequent renders.
In `@app/upgrade/success/page.tsx`:
- Around line 15-22: Clear UPGRADE_PLAN_STORAGE_KEY from sessionStorage
immediately after reading it in the success-page effect, while preserving the
existing error handling for blocked storage and the successPlanLabel call using
the retrieved value.
In `@lib/stripe-upgrade.ts`:
- Around line 70-111: The startStripeCheckout fetch can hang indefinitely and
leave the checkout flow stuck. Add an AbortController-based timeout around the
doFetch call in startStripeCheckout, pass its signal to the request, and convert
timeout aborts into the existing retryable checkout error while preserving
current network and response handling.
In `@tests/stripe-upgrade.test.mjs`:
- Around line 21-48: The checkout controller tests cover duplicate attempts and
failure retries but not releasing inFlight after success. Add a test using
createCheckoutController that begins a plan, completes or resets the successful
attempt through the exposed API, then verifies a subsequent begin for the same
or another plan is accepted and produces the expected attempt_id.
🪄 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: 232fdc99-21ea-4829-afe0-fd00373448f2
📒 Files selected for processing (8)
app/api/portal/route.tsapp/mijn-ribba/page.tsxapp/upgrade/page.tsxapp/upgrade/success/page.tsxlib/portal-session.tslib/stripe-upgrade.tstests/portal-session.test.mjstests/stripe-upgrade.test.mjs
| supabase.auth.getSession().then(async ({ data }) => { | ||
| if (!data.session) { | ||
| router.replace('/login'); | ||
| return; | ||
| } | ||
| setUserEmail(data.session.user.email ?? null); | ||
| try { | ||
| const meRes = await fetch('/api/me', { | ||
| headers: { Authorization: `Bearer ${data.session.access_token}` }, | ||
| }); | ||
| if (meRes.ok) { | ||
| const me = await meRes.json(); | ||
| if (me.school_id) setSchoolId(me.school_id); | ||
| if (me.school_name) setSchoolName(me.school_name); | ||
| } | ||
| } catch { | ||
| // foutafhandeling hieronder via ontbrekende schoolId | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
getSession() rejection leaves the page stuck on "Laden…".
The .then(...) chain has no rejection handler, and setLoading(false) runs only inside the inner try/finally. If getSession() itself rejects (network/session error), loading never clears and the user sees an indefinite spinner. Add a .catch that clears loading (and surfaces an error).
🛡️ Suggested guard
- supabase.auth.getSession().then(async ({ data }) => {
+ supabase.auth.getSession().then(async ({ data }) => {
if (!data.session) {
router.replace('/login');
return;
}
setUserEmail(data.session.user.email ?? null);
try {
const meRes = await fetch('/api/me', {
headers: { Authorization: `Bearer ${data.session.access_token}` },
});
if (meRes.ok) {
const me = await meRes.json();
if (me.school_id) setSchoolId(me.school_id);
if (me.school_name) setSchoolName(me.school_name);
}
} catch {
// foutafhandeling hieronder via ontbrekende schoolId
} finally {
setLoading(false);
}
- });
+ }).catch(() => setLoading(false));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| supabase.auth.getSession().then(async ({ data }) => { | |
| if (!data.session) { | |
| router.replace('/login'); | |
| return; | |
| } | |
| setUserEmail(data.session.user.email ?? null); | |
| try { | |
| const meRes = await fetch('/api/me', { | |
| headers: { Authorization: `Bearer ${data.session.access_token}` }, | |
| }); | |
| if (meRes.ok) { | |
| const me = await meRes.json(); | |
| if (me.school_id) setSchoolId(me.school_id); | |
| if (me.school_name) setSchoolName(me.school_name); | |
| } | |
| } catch { | |
| // foutafhandeling hieronder via ontbrekende schoolId | |
| } finally { | |
| setLoading(false); | |
| } | |
| }); | |
| supabase.auth.getSession().then(async ({ data }) => { | |
| if (!data.session) { | |
| router.replace('/login'); | |
| return; | |
| } | |
| setUserEmail(data.session.user.email ?? null); | |
| try { | |
| const meRes = await fetch('/api/me', { | |
| headers: { Authorization: `Bearer ${data.session.access_token}` }, | |
| }); | |
| if (meRes.ok) { | |
| const me = await meRes.json(); | |
| if (me.school_id) setSchoolId(me.school_id); | |
| if (me.school_name) setSchoolName(me.school_name); | |
| } | |
| } catch { | |
| // foutafhandeling hieronder via ontbrekende schoolId | |
| } finally { | |
| setLoading(false); | |
| } | |
| }).catch(() => setLoading(false)); |
🤖 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/mijn-ribba/page.tsx` around lines 32 - 52, Update the getSession promise
chain in the page component to add a rejection handler that calls
setLoading(false) and surfaces the authentication/session error using the
component’s existing error-handling mechanism. Keep the inner try/finally
behavior unchanged for successful session retrieval and API failures.
| <div className="divider" /> | ||
|
|
||
| <p className="footer-text"> | ||
| Abonnement kiezen of wijzigen? <a href="/upgrade">Bekijk de plannen</a>.<br /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use next/link for internal navigation to /upgrade.
The raw <a href="/upgrade"> triggers a full page reload instead of client-side navigation.
♻️ Suggested change
-import { useRouter } from 'next/navigation';
+import { useRouter } from 'next/navigation';
+import Link from 'next/link';- Abonnement kiezen of wijzigen? <a href="/upgrade">Bekijk de plannen</a>.<br />
+ Abonnement kiezen of wijzigen? <Link href="/upgrade">Bekijk de plannen</Link>.<br />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Abonnement kiezen of wijzigen? <a href="/upgrade">Bekijk de plannen</a>.<br /> | |
| Abonnement kiezen of wijzigen? <Link href="/upgrade">Bekijk de plannen</Link>.<br /> |
🧰 Tools
🪛 ESLint
[error] 123-123: Do not use an <a> element to navigate to /upgrade/. Use <Link /> from next/link instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages
(@next/next/no-html-link-for-pages)
🤖 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/mijn-ribba/page.tsx` at line 123, Replace the raw anchor for the internal
/upgrade navigation in the page component with Next.js’s Link component,
importing next/link as needed while preserving the existing link text and
surrounding markup.
Source: Linters/SAST tools
…cret in Vercel Correctieronde op de wiring-PR na review: - /api/portal en lib/portal-session.ts volledig verwijderd; de beslislogica leeft nu in de ribbaPro edge function stripe-portal-session (PR #266) binnen de projectbrede secretset; - Mijn Ribba roept de edge function direct geauthenticeerd aan met de user-JWT (openStripePortal-helper, zelfde patroon als checkout); - succestekst plan-neutraal: een losse ?plan=-parameter of client-side opslag mag niet bepalen wat als gekocht wordt gepresenteerd; - attempt-semantiek aangescherpt: netwerkfout/ambigue uitkomst hervat dezelfde attempt_id; ontvangen definitieve HTTP-fout sluit de poging af en een bewuste nieuwe klik krijgt een nieuwe attempt_id (Stripe cachet ook fouten op een idempotency-key); - tests bijgewerkt: 129/129 groen; Mollie-paden onveranderd zonder diff; nul Stripe-secretreferenties in de hele codebase. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
portalBusy + disabled is UI-bescherming, geen harde garantie: twee click-events kunnen dezelfde render-state zien. createPortalGate is een synchrone in-flight lock (ref-patroon, zelfde principe als de checkout-controller): de eerste aanroep verkrijgt de lock en start één request, een tweede stopt vóór getSession/fetch, finally geeft de lock vrij. Bewust géén attempt_id op het portal-pad — een portal-sessie kent geen idempotente resource; de gate bewaakt uitsluitend het aantal requests. Tests: dubbel-aanroep levert exact één Edge Function-request; gate komt vrij na definitieve fout én na netwerkfout; na terugkeer kan bewust een nieuwe sessie starten. 19/19 groen, tsc schoon. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Na een geslaagde checkout blijft de in-flight lock bewust staan tot de redirect naar Stripe. Keert de gebruiker via browser-back terug, dan herstelt de browser de pagina uit de back/forward-cache mét die actieve lock — de checkoutknoppen waren dan permanent dood (CodeRabbit-Major). Correctie: expliciete reset() op de controller die uitsluitend de lock vrijgeeft; de upgradepagina registreert een pageshow-handler die alléén bij event.persisted === true reset() aanroept en de loading-state wist, met listener-cleanup in de effect-teardown. De attempt_id-semantiek is onaangetast: een bfcache-terugkeer is geen serverantwoord, dus er wordt niets afgesloten of vernieuwd — een nieuwe klik op hetzelfde plan hervat dezelfde idempotente poging. Vijf checkouttests in paginacompositie: dubbelklik -> exact één request; succes vóór redirect -> lock actief; bfcache-terugkeer -> lock hersteld en bewuste klik mogelijk (zelfde id); definitieve fout -> lock vrij + nieuwe attempt_id; netwerkfout -> lock vrij + zelfde attempt_id. 24/24 groen, tsc schoon. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Wat (na B5-correctieronde, commit dbacbbe)
De wiring-PR uit het vierstappenplan: de bewezen Stripe-keten aangesloten op de webclient. Geen nieuwe billing-, retry-, Customer-, webhook- of licentielogica, en — na de correctie — nul Stripe-secrets in Vercel: alle Stripe-runtime blijft binnen de projectbrede Supabase-secretset (B5).
/upgrade→ Stripe:handleCheckoutroeptstripe-create-checkoutaan i.p.v. het Mollie-/api/checkout. Attempt-semantiek: éénattempt_idper bewuste poging; netwerkfout/ambigue uitkomst → zelfdeattempt_idhervat; ontvangen definitieve HTTP-fout → poging afgesloten, bewuste nieuwe klik = nieuweattempt_id(Stripe cachet ook fouten op een idempotency-key); ander plan = eigen id. Dubbelklik geblokkeerd in controller én disabled-knop; Nederlandse fouten (servertekst met NL-fallback).?plan=-parameter of client-side opslag bepaalt niet meer wat als gekocht wordt gepresenteerd; een planlabel komt pas terug met een betrouwbare server-side bron./mijn-ribba+ link op/upgrade): roept geauthenticeerd de ribbaPro edge functionstripe-portal-sessionaan (PolderLabs/ribbaPro#266) met de user-JWT — zelfde patroon als checkout; de functie geeft{ url }van een verse portal-sessie terug. Het eerdere/api/portalenlib/portal-session.tszijn volledig verwijderd.Drie poortvragen ("Stripe doet Stripe")
Bewijs (
npm test: 129/129 groen;next buildslaagt)… roept stripe-create-checkout aan met school, plan en attempt_iddubbelklik: tweede begin() … geeft nullnetwerkfout (ambigue uitkomst): retry hervat DEZELFDE attempt_iddefinitieve HTTP-fout sluit de poging af: nieuwe klik = NIEUWE attempt_idopenStripePortal roept stripe-portal-session aan met JWT en school_id(eigenaarschap/modus/accountscope: getest in ribbaPro #266)portal: serverfout (bv. 409 geen koppeling) toont de Nederlandse servertekstSTRIPE_SECRET/ACCOUNT/WEBHOOK/sk_/rk_→ 0 treffersVolgorde
Eerst PolderLabs/ribbaPro#266 beoordelen (de edge function), daarna deze PR — Mijn Ribba werkt pas nadat #266 gemerged én gedeployed is.
🤖 Generated with Claude Code
Summary by CodeRabbit