Open
Conversation
…t card details first
|
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:
📝 WalkthroughWalkthroughDefault user plan changed to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Web Frontend
participant Backend
participant Paddle
participant DB as Database
User->>Frontend: Visit app / sign in
Frontend->>Backend: Loader GET (check onboarding & plan)
Backend->>DB: Read user (onboardingStep, planCode)
alt non-self-hosted && planCode is missing or 'none'
Backend-->>Frontend: 302 -> /checkout
Frontend->>User: Show Checkout UI
User->>Frontend: Initiate Paddle checkout
Frontend->>Paddle: Load & open inline checkout (user info)
Paddle-->>Frontend: Checkout success event
Paddle->>Backend: Webhook (subscription created)
Backend->>DB: Update user planCode, trialEndDate
Backend-->>Frontend: Auth reflects updated subscription
Frontend->>User: Redirect to dashboard
else
Backend-->>Frontend: Serve dashboard
end
sequenceDiagram
participant User
participant Frontend as Web Frontend
participant Backend
participant Paddle
participant DB as Database
User->>Frontend: Open Settings -> Cancel subscription
Frontend->>User: Show cancel modal (optional feedback)
User->>Frontend: Submit cancellation
Frontend->>Backend: POST /user/subscription/cancel { feedback? }
Backend->>DB: Verify user & subscription
Backend->>Paddle: POST cancel subscription API
Paddle-->>Backend: Cancellation confirmed
Backend->>DB: Insert CancellationFeedback row
Backend-->>Frontend: 204 No Content
Frontend->>Backend: GET /auth/user (loadUser)
Backend->>DB: Read updated subscription state
Frontend->>User: Show success toast / updated UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
web/app/pages/Onboarding/Onboarding.tsx (1)
1177-1179:⚠️ Potential issue | 🟡 MinorHardcoded, non-localised error string.
'Please, verify your email address first'bypasses the i18n system. All other user-visible strings in this file uset(...).♻️ Proposed fix
- toast.error( - 'Please, verify your email address first', - ) + toast.error(t('onboarding.confirm.notVerifiedError'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Onboarding/Onboarding.tsx` around lines 1177 - 1179, The code in the Onboarding component uses a hardcoded string 'Please, verify your email address first' instead of the i18n helper; find the email-verification check in Onboarding.tsx (inside the Onboarding component / the submit or continue handler where that return occurs) and replace the literal with the localized call using the same translator used elsewhere in the file (e.g., t('Please, verify your email address first') or preferably a new key like t('onboarding.verifyEmailFirst')), adding the key to the translation resource files as needed so the message is localized.web/public/locales/en.json (1)
554-558:⚠️ Potential issue | 🟠 MajorFAQ answer contradicts the new paid-trial flow.
The FAQ item "Is there a free trial?" still states "no credit card required", but this PR specifically requires credit card details before starting a trial. This will confuse users.
Proposed fix
- "a": "Yep, you can try Swetrix for free for {{freeTrialDays}} days, no credit card required!" + "a": "Yep, you can try Swetrix for free for {{freeTrialDays}} days! Just add a payment method to get started — you won't be charged until the trial ends, and you can cancel anytime."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/public/locales/en.json` around lines 554 - 558, Update the FAQ answer for the "Is there a free trial?" item in the locales file: change the translation value associated with the "q": "Is there a free trial?" entry so it no longer says "no credit card required" and instead states that a trial is available for {{freeTrialDays}} days but requires credit card details (or similar phrasing). Locate the object containing that question/answer pair in web/public/locales/en.json and replace the "a" value accordingly to reflect the paid-trial flow.backend/apps/cloud/src/user/user.service.ts (1)
440-443:⚠️ Potential issue | 🔴 CriticalCritical bug in
isPaidTier: comma operator prevents proper plan code validation.The code
[(PlanCode.none, PlanCode.free, PlanCode.trial)]uses the comma operator, which evaluates to onlyPlanCode.trial, creating array['trial']. This allowsPlanCode.noneandPlanCode.freeusers to incorrectly pass the paid-tier check and access restricted features (unnamed projects and shared subscriptions).Fix by removing the parentheses:
Proposed fix
isPaidTier(user: User) { if (!user) { return false } - return ![(PlanCode.none, PlanCode.free, PlanCode.trial)].includes( + return ![PlanCode.none, PlanCode.free, PlanCode.trial].includes( user.planCode, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/user/user.service.ts` around lines 440 - 443, isPaidTier currently constructs the array using the comma operator which collapses to only PlanCode.trial; change the array expression in isPaidTier to list the enum values directly (e.g. [PlanCode.none, PlanCode.free, PlanCode.trial]) so that the includes check correctly tests user.planCode, preserving the existing negation (!...includes(...)) and returning true only for paid plans; update the array literal where isPaidTier is defined to remove the parentheses and commas misuse.
♻️ Duplicate comments (1)
web/app/pages/Checkout/Checkout.tsx (1)
71-74: Cookie withoutSecureflag — low risk for preference data.These cookies store UI preferences (selected plan and billing frequency), not authentication tokens or sensitive data. The CodeQL "sensitive cookie" warning is a false positive for this context. However, adding
Secureis trivial and a good defensive practice for production deployments over HTTPS.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 71 - 74, The cookie assignments inside the useEffect that set document.cookie for swetrix_selected_plan and swetrix_selected_billing (controlled by selectedPlan and selectedBillingFrequency) should include the Secure flag (and optionally SameSite=Lax) so they are only sent over HTTPS; update the two document.cookie strings in Checkout.tsx (the useEffect block) to append "; Secure" (and consider adding "; SameSite=Lax") to each cookie assignment.
🧹 Nitpick comments (9)
backend/migrations/mysql/2026_02_22_trial_with_cc.sql (2)
1-10: No rollback migration provided.The
ALTER TABLEforonboardingStepmodifies enum values — reverting it requires knowing the exact prior enum definition. Consider adding a corresponding down migration or documenting the rollback steps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/migrations/mysql/2026_02_22_trial_with_cc.sql` around lines 1 - 10, Add a down/rollback migration that reverts the changes made: for the ALTER TABLE on the user table revert the onboardingStep enum back to its previous exact definition (update the ALTER TABLE `user` MODIFY COLUMN `onboardingStep` ... back to the original enum values) and drop the newly created cancellation_feedback table (DROP TABLE `cancellation_feedback`); if you don’t have the prior enum values, document the exact prior enum set in the migration comment or source control so the down migration can use the precise enum list when implementing the rollback.
3-10: No index oncancellation_feedback.emailorcreatedAt.If support or analytics queries ever filter by email (e.g., "show all feedback from user X") or by date range, full table scans will occur. Consider adding at least a
createdAtindex now while the table is empty.♻️ Proposed addition
PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + +CREATE INDEX `idx_cancellation_feedback_email` ON `cancellation_feedback` (`email`); +CREATE INDEX `idx_cancellation_feedback_createdAt` ON `cancellation_feedback` (`createdAt`); -) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/migrations/mysql/2026_02_22_trial_with_cc.sql` around lines 3 - 10, The new cancellation_feedback table lacks indexes on email and createdAt which will cause full table scans for common queries; update the migration that creates `cancellation_feedback` (or add a follow-up ALTER TABLE) to add an index on the `email` column (e.g., `idx_cancellation_feedback_email`) and an index on the `createdAt` column (e.g., `idx_cancellation_feedback_created_at`) so queries like "feedback by user" and date-range scans use indexes; ensure the index names are unique and use the same col names `email` and `createdAt` as defined in the CREATE TABLE.backend/apps/cloud/src/user/dto/cancel-subscription.dto.ts (1)
4-10: Consider trimming or rejecting whitespace-only feedback.An empty string or a string of spaces passes
@IsStringand@MaxLength, and would be saved tocancellation_feedback.feedbackas an empty/whitespace row, which adds noise to analytics. Adding@Transformto trim and@IsNotEmpty(or skipping persistence when the trimmed string is blank at the service layer) would prevent this.♻️ Proposed fix
+import { Transform } from 'class-transformer' import { ApiPropertyOptional } from '@nestjs/swagger' -import { IsOptional, IsString, MaxLength } from 'class-validator' +import { IsNotEmpty, IsOptional, IsString, MaxLength } from 'class-validator' export class CancelSubscriptionDTO { `@ApiPropertyOptional`() `@IsOptional`() + `@Transform`(({ value }) => (typeof value === 'string' ? value.trim() : value)) `@IsString`() + `@IsNotEmpty`() `@MaxLength`(2000) feedback?: string }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/user/dto/cancel-subscription.dto.ts` around lines 4 - 10, The feedback field on CancelSubscriptionDTO can be whitespace-only and pass `@IsString/`@MaxLength, so add a `@Transform` to trim the incoming value (e.g., transform to value?.trim()) and then ensure validation treats blank trimmed strings as absent by keeping `@IsOptional`() and adding `@IsNotEmpty`() (so empty strings fail validation or are skipped); alternatively convert trimmed empty string to undefined in the `@Transform` to let `@IsOptional`() skip it. Update the decorators on the feedback property in CancelSubscriptionDTO accordingly and ensure the transform runs before validators.web/app/pages/Auth/Signup/Signup.tsx (1)
202-206: SSO post-sign-up navigates to/dashboardfor users withplanCode === 'none', causing an extra redirect hop.After a successful SSO sign-up where onboarding is already complete but
planCodeis'none', the code navigates toroutes.dashboard. The dashboard loader then immediately redirects to/checkout. The routing chain works correctly but adds a round-trip. Consider aligning with thecomplete-onboardingintent logic by checkingplanCodehere.♻️ Proposed fix
if (!user.hasCompletedOnboarding) { navigate(routes.onboarding) } else { - navigate(routes.dashboard) + navigate( + !isSelfhosted && (!user.planCode || user.planCode === 'none') + ? routes.checkout + : routes.dashboard, + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Auth/Signup/Signup.tsx` around lines 202 - 206, After SSO sign-up, the post-sign-up navigation currently sends users to navigate(routes.dashboard) whenever user.hasCompletedOnboarding is true, which causes an immediate redirect for users with user.planCode === 'none'; update the logic in the Signup post-sign-up block to check user.planCode (e.g., user.planCode === 'none') and, when onboarding is complete but planCode is 'none', navigate directly to the checkout route (navigate(routes.checkout)) instead of routes.dashboard; keep the existing navigate(routes.onboarding) branch for hasCompletedOnboarding === false.web/app/pages/Checkout/Checkout.tsx (2)
199-208: Hardcoded "-17%" discount label.This percentage should be derived from the actual monthly vs. yearly price difference, or at minimum defined as a constant, to prevent it from drifting out of sync with actual pricing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 199 - 208, The "-17%" discount label is hardcoded in Checkout.tsx and can drift from actual prices; update the UI to compute the discount dynamically (or use a named constant) instead of the literal string: derive percentage = (monthlyPrice * 12 - yearlyPrice) / (monthlyPrice * 12) * 100 (or define DISCOUNT_PERCENT = ... in the same module) and render that computed value in place of the "-17%" span; reference the discount span and the selectedBillingFrequency/Switch UI so the displayed discount always reflects the monthlyPrice and yearlyPrice values used by the component.
114-117: Unsafeas anycast for product ID — could silently passundefinedto Paddle.
(tier as any).pid/(tier as any).ypidbypasses type checking. If the selected plan tier doesn't have these properties,productwill beundefined, andPaddle.Checkout.openwill receive an invalid product. Consider adding a guard or typingPLAN_LIMITSentries to includepid/ypid.Proposed guard
const product = selectedBillingFrequency === BillingFrequency.monthly ? (tier as any).pid : (tier as any).ypid + if (!product) { + toast.error('Selected plan is not available. Please choose a different plan.') + return + } + setIsCheckoutOpen(true)web/app/pages/Auth/Signin/Signin.tsx (1)
230-239: Duplicated redirect logic — consider extracting a helper.The same
planCode-based redirect decision (!planCode || 'trial' || 'none'→ checkout, else dashboard) appears twice here and three more times inlogin.tsx. A small helper likegetPostAuthRoute(user, isSelfhosted)would centralise this, making future plan-code changes (e.g. adding a new intermediate state) less error-prone.Also applies to: 313-322
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Auth/Signin/Signin.tsx` around lines 230 - 239, Extract the duplicated planCode redirect logic into a single helper function (e.g. getPostAuthRoute(user, isSelfhosted)) that returns either routes.checkout or routes.dashboard based on user.planCode and isSelfhosted; replace the repeated conditionals in Signin.tsx (the navigate(...) branches) and the similar blocks in login.tsx with navigate(getPostAuthRoute(user, isSelfhosted)); ensure the helper is exported/usable where needed and update any call sites to pass the existing user and isSelfhosted variables so future planCode changes are handled in one place.backend/apps/cloud/src/user/user.controller.ts (1)
739-784: Well-structured cancellation endpoint; consider PII cleanup on account deletion.The endpoint correctly validates the subscription state, cancels via the service, saves feedback non-blockingly, and tracks analytics.
However,
saveCancellationFeedbackpersists the user'sdeleteSelfmethod (line 203) doesn't clean upcancellationFeedbackrecords. Compare withsaveDeleteFeedbackwhich only stores anonymous feedback text. Consider either:
- Anonymising the email in cancellation feedback on account deletion, or
- Removing the email column and relying on the timestamp + planCode for analysis.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/user/user.controller.ts` around lines 739 - 784, The cancellation feedback flow persists PII (user email) via userService.saveCancellationFeedback called from cancelSubscription; ensure cancellationFeedback records are cleaned or anonymised on account deletion by updating the deleteSelf flow (and related service/repository) to either remove associated cancellationFeedback rows or replace the email with an anonymised token/timestamp; alternatively, stop storing email by changing saveCancellationFeedback to persist only non-PII (feedback text, planCode, timestamp) and remove the email column and any usages; update the UserService methods (saveCancellationFeedback, cancelSubscription, deleteSelf) and corresponding DB migration/repository code and tests to reflect the chosen approach.web/app/routes/checkout.tsx (1)
40-40:redirectIfNotAuthenticatedreturn value is silently discarded.The function's signature is
(): Response | null. It throws a redirect when unauthenticated (so the discard is functionally harmless), but dropping the return value without acknowledgement is misleading. Consider prefixing withvoidto signal intentional discard, or — sincegetAuthenticatedUseron the next line already handles auth failure — consider removing the earlyredirectIfNotAuthenticatedcall entirely and handling thenullauthResult explicitly (see comment above).- redirectIfNotAuthenticated(request) + void redirectIfNotAuthenticated(request)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/routes/checkout.tsx` at line 40, The call to redirectIfNotAuthenticated(request) returns Response|null but its value is ignored; either explicitly mark the discard by prefixing the call with void (void redirectIfNotAuthenticated(request)) to signal intentional drop, or remove the call entirely and rely on getAuthenticatedUser(request) to handle auth by checking its result (authResult) and performing a redirect/throw when null; update the code in checkout.tsx around redirectIfNotAuthenticated and getAuthenticatedUser to use one of these approaches and remove the silent-return confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/user/entities/user.entity.ts`:
- Around line 231-235: Add a new migration that alters the database default for
the user entity's planCode column from 'trial' to 'none' so DB behavior matches
the TypeORM entity (enum PlanCode, column planCode). The migration should update
the column default constraint for the users table to use PlanCode.none (not rely
on the old 2025_11_03.sql), include a safe down() to restore 'trial' if rolled
back, and run the migration to ensure raw inserts/omitted fields receive 'none'
by default.
In `@backend/apps/cloud/src/webhook/webhook.controller.ts`:
- Around line 176-178: The code only sets updateParams.trialEndDate when
isTrialing is true, so when a subscription moves from 'trialing' to 'active' the
old trialEndDate remains; update the webhook handling logic in
webhook.controller.ts so that after computing isTrialing and nextBillDate you
explicitly set updateParams.trialEndDate = nextBillDate when isTrialing is true,
and set updateParams.trialEndDate = null when isTrialing is false (i.e., clear
the field on transition out of trial). Ensure you update the same updateParams
object used to persist the subscription change so downstream checks
(UserSettings.tsx / Header/index.tsx) see a null trialEndDate.
In `@web/app/pages/Checkout/Checkout.tsx`:
- Line 312: The JSX in the Checkout component is rendering a hardcoded English
string "Redirecting..." next to {t('common.success')}; replace the literal with
a translation call (e.g. use t('common.redirecting') or a similarly named key)
and add that key to your localization files so all languages show the proper
text; locate the usage in the Checkout component where t is already
imported/used and update the string there.
- Around line 76-95: The Paddle polling loop in the useEffect that calls
loadScript(PADDLE_JS_URL) needs an upper bound so it doesn’t poll forever; add a
max-attempt counter or timeout inside the effect that increments on each
interval tick and clears the interval when exceeded, then set an error/loading
state (e.g. introduce setPaddleLoadError or setIsPaddleLoaded(false)) and stop
calling (window as any).Paddle.Setup or setHasCompletedCheckout; ensure the
created interval is always cleared on success or timeout and that the timeout
branch surfaces an error state to the UI.
In `@web/app/routes/checkout.tsx`:
- Around line 40-53: The current flow calls redirectIfNotAuthenticated then uses
getAuthenticatedUser, but if getAuthenticatedUser returns null we incorrectly
treat the user as unauthenticated-onboarded and redirect to /onboarding with
stale cookies; change the logic in the checkout loader to detect when authResult
is null (or authResult.user is missing) and instead redirect to '/login' while
clearing auth cookies (use the same cookie-clearing/header behavior as
redirectIfNotAuthenticated), i.e., when authResult is null and cookies exist
send redirect('/login', { headers: createHeadersWithCookies([]) }) or the
equivalent cookie-clear headers, otherwise redirect to '/login' without cookies.
- Line 26: The checkout route is using the signup i18n key; update the call to
getDescription so it uses the checkout key (replace t('description.signup') with
t('description.checkout') in the checkout route where getDescription(...) is
invoked) and add the corresponding "description.checkout" entry to your locale
files so the meta description renders correctly for the checkout page.
In `@web/app/routes/login.tsx`:
- Around line 43-44: The loader's plan-check is missing the 'trial' value and
should be updated to match the 2FA and regular login checks; in the loader
function (where isSelfhosted and user.planCode are evaluated) include planCode
=== 'trial' in the redirect-to-checkout condition (or use a single includes
check like ['none','trial'].includes(user.planCode)) so authenticated users on a
'trial' plan are redirected to '/checkout' consistently.
In `@web/public/locales/en.json`:
- Around line 2182-2194: The subtitle at onboarding.selectPlan.subtitle
currently hardcodes "14-day" and should use interpolation to match other
strings; update the JSON value to include an interpolation placeholder (e.g.,
"{{days}}") instead of the literal "14-day" and ensure the code that renders
this string passes the TRIAL_DAYS value (same source used by
checkout.subtitle/checkout.freeTrialAnytime) so the subtitle displays the
runtime TRIAL_DAYS consistently.
---
Outside diff comments:
In `@backend/apps/cloud/src/user/user.service.ts`:
- Around line 440-443: isPaidTier currently constructs the array using the comma
operator which collapses to only PlanCode.trial; change the array expression in
isPaidTier to list the enum values directly (e.g. [PlanCode.none, PlanCode.free,
PlanCode.trial]) so that the includes check correctly tests user.planCode,
preserving the existing negation (!...includes(...)) and returning true only for
paid plans; update the array literal where isPaidTier is defined to remove the
parentheses and commas misuse.
In `@web/app/pages/Onboarding/Onboarding.tsx`:
- Around line 1177-1179: The code in the Onboarding component uses a hardcoded
string 'Please, verify your email address first' instead of the i18n helper;
find the email-verification check in Onboarding.tsx (inside the Onboarding
component / the submit or continue handler where that return occurs) and replace
the literal with the localized call using the same translator used elsewhere in
the file (e.g., t('Please, verify your email address first') or preferably a new
key like t('onboarding.verifyEmailFirst')), adding the key to the translation
resource files as needed so the message is localized.
In `@web/public/locales/en.json`:
- Around line 554-558: Update the FAQ answer for the "Is there a free trial?"
item in the locales file: change the translation value associated with the "q":
"Is there a free trial?" entry so it no longer says "no credit card required"
and instead states that a trial is available for {{freeTrialDays}} days but
requires credit card details (or similar phrasing). Locate the object containing
that question/answer pair in web/public/locales/en.json and replace the "a"
value accordingly to reflect the paid-trial flow.
---
Duplicate comments:
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 71-74: The cookie assignments inside the useEffect that set
document.cookie for swetrix_selected_plan and swetrix_selected_billing
(controlled by selectedPlan and selectedBillingFrequency) should include the
Secure flag (and optionally SameSite=Lax) so they are only sent over HTTPS;
update the two document.cookie strings in Checkout.tsx (the useEffect block) to
append "; Secure" (and consider adding "; SameSite=Lax") to each cookie
assignment.
---
Nitpick comments:
In `@backend/apps/cloud/src/user/dto/cancel-subscription.dto.ts`:
- Around line 4-10: The feedback field on CancelSubscriptionDTO can be
whitespace-only and pass `@IsString/`@MaxLength, so add a `@Transform` to trim the
incoming value (e.g., transform to value?.trim()) and then ensure validation
treats blank trimmed strings as absent by keeping `@IsOptional`() and adding
`@IsNotEmpty`() (so empty strings fail validation or are skipped); alternatively
convert trimmed empty string to undefined in the `@Transform` to let `@IsOptional`()
skip it. Update the decorators on the feedback property in CancelSubscriptionDTO
accordingly and ensure the transform runs before validators.
In `@backend/apps/cloud/src/user/user.controller.ts`:
- Around line 739-784: The cancellation feedback flow persists PII (user email)
via userService.saveCancellationFeedback called from cancelSubscription; ensure
cancellationFeedback records are cleaned or anonymised on account deletion by
updating the deleteSelf flow (and related service/repository) to either remove
associated cancellationFeedback rows or replace the email with an anonymised
token/timestamp; alternatively, stop storing email by changing
saveCancellationFeedback to persist only non-PII (feedback text, planCode,
timestamp) and remove the email column and any usages; update the UserService
methods (saveCancellationFeedback, cancelSubscription, deleteSelf) and
corresponding DB migration/repository code and tests to reflect the chosen
approach.
In `@backend/migrations/mysql/2026_02_22_trial_with_cc.sql`:
- Around line 1-10: Add a down/rollback migration that reverts the changes made:
for the ALTER TABLE on the user table revert the onboardingStep enum back to its
previous exact definition (update the ALTER TABLE `user` MODIFY COLUMN
`onboardingStep` ... back to the original enum values) and drop the newly
created cancellation_feedback table (DROP TABLE `cancellation_feedback`); if you
don’t have the prior enum values, document the exact prior enum set in the
migration comment or source control so the down migration can use the precise
enum list when implementing the rollback.
- Around line 3-10: The new cancellation_feedback table lacks indexes on email
and createdAt which will cause full table scans for common queries; update the
migration that creates `cancellation_feedback` (or add a follow-up ALTER TABLE)
to add an index on the `email` column (e.g., `idx_cancellation_feedback_email`)
and an index on the `createdAt` column (e.g.,
`idx_cancellation_feedback_created_at`) so queries like "feedback by user" and
date-range scans use indexes; ensure the index names are unique and use the same
col names `email` and `createdAt` as defined in the CREATE TABLE.
In `@web/app/pages/Auth/Signin/Signin.tsx`:
- Around line 230-239: Extract the duplicated planCode redirect logic into a
single helper function (e.g. getPostAuthRoute(user, isSelfhosted)) that returns
either routes.checkout or routes.dashboard based on user.planCode and
isSelfhosted; replace the repeated conditionals in Signin.tsx (the navigate(...)
branches) and the similar blocks in login.tsx with
navigate(getPostAuthRoute(user, isSelfhosted)); ensure the helper is
exported/usable where needed and update any call sites to pass the existing user
and isSelfhosted variables so future planCode changes are handled in one place.
In `@web/app/pages/Auth/Signup/Signup.tsx`:
- Around line 202-206: After SSO sign-up, the post-sign-up navigation currently
sends users to navigate(routes.dashboard) whenever user.hasCompletedOnboarding
is true, which causes an immediate redirect for users with user.planCode ===
'none'; update the logic in the Signup post-sign-up block to check user.planCode
(e.g., user.planCode === 'none') and, when onboarding is complete but planCode
is 'none', navigate directly to the checkout route (navigate(routes.checkout))
instead of routes.dashboard; keep the existing navigate(routes.onboarding)
branch for hasCompletedOnboarding === false.
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 199-208: The "-17%" discount label is hardcoded in Checkout.tsx
and can drift from actual prices; update the UI to compute the discount
dynamically (or use a named constant) instead of the literal string: derive
percentage = (monthlyPrice * 12 - yearlyPrice) / (monthlyPrice * 12) * 100 (or
define DISCOUNT_PERCENT = ... in the same module) and render that computed value
in place of the "-17%" span; reference the discount span and the
selectedBillingFrequency/Switch UI so the displayed discount always reflects the
monthlyPrice and yearlyPrice values used by the component.
In `@web/app/routes/checkout.tsx`:
- Line 40: The call to redirectIfNotAuthenticated(request) returns Response|null
but its value is ignored; either explicitly mark the discard by prefixing the
call with void (void redirectIfNotAuthenticated(request)) to signal intentional
drop, or remove the call entirely and rely on getAuthenticatedUser(request) to
handle auth by checking its result (authResult) and performing a redirect/throw
when null; update the code in checkout.tsx around redirectIfNotAuthenticated and
getAuthenticatedUser to use one of these approaches and remove the silent-return
confusion.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
admin/src/entities/user.entity.tsbackend/apps/cloud/src/common/templates/en/customer_sign-up.htmlbackend/apps/cloud/src/common/templates/en/trial-ends-tomorrow.htmlbackend/apps/cloud/src/mailer/mailer.service.tsbackend/apps/cloud/src/task-manager/task-manager.service.tsbackend/apps/cloud/src/user/dto/cancel-subscription.dto.tsbackend/apps/cloud/src/user/entities/cancellation-feedback.entity.tsbackend/apps/cloud/src/user/entities/user.entity.tsbackend/apps/cloud/src/user/user.controller.tsbackend/apps/cloud/src/user/user.module.tsbackend/apps/cloud/src/user/user.service.tsbackend/apps/cloud/src/webhook/webhook.controller.tsbackend/migrations/mysql/2026_02_22_trial_with_cc.sqlweb/app/components/Header/index.tsxweb/app/pages/Auth/Signin/Signin.tsxweb/app/pages/Auth/Signup/Signup.tsxweb/app/pages/Checkout/Checkout.tsxweb/app/pages/Checkout/index.tsxweb/app/pages/Onboarding/Onboarding.tsxweb/app/pages/UserSettings/UserSettings.tsxweb/app/routes/checkout.tsxweb/app/routes/dashboard.tsxweb/app/routes/login.tsxweb/app/routes/onboarding.tsxweb/app/routes/signup.tsxweb/app/routes/user-settings.tsxweb/app/utils/routes.tsweb/public/locales/en.json
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
web/app/pages/Checkout/Checkout.tsx (2)
298-299: “Redirecting…” is still hardcoded (already raised).Please replace with a translation key and update locale files accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 298 - 299, Replace the hardcoded "Redirecting..." literal inside the Text element with a translation lookup (use the existing i18n t function) instead of plain text; for example, call t('checkout.redirecting') where the current hardcoded string appears alongside t('common.success'), and add a matching key "checkout.redirecting" to your locale files (e.g., en.json and any other supported locales) with the appropriate translated strings.
62-78: Paddle polling still lacks a timeout (already raised).This keeps polling forever if the script never loads (ad‑blockers/network errors). Please apply the previously suggested max‑attempt/timeout with an error state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 62 - 78, The polling loop in useEffect that calls loadScript(PADDLE_JS_URL) and checks (window as any)?.Paddle currently never times out; add a max-attempts or timeout mechanism to stop polling and set an error state (e.g., setPaddleLoadError / setIsPaddleLoaded(false)) if Paddle doesn't appear within the limit, ensure you clearInterval(interval) on both success (where you call (window as any).Paddle.Setup with PADDLE_VENDOR_ID and eventCallback that sets setHasCompletedCheckout) and on timeout, and log or surface the failure so the UI can show an error instead of polling forever.
🧹 Nitpick comments (1)
web/app/pages/Checkout/Checkout.tsx (1)
55-60: Avoid a hardcoded “-17%” badge.If pricing changes or varies by plan/currency, this can become inaccurate. Compute it from monthly/yearly prices (or hide if unavailable).
♻️ Suggested refactor
const displayPrice = selectedBillingFrequency === 'yearly' ? yearlyPrice : monthlyPrice + const yearlyDiscountPct = + monthlyPrice && yearlyPrice + ? Math.round((1 - yearlyPrice / (monthlyPrice * 12)) * 100) + : null- <span className='rounded-md bg-emerald-100 px-1.5 py-0.5 text-xs font-semibold text-emerald-700 dark:bg-emerald-500/20 dark:text-emerald-300'> - -17% - </span> + {yearlyDiscountPct && yearlyDiscountPct > 0 ? ( + <span className='rounded-md bg-emerald-100 px-1.5 py-0.5 text-xs font-semibold text-emerald-700 dark:bg-emerald-500/20 dark:text-emerald-300'> + -{yearlyDiscountPct}% + </span> + ) : null}Also applies to: 185-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 55 - 60, The UI currently shows a hardcoded “-17%” badge; instead compute a dynamic discount using monthlyPrice and yearlyPrice (both defined above) by comparing (monthlyPrice * 12) to yearlyPrice and deriving percent = round((1 - yearlyPrice / (monthlyPrice * 12)) * 100), and only render the badge when both prices are present and percent > 0 (or hide it if prices are unavailable); update any other occurrences (e.g., around lines 185-190) that currently show the hardcoded value to use this computed percent and ensure selectedBillingFrequency/displayPrice logic remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 93-95: Replace the hardcoded toast message inside the
payment-loading guard in Checkout.tsx (the block checking isPaddleLoaded and
(window as any).Paddle) with a localized string using the t(...) i18n function
(e.g. t('checkout.paymentLoading')), and ensure the component has access to the
t function (via the existing i18n hook or import) and that a corresponding
translation key (checkout.paymentLoading) is added to the locales; update the
toast.error call to use t(...) instead of the literal string so the message is
localizable.
- Around line 32-34: formatEventsLong currently hardcodes 'en-US' which forces
US number formatting; change it to use the component's locale (i18n.language)
instead — either by making formatEventsLong accept a locale parameter (e.g.
formatEventsLong(value, locale)) and passing i18n.language from the Checkout
component, or by reading i18n.language inside the function via the
useTranslation hook; ensure you keep a sensible fallback (like 'en-US') if
i18n.language is undefined; update all call sites to pass the locale if you
choose the parameter approach and leave INITIAL_VISIBLE_PLANS unchanged.
- Around line 92-118: handleStartCheckout sets setIsCheckoutOpen(true) before
calling (window as any).Paddle.Checkout.open, which can throw and leave the UI
stuck; wrap the Paddle.Checkout.open call in a try-catch inside
handleStartCheckout, call setIsCheckoutOpen(false) in the catch (and optionally
after a failed open), show an error toast (e.g., toast.error with the caught
error.message) so the button/modal can be retried, and only keep
setIsCheckoutOpen(true) if the open call succeeds; reference
handleStartCheckout, setIsCheckoutOpen, and (window as any).Paddle.Checkout.open
when making the change.
---
Duplicate comments:
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 298-299: Replace the hardcoded "Redirecting..." literal inside the
Text element with a translation lookup (use the existing i18n t function)
instead of plain text; for example, call t('checkout.redirecting') where the
current hardcoded string appears alongside t('common.success'), and add a
matching key "checkout.redirecting" to your locale files (e.g., en.json and any
other supported locales) with the appropriate translated strings.
- Around line 62-78: The polling loop in useEffect that calls
loadScript(PADDLE_JS_URL) and checks (window as any)?.Paddle currently never
times out; add a max-attempts or timeout mechanism to stop polling and set an
error state (e.g., setPaddleLoadError / setIsPaddleLoaded(false)) if Paddle
doesn't appear within the limit, ensure you clearInterval(interval) on both
success (where you call (window as any).Paddle.Setup with PADDLE_VENDOR_ID and
eventCallback that sets setHasCompletedCheckout) and on timeout, and log or
surface the failure so the UI can show an error instead of polling forever.
---
Nitpick comments:
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 55-60: The UI currently shows a hardcoded “-17%” badge; instead
compute a dynamic discount using monthlyPrice and yearlyPrice (both defined
above) by comparing (monthlyPrice * 12) to yearlyPrice and deriving percent =
round((1 - yearlyPrice / (monthlyPrice * 12)) * 100), and only render the badge
when both prices are present and percent > 0 (or hide it if prices are
unavailable); update any other occurrences (e.g., around lines 185-190) that
currently show the hardcoded value to use this computed percent and ensure
selectedBillingFrequency/displayPrice logic remains unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/migrations/mysql/2026_02_22_trial_with_cc.sqlweb/app/pages/Checkout/Checkout.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/migrations/mysql/2026_02_22_trial_with_cc.sql
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
web/app/pages/Checkout/Checkout.tsx (2)
290-290:⚠️ Potential issue | 🟡 MinorHardcoded English string
"Redirecting..."— use i18n.The rest of the component uses
t(...)for translations.Proposed fix
- {t('common.success')}! Redirecting... + {t('common.success')}! {t('common.redirecting')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` at line 290, Replace the hardcoded "Redirecting..." string with a call to the i18n translator used in this component (e.g., use t('common.redirecting') or t('checkout.redirecting')) inside the JSX in Checkout (Checkout.tsx); also add the corresponding key and translations to your locale JSON files so the text is available in all languages.
97-110:⚠️ Potential issue | 🟠 MajorWrap
Paddle.Checkout.open()in a try-catch to prevent stuck UI.
setIsCheckoutOpen(true)runs before the Paddle call. IfPaddle.Checkout.open()throws, the button stays hidden and users can't retry. This was flagged previously and appears to still be unaddressed.Proposed fix
setIsCheckoutOpen(true) + try { ;(window as any).Paddle.Checkout.open({ product, method: 'inline', frameTarget: 'checkout-container', frameInitialHeight: 416, frameStyle: 'width:100%; min-width:312px; background-color: transparent; border: none; border-radius: 10px;', email: user?.email, passthrough: JSON.stringify({ uid: user?.id }), locale: paddleLanguageMapping[i18n.language] || i18n.language, displayModeTheme: theme, country: metainfo.country, }) + } catch (err) { + setIsCheckoutOpen(false) + toast.error(t('billing.paddleLoadError')) + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 97 - 110, The call to Paddle.Checkout.open is unprotected so if it throws after setIsCheckoutOpen(true) the UI stays in "open" state; wrap the Paddle.Checkout.open(...) invocation inside a try-catch around the existing call in Checkout.tsx (the block that calls setIsCheckoutOpen(true) and (window as any).Paddle.Checkout.open) and in the catch setIsCheckoutOpen(false) (or otherwise restore UI state) and log or surface the error so the button becomes usable again; ensure you only wrap the Paddle.Checkout.open call (leave setIsCheckoutOpen call placement intact) and reference the same paddleLanguageMapping, passthrough and props when invoking the call inside the try block.
🧹 Nitpick comments (1)
web/app/pages/UserSettings/UserSettings.tsx (1)
1803-1811: Raw<textarea>inconsistent with the rest of the file.The account-deletion modal at Line 1728 uses the project's
<Textarea>component, but this cancellation feedback uses a raw<textarea>with inline Tailwind classes. Consider using the<Textarea>component for visual consistency and to inherit any shared styling/behavior (dark mode, focus rings, etc.).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/UserSettings/UserSettings.tsx` around lines 1803 - 1811, Replace the raw HTML <textarea> used for cancellation feedback with the project's <Textarea> component to ensure consistent styling/behavior (dark mode, focus rings, etc.); in the UserSettings component update the element at the cancellation-feedback id to use <Textarea> and pass the same props (id='cancellation-feedback', rows={3}, placeholder={t('billing.cancellationFeedbackPlaceholder')}, value={cancellationFeedback}, onChange={(e) => setCancellationFeedback(e.target.value)}, disabled={isCancellingSubscription}) and remove the inline Tailwind className so the shared <Textarea> styling is applied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/app/hooks/usePaddle.ts`:
- Around line 24-51: The usePaddle hook currently calls Paddle.Setup per
instance which allows later mounts to overwrite the global Paddle eventCallback;
change usePaddle to a singleton pattern by moving setup logic out of the
instance effect into module-level initialization that calls
loadScript(PADDLE_JS_URL) once and calls (window as any).Paddle.Setup only once;
maintain a module-level array of listeners and expose subscribe/unsubscribe
functions that use onEventRef (or similar) so each hook instance registers its
handler and receives events; ensure setIsPaddleLoaded and setPaddleLoadError are
driven by the shared initialization state so individual useEffect hooks only
subscribe/unsubscribe instead of re-calling Paddle.Setup.
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 92-95: The code uses unsafe casts (tier as any).pid/ypid to set
product which can be undefined and break Paddle.Checkout.open; update the
product assignment in Checkout.tsx to perform a runtime guard on tier for the
expected properties (check selectedBillingFrequency === BillingFrequency.monthly
? 'pid' in tier && typeof (tier as any).pid === 'string' : 'ypid' in tier &&
typeof (tier as any).ypid === 'string'), then set product to that value only if
present; if missing, log or surface an error and avoid calling
Paddle.Checkout.open (or use a safe fallback SKU), and update the Tier type (or
make pid/ypid optional) so the compiler reflects the runtime check (referencing
product, selectedBillingFrequency, BillingFrequency, tier, pid, ypid, and the
Paddle.Checkout.open call).
- Around line 119-121: The page currently recalculates trialEndDate client-side
using TRIAL_DAYS (variables trialEndDate and today); instead, read and use the
server-provided value user.trialEndDate when available. Update the Checkout
component to set trialEndDate = new Date(user.trialEndDate) if
user?.trialEndDate exists, and only fall back to computing today + TRIAL_DAYS
when the server value is absent; ensure you reference the existing trialEndDate
and TRIAL_DAYS symbols and keep date conversion via new Date(...) to preserve
correct formatting.
- Around line 50-54: The onPaddleEvent handler only handles 'Checkout.Complete'
so closing the Paddle modal leaves isCheckoutOpen true; update the onPaddleEvent
function to also handle eventData.event === 'Checkout.Close' and call
setIsCheckoutOpen(false) (and optionally reset setHasCompletedCheckout(false) if
you want to clear completion state), and add setIsCheckoutOpen (and
setHasCompletedCheckout if used) to the useCallback dependency array so the
handler uses the latest setters; the symbols to change are onPaddleEvent,
setIsCheckoutOpen, and setHasCompletedCheckout.
---
Duplicate comments:
In `@web/app/pages/Checkout/Checkout.tsx`:
- Line 290: Replace the hardcoded "Redirecting..." string with a call to the
i18n translator used in this component (e.g., use t('common.redirecting') or
t('checkout.redirecting')) inside the JSX in Checkout (Checkout.tsx); also add
the corresponding key and translations to your locale JSON files so the text is
available in all languages.
- Around line 97-110: The call to Paddle.Checkout.open is unprotected so if it
throws after setIsCheckoutOpen(true) the UI stays in "open" state; wrap the
Paddle.Checkout.open(...) invocation inside a try-catch around the existing call
in Checkout.tsx (the block that calls setIsCheckoutOpen(true) and (window as
any).Paddle.Checkout.open) and in the catch setIsCheckoutOpen(false) (or
otherwise restore UI state) and log or surface the error so the button becomes
usable again; ensure you only wrap the Paddle.Checkout.open call (leave
setIsCheckoutOpen call placement intact) and reference the same
paddleLanguageMapping, passthrough and props when invoking the call inside the
try block.
---
Nitpick comments:
In `@web/app/pages/UserSettings/UserSettings.tsx`:
- Around line 1803-1811: Replace the raw HTML <textarea> used for cancellation
feedback with the project's <Textarea> component to ensure consistent
styling/behavior (dark mode, focus rings, etc.); in the UserSettings component
update the element at the cancellation-feedback id to use <Textarea> and pass
the same props (id='cancellation-feedback', rows={3},
placeholder={t('billing.cancellationFeedbackPlaceholder')},
value={cancellationFeedback}, onChange={(e) =>
setCancellationFeedback(e.target.value)}, disabled={isCancellingSubscription})
and remove the inline Tailwind className so the shared <Textarea> styling is
applied.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/app/hooks/usePaddle.tsweb/app/pages/Checkout/Checkout.tsxweb/app/pages/UserSettings/UserSettings.tsxweb/public/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- web/public/locales/en.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/public/locales/en.json (1)
556-558:⚠️ Potential issue | 🟡 MinorFAQ text contradicts the new paid-trial checkout flow.
The FAQ answer states "no credit card required!" but the new checkout flow (
checkout.title,onboarding.selectPlan.subtitle,billing.trialChargeWarning) explicitly requires payment details upfront and warns about automatic charges. This will confuse users.The same issue exists at line 2074 in
pricing.adv: "No credit card required. No strings attached."Both strings should be updated to reflect the paid-trial model (e.g., "Your card won't be charged during the trial").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/public/locales/en.json` around lines 556 - 558, Update the FAQ and pricing copy to reflect the paid-trial checkout flow: change the FAQ answer for the entry with q "Is there a free trial?" (key under the FAQ block shown) to indicate the card is collected but not charged during the trial (e.g., "Your card won't be charged during the {{freeTrialDays}}-day trial; you'll be billed automatically afterward unless you cancel"), and also update the "pricing.adv" string that currently reads "No credit card required. No strings attached." to the same paid-trial wording; ensure the wording aligns with related keys checkout.title, onboarding.selectPlan.subtitle, and billing.trialChargeWarning so messaging is consistent about collecting payment details upfront but not charging during the trial.
♻️ Duplicate comments (5)
web/app/pages/Checkout/Checkout.tsx (3)
120-122:⚠️ Potential issue | 🟡 MinorTrial end date is recalculated client-side instead of using the server value.
For users who revisit the checkout page after their trial has already started,
today + TRIAL_DAYSwill show a later date than the actual trial end. The server providesuser.trialEndDate— prefer that when available, falling back to the local calculation for new users.- const today = new Date() - const trialEndDate = new Date() - trialEndDate.setDate(today.getDate() + TRIAL_DAYS) + const trialEndDate = user?.trialEndDate + ? new Date(user.trialEndDate) + : (() => { + const d = new Date() + d.setDate(d.getDate() + TRIAL_DAYS) + return d + })()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 120 - 122, Replace the client-only calculation of trialEndDate with the server-provided value when available: if user.trialEndDate exists, parse it (e.g., new Date(user.trialEndDate)) and use that for trialEndDate; otherwise fall back to the existing calculation using today and TRIAL_DAYS. Update the code that currently defines today, trialEndDate (and any downstream uses) to prefer parsed user.trialEndDate to ensure revisiting users see the real trial end date.
93-111:⚠️ Potential issue | 🟠 MajorNo error handling around
Paddle.Checkout.open()— UI can get stuck.Two unaddressed issues from prior review remain here:
No
productguard:(tier as any).pid/(tier as any).ypidcan beundefinedif the plan tier doesn't carry these Paddle IDs, causing a silent failure or exception.No try-catch: If
Paddle.Checkout.open()throws,isCheckoutOpenremainstrueand the button stays hidden.Proposed fix
const product = selectedBillingFrequency === BillingFrequency.monthly ? (tier as any).pid : (tier as any).ypid + if (!product) { + toast.error(t('apiNotifications.somethingWentWrong')) + return + } + setIsCheckoutOpen(true) - ;(window as any).Paddle.Checkout.open({ + try { + ;(window as any).Paddle.Checkout.open({ ... - }) + }) + } catch (err) { + setIsCheckoutOpen(false) + toast.error(t('billing.paddleLoadError')) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 93 - 111, The code currently sets setIsCheckoutOpen(true) before calling (window as any).Paddle.Checkout.open and uses (tier as any).pid / ypid without validating them; add a guard that resolves the correct productId from tier (use selectedBillingFrequency, BillingFrequency.monthly, and tier.pid / tier.ypid) and if productId is falsy return early and do not change UI, then wrap Paddle.Checkout.open(...) in a try/catch/finally so setIsCheckoutOpen(true) is only set if open succeeds (or set it optimistically but ensure in catch you setIsCheckoutOpen(false) and log the error), and include safe fallback logging for errors to avoid leaving the checkout button hidden; target symbols: product variable, setIsCheckoutOpen, Paddle.Checkout.open, selectedBillingFrequency, BillingFrequency.monthly, and tier.pid/ypid.
51-55:⚠️ Potential issue | 🟠 MajorMissing
Checkout.Closeevent handler — user gets stuck if they dismiss the Paddle modal.When the user closes the Paddle overlay without completing payment,
isCheckoutOpenstaystrue, which hides the "Start free trial" button permanently. Handle the close event to reset state:const onPaddleEvent = useCallback((eventData: any) => { if (eventData?.event === 'Checkout.Complete') { setHasCompletedCheckout(true) + } else if (eventData?.event === 'Checkout.Close') { + setIsCheckoutOpen(false) } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 51 - 55, The onPaddleEvent callback only handles 'Checkout.Complete' and doesn't reset UI when the user dismisses the Paddle modal; update onPaddleEvent to also handle eventData.event === 'Checkout.Close' and in that branch call the state setters to revert the checkout UI (e.g., setIsCheckoutOpen(false) and setHasCompletedCheckout(false) or whatever the local setters are named) so the "Start free trial" button is shown again after the overlay is closed; keep both branches inside the same useCallback (onPaddleEvent) so the component responds to both complete and close events.web/app/routes/checkout.tsx (1)
45-54:⚠️ Potential issue | 🟠 MajorNull user still incorrectly redirected to
/onboarding— potential redirect loop.When
authResult?.user?.userisnull(e.g. expired tokens that passed the shallow check),!user?.hasCompletedOnboardingevaluates totrue, sending the user to/onboarding. If/onboardingperforms a similar auth check and redirects back, this creates a loop.The safe fallback is to redirect to
/loginand clear stale cookies whenuseris null:const user = authResult?.user?.user + if (!user) { + return redirect('/login', { + headers: cookies.length > 0 ? createHeadersWithCookies(cookies) : undefined, + }) + } + - if (!user?.hasCompletedOnboarding) { + if (!user.hasCompletedOnboarding) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/routes/checkout.tsx` around lines 45 - 54, The current logic treats a null authResult?.user?.user as an incomplete onboarding and redirects to '/onboarding', which can cause a redirect loop; update the check in the checkout route to explicitly handle a null user: if user is null (authResult?.user?.user === null) then redirect to '/login' and clear stale cookies via createHeadersWithCookies([]) (or equivalent) instead of sending them to '/onboarding'; only fall back to redirect('/onboarding') when user exists but hasCompletedOnboarding is false.web/public/locales/en.json (1)
2185-2197: Previous hardcoded "14-day" issue is resolved — uses{{days}}interpolation now.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/public/locales/en.json` around lines 2185 - 2197, The hardcoded "14-day" text was replaced with interpolation "{{days}}" in the selectPlan block; confirm the change is applied to selectPlan.title/subtitle and timeline keys (timeline.todayDesc, timeline.reminder, timeline.reminderDesc, timeline.charge, timeline.chargeDesc) across all locale files and update any remaining duplicates or stale entries, then run your i18n extraction/validation step to ensure no translation keys expect the old hardcoded string before merging.
🧹 Nitpick comments (1)
web/app/pages/Checkout/Checkout.tsx (1)
178-183: Hardcoded "-17%" discount badge will drift if pricing changes.The yearly discount percentage is hardcoded in the UI. If plan prices are updated, this label won't reflect the actual savings. Consider computing it from the monthly/yearly price data, or at minimum extracting it as a named constant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Checkout/Checkout.tsx` around lines 178 - 183, The "-17%" badge is hardcoded and will become incorrect if prices change; replace it by computing the percentage savings from the monthly and yearly price values used to render the page (e.g., compute savings = Math.round((1 - yearlyPrice / (monthlyPrice * 12)) * 100) and format as `-{savings}%`) and render that result instead of the literal "-17%". Update the Checkout component code that renders the badge (the span next to t('pricing.billedYearly')) to read the monthly/yearly price variables (or pricing.monthly/pricing.yearly) and fall back to a named constant only if those values are unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/public/locales/en.json`:
- Around line 556-558: Update the FAQ and pricing copy to reflect the paid-trial
checkout flow: change the FAQ answer for the entry with q "Is there a free
trial?" (key under the FAQ block shown) to indicate the card is collected but
not charged during the trial (e.g., "Your card won't be charged during the
{{freeTrialDays}}-day trial; you'll be billed automatically afterward unless you
cancel"), and also update the "pricing.adv" string that currently reads "No
credit card required. No strings attached." to the same paid-trial wording;
ensure the wording aligns with related keys checkout.title,
onboarding.selectPlan.subtitle, and billing.trialChargeWarning so messaging is
consistent about collecting payment details upfront but not charging during the
trial.
---
Duplicate comments:
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 120-122: Replace the client-only calculation of trialEndDate with
the server-provided value when available: if user.trialEndDate exists, parse it
(e.g., new Date(user.trialEndDate)) and use that for trialEndDate; otherwise
fall back to the existing calculation using today and TRIAL_DAYS. Update the
code that currently defines today, trialEndDate (and any downstream uses) to
prefer parsed user.trialEndDate to ensure revisiting users see the real trial
end date.
- Around line 93-111: The code currently sets setIsCheckoutOpen(true) before
calling (window as any).Paddle.Checkout.open and uses (tier as any).pid / ypid
without validating them; add a guard that resolves the correct productId from
tier (use selectedBillingFrequency, BillingFrequency.monthly, and tier.pid /
tier.ypid) and if productId is falsy return early and do not change UI, then
wrap Paddle.Checkout.open(...) in a try/catch/finally so setIsCheckoutOpen(true)
is only set if open succeeds (or set it optimistically but ensure in catch you
setIsCheckoutOpen(false) and log the error), and include safe fallback logging
for errors to avoid leaving the checkout button hidden; target symbols: product
variable, setIsCheckoutOpen, Paddle.Checkout.open, selectedBillingFrequency,
BillingFrequency.monthly, and tier.pid/ypid.
- Around line 51-55: The onPaddleEvent callback only handles 'Checkout.Complete'
and doesn't reset UI when the user dismisses the Paddle modal; update
onPaddleEvent to also handle eventData.event === 'Checkout.Close' and in that
branch call the state setters to revert the checkout UI (e.g.,
setIsCheckoutOpen(false) and setHasCompletedCheckout(false) or whatever the
local setters are named) so the "Start free trial" button is shown again after
the overlay is closed; keep both branches inside the same useCallback
(onPaddleEvent) so the component responds to both complete and close events.
In `@web/app/routes/checkout.tsx`:
- Around line 45-54: The current logic treats a null authResult?.user?.user as
an incomplete onboarding and redirects to '/onboarding', which can cause a
redirect loop; update the check in the checkout route to explicitly handle a
null user: if user is null (authResult?.user?.user === null) then redirect to
'/login' and clear stale cookies via createHeadersWithCookies([]) (or
equivalent) instead of sending them to '/onboarding'; only fall back to
redirect('/onboarding') when user exists but hasCompletedOnboarding is false.
In `@web/public/locales/en.json`:
- Around line 2185-2197: The hardcoded "14-day" text was replaced with
interpolation "{{days}}" in the selectPlan block; confirm the change is applied
to selectPlan.title/subtitle and timeline keys (timeline.todayDesc,
timeline.reminder, timeline.reminderDesc, timeline.charge, timeline.chargeDesc)
across all locale files and update any remaining duplicates or stale entries,
then run your i18n extraction/validation step to ensure no translation keys
expect the old hardcoded string before merging.
---
Nitpick comments:
In `@web/app/pages/Checkout/Checkout.tsx`:
- Around line 178-183: The "-17%" badge is hardcoded and will become incorrect
if prices change; replace it by computing the percentage savings from the
monthly and yearly price values used to render the page (e.g., compute savings =
Math.round((1 - yearlyPrice / (monthlyPrice * 12)) * 100) and format as
`-{savings}%`) and render that result instead of the literal "-17%". Update the
Checkout component code that renders the badge (the span next to
t('pricing.billedYearly')) to read the monthly/yearly price variables (or
pricing.monthly/pricing.yearly) and fall back to a named constant only if those
values are unavailable.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
web/app/routes/login.tsx (1)
43-44: Missing'trial'planCode in all three redirect checks.All three redirect paths (loader line 43, 2FA action line 118, regular login line 171) only gate on
'none', but'trial'is the exact plan code this "paid trials" PR introduces. If the intent is that trial-plan users must also complete checkout, all three conditions need to include it. They are at least consistently omitting it, but that consistency may itself be a bug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/routes/login.tsx` around lines 43 - 44, The redirect checks in web/app/routes/login.tsx (the loader, the 2FA action, and the regular login flow) currently only treat user.planCode === 'none' as requiring checkout; add the 'trial' plan to those gates so trial users are redirected as well (e.g., change the condition that checks user.planCode === 'none' to also test for user.planCode === 'trial' or test for an allowedPlans set). Update the three locations referenced (the loader, the 2FA action, and the normal login branch) to include the 'trial' case in their redirect logic to /checkout.web/app/pages/Auth/Signin/Signin.tsx (1)
230-234: Missing'trial'planCode — same gap as inlogin.tsx.Both SSO-login and account-linking redirect branches apply the same
!user.planCode || user.planCode === 'none'condition, which is also missing'trial'. This is consistent withlogin.tsxbut the omission is duplicated here too.Also applies to: 311-315
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Auth/Signin/Signin.tsx` around lines 230 - 234, The redirect condition in Signin.tsx that sends users to routes.checkout treats missing plans and 'none' but omits 'trial'; update the conditional used in the SSO-login/account-linking branches (the block around the navigate(routes.checkout) check) to also consider user.planCode === 'trial' (e.g., change !user.planCode || user.planCode === 'none' to include || user.planCode === 'trial'), and make the same change in the duplicate location later in the file (the second occurrence around lines 311-315) so both branches handle 'trial' consistently.
🧹 Nitpick comments (1)
web/app/routes/login.tsx (1)
41-45: Extract the repeated redirect-decision logic into a shared helper.The same three-branch
hasCompletedOnboarding → planCode → dashboarddecision is duplicated three times in this file and twice more inSignin.tsx(lines 228–237 and 309–318), making future plan-code additions error-prone (as the missing'trial'check already illustrates).♻️ Proposed helper (add to a shared util, e.g.
~/utils/auth.ts)+// ~/utils/auth.ts +import { isSelfhosted } from '~/lib/constants' + +const PAID_PLAN_CODES = new Set(['none', 'trial']) + +export function getPostLoginRedirect(user: { + hasCompletedOnboarding: boolean + planCode?: string +}): '/onboarding' | '/checkout' | '/dashboard' { + if (!user.hasCompletedOnboarding) return '/onboarding' + if (!isSelfhosted && (!user.planCode || PAID_PLAN_CODES.has(user.planCode))) { + return '/checkout' + } + return '/dashboard' +}Then in
login.tsxloader, 2FA action, and regular login action — and inSignin.tsxSSO/linking flows — replace each block with:- if (!user.hasCompletedOnboarding) { - redirectTo = '/onboarding' - } else if (!isSelfhosted && (!user.planCode || user.planCode === 'none')) { - redirectTo = '/checkout' - } + redirectTo = getPostLoginRedirect(user)Also applies to: 112-122, 164-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/routes/login.tsx` around lines 41 - 45, Extract the repeated three-branch redirect logic into a single shared helper (e.g. decidePostAuthRedirect(user, isSelfhosted) in a common utils/auth module) that returns the appropriate path ('/onboarding', '/checkout', or '/dashboard') based on user.hasCompletedOnboarding and valid planCode values; then replace the duplicated blocks in login.tsx (loader, 2FA action, regular login action) and the duplicated branches in Signin.tsx (SSO/linking flows) to call decidePostAuthRedirect and use its result for redirect. Ensure the helper treats planCode values consistently (include 'none', 'trial', etc.) and update all callsites to import and use the helper so the logic is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@web/app/pages/Auth/Signin/Signin.tsx`:
- Around line 230-234: The redirect condition in Signin.tsx that sends users to
routes.checkout treats missing plans and 'none' but omits 'trial'; update the
conditional used in the SSO-login/account-linking branches (the block around the
navigate(routes.checkout) check) to also consider user.planCode === 'trial'
(e.g., change !user.planCode || user.planCode === 'none' to include ||
user.planCode === 'trial'), and make the same change in the duplicate location
later in the file (the second occurrence around lines 311-315) so both branches
handle 'trial' consistently.
In `@web/app/routes/login.tsx`:
- Around line 43-44: The redirect checks in web/app/routes/login.tsx (the
loader, the 2FA action, and the regular login flow) currently only treat
user.planCode === 'none' as requiring checkout; add the 'trial' plan to those
gates so trial users are redirected as well (e.g., change the condition that
checks user.planCode === 'none' to also test for user.planCode === 'trial' or
test for an allowedPlans set). Update the three locations referenced (the
loader, the 2FA action, and the normal login branch) to include the 'trial' case
in their redirect logic to /checkout.
---
Nitpick comments:
In `@web/app/routes/login.tsx`:
- Around line 41-45: Extract the repeated three-branch redirect logic into a
single shared helper (e.g. decidePostAuthRedirect(user, isSelfhosted) in a
common utils/auth module) that returns the appropriate path ('/onboarding',
'/checkout', or '/dashboard') based on user.hasCompletedOnboarding and valid
planCode values; then replace the duplicated blocks in login.tsx (loader, 2FA
action, regular login action) and the duplicated branches in Signin.tsx
(SSO/linking flows) to call decidePostAuthRedirect and use its result for
redirect. Ensure the helper treats planCode values consistently (include 'none',
'trial', etc.) and update all callsites to import and use the helper so the
logic is centralized.
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.
Changes
Related:
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Content & Messaging