-
-
Notifications
You must be signed in to change notification settings - Fork 47
Move Billing to user settings #470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughStandalone Billing page removed; billing moved into a new Billing tab inside UserSettings. Pricing components replace radio selectors with a yearly toggle and discount badge; server actions and API calls moved from Changes
Sequence Diagram(s)sequenceDiagram
participant User as Browser (UserSettings UI)
participant BillingUI as BillingPricing Component
participant Fetcher as fetcher -> /user-settings (action)
participant Server as Remix Route (user-settings action)
participant API as Internal API (user/preview-plan, user/change-plan)
rect rgba(200,220,255,0.5)
User->>BillingUI: click "Yearly" / select plan
BillingUI->>Fetcher: POST preview-subscription-update { planId }
Fetcher->>Server: action: preview-subscription-update
Server->>API: POST user/preview-plan { planId }
API-->>Server: preview data
Server-->>Fetcher: { success:true, data: preview }
Fetcher-->>BillingUI: preview response
BillingUI-->>User: show preview/discount
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@web/app/components/pricing/BillingPricing.tsx`:
- Around line 318-341: The Switch is inert because its onChange is a no-op and
it stops propagation; update the Switch component inside BillingPricing so its
onChange toggles billingFrequency using the same logic as the surrounding button
(call setBillingFrequency with billingFrequency === BillingFrequency.yearly ?
BillingFrequency.monthly : BillingFrequency.yearly), keeping checked bound to
billingFrequency === BillingFrequency.yearly so the control actually toggles
when clicked.
In `@web/app/components/pricing/MarketingPricing.tsx`:
- Around line 161-184: The Switch's onChange is a no-op so clicking the control
does nothing; wire the Switch to the same state updater used by the surrounding
button by calling setBillingFrequency with the toggled value (using
billingFrequency and BillingFrequency enum) inside the Switch's onChange handler
so it mirrors the button's onClick behavior (ensure you don't double-toggle by
making the Switch handler perform the toggle logic itself rather than relying on
event propagation).
In `@web/app/pages/UserSettings/UserSettings.tsx`:
- Around line 293-297: The percentage math for totalUsage and remainingUsage
isn't guarded against maxEventsCount being 0; update the totalUsage calculation
in the UserSettings component to avoid division-by-zero and clamp results: when
computing totalUsage (using maxEventsCount, usageInfo.total and _round) treat a
zero maxEventsCount as a safe base (e.g., treat denominator as at least 1 or if
maxEventsCount === 0 and usageInfo.total > 0 set totalUsage to 100) and then
clamp totalUsage to [0,100]; compute remainingUsage as _round(Math.max(0, 100 -
totalUsage), 2) so it can never be negative or NaN. Apply the same
guard/clamping logic wherever the same block is repeated (the other occurrence
around lines 1418-1455).
- Around line 298-312: The useEffect creating the polling interval for
paddleSetup leaks the timer if the component unmounts before window.Paddle
appears; add a cleanup function to always clear the interval on unmount. In the
useEffect that calls loadScript(PADDLE_JS_URL) and sets const interval =
setInterval(paddleSetup, 200), return a cleanup callback that calls
clearInterval(interval) (and optionally nulls it) so the interval is cleared
regardless of whether paddleSetup ever finds window.Paddle; keep existing
paddleSetup, PADDLE_VENDOR_ID, and setLastEvent usage unchanged.
In `@web/app/routes/user-settings.tsx`:
- Around line 474-514: Validate the planId parsed from formData.get('planId')
before calling billing endpoints: in both the 'preview-subscription-update' and
'change-subscription-plan' branches, parse planId and check that
Number.isFinite(planId) (or !Number.isNaN(planId)) and that it's a positive
integer as required; if invalid, return data<UserSettingsActionData>({ intent,
error: 'Invalid planId' }, { status: 400 }) early instead of calling
serverFetch('user/preview-plan'/'user/change-plan'); keep the existing response
shape and header handling (createHeadersWithCookies) for the success path.
🧹 Nitpick comments (3)
web/app/components/pricing/MarketingPricing.tsx (1)
161-171: Align toggle transition with motion guidelines.Add a 200ms ease-out transition and respect reduced-motion preferences on the new toggle button.
As per coding guidelines: Apply default transition of 200ms with ease-out easing for entrances and ease-in for exits; Respect reduced-motion preferences in animations and transitions.🎨 Suggested tweak
- className='flex cursor-pointer items-center gap-2 rounded-lg bg-white/10 px-3 py-2 transition-colors hover:bg-white/20' + className='flex cursor-pointer items-center gap-2 rounded-lg bg-white/10 px-3 py-2 transition-colors duration-200 ease-out motion-reduce:transition-none hover:bg-white/20'web/app/components/pricing/BillingPricing.tsx (1)
318-327: Match toggle transition to 200ms + reduced-motion.Apply the standard transition duration/easing and reduced-motion handling for the new toggle button.
As per coding guidelines: Apply default transition of 200ms with ease-out easing for entrances and ease-in for exits; Respect reduced-motion preferences in animations and transitions.🎨 Suggested tweak
- className='flex cursor-pointer items-center gap-2 rounded-lg bg-gray-100 px-3 py-2 transition-colors hover:bg-gray-200 dark:bg-slate-800 dark:hover:bg-slate-700' + className='flex cursor-pointer items-center gap-2 rounded-lg bg-gray-100 px-3 py-2 transition-colors duration-200 ease-out motion-reduce:transition-none hover:bg-gray-200 dark:bg-slate-800 dark:hover:bg-slate-700'web/app/pages/UserSettings/UserSettings.tsx (1)
182-191: Use primary indigo accent for the new Billing tab icon.The Billing icon color should align with the app’s primary accent.
As per coding guidelines: Use indigo-600 as the primary accent color in light mode and indigo-500 in dark mode.🎨 Suggested tweak
- case TAB_MAPPING.BILLING: - return 'text-emerald-500' + case TAB_MAPPING.BILLING: + return 'text-indigo-600 dark:text-indigo-500'
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
Removed
Localization
✏️ Tip: You can customize this high-level summary in your review settings.