feat: add subscription management to settings panel #2868
Conversation
Add a new Subscription tab in the settings panel that allows users to: - View their current subscription plan and status - Upgrade or change their subscription plan - Cancel their subscription with a confirmation modal - Manage billing through Stripe customer portal The implementation includes: - New subscription-tab component with plan management UI - Integration with existing subscription data and modals - Dropdown menus for subscription and payment management - Cancel subscription confirmation dialog (UI only) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a Subscription tab and related components, simplifies Preferences to a UserDeleteSection, unifies settings modal controls, adds a user-delete API and admin Supabase client, updates env schema, introduces FK cascade constraints in DB schemas, adjusts seeded Message shape, and adds a devDependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Avatar as AvatarDropdown
participant State as stateManager
participant Modal as SettingsModal
participant Tabs as TabsRenderer
participant Sub as SubscriptionTab
participant API as TRPC Backend
participant Stripe as Stripe Billing Portal
participant Auth as Client Auth
User->>Avatar: Click "Subscription" or "Settings"
Avatar->>State: settingsTab = SUBSCRIPTION / PREFERENCES\nisSettingsModalOpen = true
State->>Modal: open with active tab
Modal->>Tabs: render active tab
alt Subscription tab active
Tabs->>Sub: mount SubscriptionTab
Sub->>API: query subscription data
User->>Sub: Click "Manage" (billing portal)
Sub->>API: mutation create billing portal session
API-->>Sub: returns portalUrl
Sub->>Stripe: open portalUrl (new tab)
User->>Sub: Click "Cancel subscription"
Sub->>Sub: show SubscriptionCancelModal (local)
end
alt Account deletion path
Tabs->>Auth: User triggers delete via UserDeleteSection
User->>API: call api.user.delete mutation
API->>DB: delete authUsers where id = ctx.user.id
API-->>Auth: mutation success
Auth->>User: sign out & redirect to login
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION ? ( | ||
| <>Pro plan (cancelling on {subscription.scheduledChange.scheduledChangeAt.toLocaleDateString()})</> |
There was a problem hiding this comment.
Potential null pointer exception when accessing subscription.scheduledChange.scheduledChangeAt. If scheduledChange exists but scheduledChangeAt is null/undefined, calling toLocaleDateString() will throw a runtime error. Add null checking: subscription.scheduledChange?.scheduledChangeAt?.toLocaleDateString()
| subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION ? ( | |
| <>Pro plan (cancelling on {subscription.scheduledChange.scheduledChangeAt.toLocaleDateString()})</> | |
| subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION ? ( | |
| <>Pro plan (cancelling on {subscription.scheduledChange?.scheduledChangeAt?.toLocaleDateString()})</> | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
|
|
||
| export const SubscriptionTab = observer(() => { | ||
| const stateManager = useStateManager(); | ||
| const { data: user } = api.user.get.useQuery(); |
There was a problem hiding this comment.
The 'user' data from the API query is fetched but never used. Consider removing it if not needed.
| const { data: user } = api.user.get.useQuery(); |
…rtal login - Update cancel subscription text to use text-red-200 with hover:text-red-100 - Apply same color scheme to the cancel icon with group hover state - Simplify payment section to use a single Manage button (no dropdown) - Implement automatic Stripe customer portal session creation - Add loading state for portal session creation - Use existing manageSubscription API endpoint instead of hardcoded URL Users are now automatically logged into their Stripe billing portal without needing to sign in again, providing a seamless experience. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
![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
🧹 Nitpick comments (3)
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (3)
17-17: Use path alias for cross‑folder import.Aligns with repo alias policy for src imports.
-import { useSubscription } from '../pricing-modal/use-subscription'; +import { useSubscription } from '@/components/ui/pricing-modal/use-subscription';
4-4: Remove unused import and query.Avoid unnecessary network calls and dead imports.
-import { api } from '@/trpc/react'; @@ -import { ProductType, ScheduledSubscriptionAction } from '@onlook/stripe'; +import { ScheduledSubscriptionAction } from '@onlook/stripe'; @@ - const { data: user } = api.user.get.useQuery();Also applies to: 18-18, 24-24
71-71: Use nullish coalescing for numeric 0.Avoids treating 0 as “Unlimited”.
- <>Pro plan - {subscription?.price?.monthlyMessageLimit || 'Unlimited'} messages per month</> + <>Pro plan - {subscription?.price?.monthlyMessageLimit ?? 'Unlimited'} messages per month</>If adopting i18n per above, use t('unlimited') and t('messagesPerMonth').
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/client/src/components/ui/settings-modal/helpers.tsx(1 hunks)apps/web/client/src/components/ui/settings-modal/non-project.tsx(2 hunks)apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx(1 hunks)apps/web/client/src/components/ui/settings-modal/with-project.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/ui/settings-modal/helpers.tsxapps/web/client/src/components/ui/settings-modal/with-project.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/non-project.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/components/ui/settings-modal/helpers.tsxapps/web/client/src/components/ui/settings-modal/with-project.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/non-project.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/components/ui/settings-modal/helpers.tsxapps/web/client/src/components/ui/settings-modal/with-project.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/non-project.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/components/ui/settings-modal/helpers.tsxapps/web/client/src/components/ui/settings-modal/with-project.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/non-project.tsx
🧬 Code graph analysis (3)
apps/web/client/src/components/ui/settings-modal/with-project.tsx (1)
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (1)
SubscriptionTab(22-175)
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (3)
apps/web/client/src/components/store/state/index.ts (1)
useStateManager(6-6)apps/web/client/src/trpc/react.tsx (1)
api(23-23)apps/web/client/src/components/ui/pricing-modal/use-subscription.tsx (1)
useSubscription(7-30)
apps/web/client/src/components/ui/settings-modal/non-project.tsx (2)
packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (1)
SubscriptionTab(22-175)
🔇 Additional comments (7)
apps/web/client/src/components/ui/settings-modal/with-project.tsx (2)
14-14: Import OK.Local import is fine here; no alias needed for same‑folder modules.
72-76: Correct placement in Global Settings.The tab wiring (label, icon, component) matches patterns used for other global tabs.
apps/web/client/src/components/ui/settings-modal/non-project.tsx (2)
11-11: Import OK.Matches local module usage and component boundaries.
22-26: Tab wiring LGTM.Label, icon, and component are consistent with the project modal.
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (2)
20-20: Avoid hardcoding the Stripe billing portal URL — use a per-user Billing Portal Session or an env var (apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx:20)Replace STRIPE_BILLING_PORTAL_URL with a backend-created per-user Stripe Billing Portal Session (recommended) or, at minimum, read the portal URL from a public env var; this prevents stale environment-specific links and reduces exposure.
Verification: an rg search for a server endpoint to create portal sessions returned "No files were searched" — cannot confirm an existing server-side endpoint; confirm whether one exists or implement a server endpoint that creates Billing Portal Sessions and update this file.
69-70: scheduledChangeAt is already a Date — no conversion required.ScheduledChange.scheduledChangeAt is typed as Date (packages/stripe/src/types.ts); DB mappers return a Date (packages/db/src/mappers/subscription.ts); webhook/server set it with new Date(...) (apps/web/client/src/app/webhook/stripe/subscription/update.ts); tRPC uses SuperJSON on server + client (apps/web/server/src/router/trpc.ts, apps/web/client/src/server/api/trpc.ts). Calling subscription.scheduledChange.scheduledChangeAt.toLocaleDateString() in apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx is safe.
Optional: standardize date formatting (use app locale or a central formatter — several files use 'en-US').
apps/web/client/src/components/ui/settings-modal/helpers.tsx (1)
5-5: Enum addition looks good — confirm tabs list and deep-linking.Default remains SettingsTabValue.SITE (apps/web/client/src/components/store/state/manager.ts:7). Modal openers set specific tabs (advanced-settings.tsx → SettingsTabValue.DOMAIN; custom-domain/provider.tsx → SettingsTabValue.DOMAIN; top-bar/index.tsx → SettingsTabValue.VERSIONS). Settings modal UI sets/compares stateManager.settingsTab with tab.label (apps/web/client/src/components/ui/settings-modal/with-project.tsx and non-project.tsx). Ensure apps/web/client/src/components/ui/settings-modal/helpers.tsx includes SettingsTabValue.SUBSCRIPTION in the tabs array and that any URL/query deep-link parsing maps the "subscription" value to the new enum.
| const handleManageBilling = () => { | ||
| window.open(STRIPE_BILLING_PORTAL_URL, '_blank'); | ||
| setIsPaymentDropdownOpen(false); | ||
| }; |
There was a problem hiding this comment.
Add noopener/noreferrer to window.open.
Prevents reverse‑tabnabbing and removes opener reference.
- const handleManageBilling = () => {
- window.open(STRIPE_BILLING_PORTAL_URL, '_blank');
+ const handleManageBilling = () => {
+ window.open(STRIPE_BILLING_PORTAL_URL, '_blank', 'noopener,noreferrer');
setIsPaymentDropdownOpen(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.
| const handleManageBilling = () => { | |
| window.open(STRIPE_BILLING_PORTAL_URL, '_blank'); | |
| setIsPaymentDropdownOpen(false); | |
| }; | |
| const handleManageBilling = () => { | |
| window.open(STRIPE_BILLING_PORTAL_URL, '_blank', 'noopener,noreferrer'); | |
| setIsPaymentDropdownOpen(false); | |
| }; |
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx around
lines 46-49, the call to window.open(STRIPE_BILLING_PORTAL_URL, '_blank') should
be hardened to prevent reverse-tabnabbing by either passing the feature string
'noopener,noreferrer' as the third argument (window.open(url, '_blank',
'noopener,noreferrer')) or by assigning newWindow.opener = null on the returned
window object; update the code to open the URL with noopener and noreferrer and
keep the existing setIsPaymentDropdownOpen(false) behavior.
- Change avatar dropdown "Subscription" button behavior - Now opens settings modal directly to the subscription tab - Users can manage their subscription from the unified settings UI - Provides better UX by consolidating subscription management 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix settings dropdown to navigate to Preferences tab (first tab) by default - Only subscription dropdown navigates to Subscription tab - Capitalize "Subscription" in cancel modal button text - Add proper spacing between modal buttons (gap-3) - Improve cancel button hover state with red background and brighter text - Use custom styling instead of destructive variant for better UX 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add Delete Account section below Payment section - Implement multi-step confirmation process matching ChatGPT's flow - First modal requires user to enter their email and type "DELETE" - Button remains locked until both validations pass - Second modal provides final confirmation before deletion - Include comprehensive warning list about deletion consequences - Add proper form validation and state management - UI only - backend deletion logic to be implemented separately 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/components/ui/avatar-dropdown/index.tsx (1)
55-61: Externalize user‑facing labels to next‑intlAvoid hardcoded strings in the BUTTONS array per repo i18n guideline. Example patch:
+import { useTranslations } from 'next-intl'; @@ export const CurrentUserAvatar = ({ className }: { className?: string }) => { + const t = useTranslations('avatarMenu'); @@ - const BUTTONS = [ + const BUTTONS = [ { - label: 'Subscription', + label: t('subscription'), icon: Icons.CreditCard, onClick: handleOpenSubscription, }, { - label: 'Settings', + label: t('settings'), icon: Icons.Gear, onClick: handleOpenSettings, }, { - label: 'Send Feedback', + label: t('sendFeedback'), icon: Icons.MessageSquare, onClick: () => { void openFeedbackWidget(); setOpen(false); }, }, { - label: 'Sign Out', + label: t('signOut'), icon: Icons.Exit, onClick: handleSignOut, }, ];Also convert the user header and any other visible strings in this menu to translations.
Also applies to: 62-79
♻️ Duplicate comments (6)
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (6)
22-22: Remove unused user query
userisn’t used; drop the query to avoid unnecessary network work.- const { data: user } = api.user.get.useQuery(); + // removed unused user query
34-39: Add user‑visible error feedback for portal failuresConsole logging only is poor UX. Show a toast on error (and possibly success), wired into i18n.
- onError: (error) => { - console.error('Failed to create portal session:', error); - }, + onError: (error) => { + console.error('Failed to create portal session:', error); + // import { toast } from 'sonner' (or project toast) + // toast.error(t('payment.portalError')); + },
28-33: Harden window.open to prevent reverse‑tabnabbingOpen new tab with noopener/noreferrer.
- onSuccess: (session) => { + onSuccess: (session) => { if (session?.url) { - window.open(session.url, '_blank'); + window.open(session.url, '_blank', 'noopener,noreferrer'); } },
81-86: Null‑safe date formatting for scheduled cancellation
scheduleChangeAtmay be null/undefined; current code can throw at runtime.- <>Pro plan (cancelling on {subscription.scheduledChange.scheduledChangeAt.toLocaleDateString()})</> + <>Pro plan (cancelling on {subscription.scheduledChange?.scheduledChangeAt?.toLocaleDateString()})</>Optional: provide a fallback if the date is absent (e.g., “cancelling soon” via i18n).
118-126: “Reactivate” label calls cancel handlerLabel toggles to “Reactivate subscription” but still calls the cancel flow.
- {isPro && ( - <DropdownMenuItem - onClick={handleCancelSubscription} - className="cursor-pointer text-red-200 hover:text-red-100 group" - > - <Icons.CrossS className="mr-2 h-4 w-4 text-red-200 group-hover:text-red-100" /> - {subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION ? 'Reactivate subscription' : 'Cancel subscription'} - </DropdownMenuItem> - )} + {isPro && ( + <DropdownMenuItem + onClick={ + subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION + ? handleReactivateSubscription + : handleCancelSubscription + } + className={`cursor-pointer ${ + subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION + ? '' + : 'text-red-200 hover:text-red-100 group' + }`} + > + <Icons.CrossS className="mr-2 h-4 w-4 text-red-200 group-hover:text-red-100" /> + {subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION + ? 'Reactivate subscription' + : 'Cancel subscription'} + </DropdownMenuItem> + )}Add the handler and an
isCancellinghelper:@@ - const [isLoadingPortal, setIsLoadingPortal] = useState(false); + const [isLoadingPortal, setIsLoadingPortal] = useState(false); + const isCancelling = + subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION; @@ const handleCancelSubscription = () => { setShowCancelModal(true); setIsManageDropdownOpen(false); }; + + const handleReactivateSubscription = () => { + // Reactivation logic (API) to be implemented + setIsManageDropdownOpen(false); + };
70-74: Internationalize all user‑facing strings (next‑intl)Replace headings, descriptions, button/menu labels, and dialog copy with translations per repo policy.
Example patch (apply pattern throughout):
+import { useTranslations } from 'next-intl'; @@ export const SubscriptionTab = observer(() => { + const t = useTranslations('settings.subscription'); @@ - <h2 className="text-title3 mb-2">Subscription</h2> - <p className="text-muted-foreground text-small">Manage your subscription plan and billing</p> + <h2 className="text-title3 mb-2">{t('title')}</h2> + <p className="text-muted-foreground text-small">{t('subtitle')}</p> @@ - <p className="text-regularPlus font-medium">Current Plan</p> + <p className="text-regularPlus font-medium">{t('currentPlan')}</p> @@ - 'You are currently on the Free plan' + {t('freeStatus')} @@ - <Button variant="outline" size="sm">Manage<Icons.ChevronDown className="ml-1 h-3 w-3" /></Button> + <Button variant="outline" size="sm">{t('manage')}<Icons.ChevronDown className="ml-1 h-3 w-3" /></Button> @@ - Upgrade plan + {t('upgrade')} @@ - Change plan + {t('change')} @@ - Reactivate subscription + {t('reactivate')} @@ - Cancel subscription + {t('cancel')} @@ - <p className="text-regularPlus font-medium">Payment</p> + <p className="text-regularPlus font-medium">{t('payment.title')}</p> @@ - Manage your payment methods and billing details + {t('payment.subtitle')} @@ - {isLoadingPortal ? 'Opening...' : 'Manage'} + {isLoadingPortal ? t('opening') : t('manage')} @@ - <DialogTitle>Cancel Subscription</DialogTitle> + <DialogTitle>{t('cancel.title')}</DialogTitle> @@ - Are you sure you want to cancel your subscription? You'll lose access to all premium features at the end of your current billing period. + {t('cancel.description')} @@ - Keep Subscription + {t('cancel.keep')} @@ - Cancel Subscription + {t('cancel.confirm')}Also applies to: 79-90, 94-98, 100-117, 133-149, 157-176
🧹 Nitpick comments (4)
apps/web/client/src/components/ui/avatar-dropdown/index.tsx (2)
22-22: Use path alias instead of relative importRepo guideline: apps/web/client/src/* should import via @/* or ~/*. Replace the relative helpers import.
-import { SettingsTabValue } from '../settings-modal/helpers'; +import { SettingsTabValue } from '@/components/ui/settings-modal/helpers';
84-90: Button should be type="button" and have an accessible labelPrevents accidental form submits and improves a11y. Use i18n for the label.
- <button> + <button type="button" aria-label={t('openAccountMenu')}>apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (2)
85-86: Use nullish coalescing for limitsAvoid treating 0 as “Unlimited”.
- <>Pro plan - {subscription?.price?.monthlyMessageLimit || 'Unlimited'} messages per month</> + <>Pro plan - {subscription?.price?.monthlyMessageLimit ?? 'Unlimited'} messages per month</>
17-18: Use path alias for internal importFollow repo import policy for src/*.
-import { useSubscription } from '../pricing-modal/use-subscription'; +import { useSubscription } from '@/components/ui/pricing-modal/use-subscription';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/client/src/components/ui/avatar-dropdown/index.tsx(2 hunks)apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/ui/avatar-dropdown/index.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/components/ui/avatar-dropdown/index.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/components/ui/avatar-dropdown/index.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/components/ui/avatar-dropdown/index.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
🧬 Code graph analysis (1)
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (3)
apps/web/client/src/components/store/state/index.ts (1)
useStateManager(6-6)apps/web/client/src/trpc/react.tsx (1)
api(23-23)apps/web/client/src/components/ui/pricing-modal/use-subscription.tsx (1)
useSubscription(7-30)
🔇 Additional comments (2)
apps/web/client/src/components/ui/avatar-dropdown/index.tsx (2)
44-46: Good: unified settings modal navigation worksSwitching to settingsTab + isSettingsModalOpen is cleaner than a separate subscription modal flag.
50-52: Good: direct navigation to Preferences tabConsistent with the new tabbed settings UX.
| setDeleteConfirmText(''); | ||
| }; | ||
|
|
||
| const canProceedWithDelete = deleteEmail === user?.email && deleteConfirmText === 'DELETE'; |
There was a problem hiding this comment.
Consider trimming and normalizing the account email and confirmation input to avoid input mismatches in the deletion check.
| const canProceedWithDelete = deleteEmail === user?.email && deleteConfirmText === 'DELETE'; | |
| const canProceedWithDelete = deleteEmail.trim().toLowerCase() === user?.email?.trim().toLowerCase() && deleteConfirmText.trim() === 'DELETE'; |
| setShowFinalDeleteConfirm(true); | ||
| }; | ||
|
|
||
| const handleFinalDeleteAccount = () => { |
There was a problem hiding this comment.
handleFinalDeleteAccount currently only logs the deletion request. Implement the actual API call and error handling for account deletion.
|
@Kitenite I setup the UI for this + the UI leading up to it. I think you'll need to do the heavy-lifting on the connecting + deleting of accounts, plus perhaps validating the subscription stuff. |
| const handleFinalDeleteAccount = async () => { | ||
| await deleteUser(); | ||
| setShowFinalDeleteConfirm(false); | ||
| // Reset form | ||
| setDeleteEmail(''); | ||
| setDeleteConfirmText(''); | ||
| handleDeleteSuccess(); | ||
| }; |
There was a problem hiding this comment.
The handleFinalDeleteAccount function doesn't properly handle potential errors from the deleteUser mutation. Currently, even if the account deletion fails, the code will still proceed to sign the user out and redirect them via handleDeleteSuccess().
Consider adding error handling:
const handleFinalDeleteAccount = async () => {
try {
await deleteUser();
setShowFinalDeleteConfirm(false);
setDeleteEmail('');
setDeleteConfirmText('');
handleDeleteSuccess();
} catch (error) {
toast.error('Failed to delete account. Please try again.');
}
};Alternatively, utilize the mutation's built-in callbacks:
const { mutate: deleteUser } = api.user.delete.useMutation({
onSuccess: () => {
setShowFinalDeleteConfirm(false);
setDeleteEmail('');
setDeleteConfirmText('');
handleDeleteSuccess();
},
onError: () => {
toast.error('Failed to delete account. Please try again.');
}
});| const handleFinalDeleteAccount = async () => { | |
| await deleteUser(); | |
| setShowFinalDeleteConfirm(false); | |
| // Reset form | |
| setDeleteEmail(''); | |
| setDeleteConfirmText(''); | |
| handleDeleteSuccess(); | |
| }; | |
| const handleFinalDeleteAccount = async () => { | |
| try { | |
| await deleteUser(); | |
| setShowFinalDeleteConfirm(false); | |
| // Reset form | |
| setDeleteEmail(''); | |
| setDeleteConfirmText(''); | |
| handleDeleteSuccess(); | |
| } catch (error) { | |
| toast.error('Failed to delete account. Please try again.'); | |
| } | |
| }; | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
apps/web/client/src/components/ui/settings-modal/preferences-tab.tsx (3)
1-1: Drop unused tRPC import (after removing hooks)Keeps the module clean and avoids linter noise.
- import { api } from '@/trpc/react';
8-12: Avoid rendering an empty wrapperEmpty padding-only containers create layout noise. Either return null or render a localized placeholder later.
- return ( - <div className="flex flex-col gap-8 p-6"> - - </div> - ); + return null;Note: If you prefer a placeholder, use next-intl (e.g., t('preferences.empty')) instead of hardcoded text.
2-4: Remove unnecessary MobX observer wrapperNo observable state is used here; the observer adds overhead without benefit.
-import { observer } from 'mobx-react-lite'; +// (observer not needed here) -export const PreferencesTab = observer(() => { +export const PreferencesTab = () => { @@ -}); +};Also applies to: 13-13
apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx (1)
56-60: Replace hardcoded strings with next‑intlRepo guideline: avoid hardcoded UI text in apps/web/client/src/**/*.tsx. Use useTranslations and message keys.
Example:
+import { useTranslations } from 'next-intl'; @@ -export const UserDeleteSection = observer(() => { +export const UserDeleteSection = observer(() => { + const t = useTranslations('settings.deleteAccount'); @@ - <p className="text-regularPlus font-medium">Delete Account</p> + <p className="text-regularPlus font-medium">{t('title')}</p> @@ - <DialogTitle>Delete account - are you sure?</DialogTitle> + <DialogTitle>{t('confirmTitle')}</DialogTitle>Also applies to: 71-86, 90-109, 111-129, 138-141, 153-160
apps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsx (2)
12-36: Internationalize modal copyReplace literals with next‑intl strings to meet repo guidelines.
+import { useTranslations } from 'next-intl'; @@ -export const SubscriptionCancelModal = ({ open, onOpenChange, onConfirmCancel }: SubscriptionCancelModalProps) => { +export const SubscriptionCancelModal = ({ open, onOpenChange, onConfirmCancel }: SubscriptionCancelModalProps) => { + const t = useTranslations('settings.subscription.cancel'); @@ - <DialogTitle>Cancel Subscription</DialogTitle> + <DialogTitle>{t('title')}</DialogTitle> @@ - Are you sure you want to cancel your subscription? You'll lose access to all premium features at the end of your current billing period. + {t('description')} @@ - Keep Subscription + {t('keep')} @@ - Cancel Subscription + {t('confirm')}
30-36: Optional: use a destructive button style for the confirm actionIf your UI kit supports variant="destructive", prefer that over custom red classes for consistency.
- <Button - variant="outline" + <Button + variant="destructive" onClick={onConfirmCancel} - className="order-1 sm:order-2 text-red-200 hover:text-red-100 hover:bg-red-500/10 border-red-200 hover:border-red-100" + className="order-1 sm:order-2" >apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (1)
82-86: Null‑safe date access and correct nullish handlingGuard scheduledChangeAt and use ?? so 0 isn’t treated as “Unlimited”.
- <>Pro plan (cancelling on {subscription.scheduledChange.scheduledChangeAt.toLocaleDateString()})</> + <>Pro plan (cancelling on {subscription.scheduledChange?.scheduledChangeAt?.toLocaleDateString()})</> @@ - <>Pro plan - {subscription?.price?.monthlyMessageLimit || 'Unlimited'} messages per month</> + <>Pro plan - {subscription?.price?.monthlyMessageLimit ?? 'Unlimited'} messages per month</>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/client/src/components/ui/settings-modal/preferences-tab.tsx(1 hunks)apps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsx(1 hunks)apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx(1 hunks)apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx(1 hunks)apps/web/client/src/server/api/routers/user/user.ts(1 hunks)packages/db/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/user/user.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/server/api/routers/user/user.tsapps/web/client/src/components/ui/settings-modal/user-delete-section.tsxapps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/preferences-tab.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/server/api/routers/user/user.tsapps/web/client/src/components/ui/settings-modal/user-delete-section.tsxapps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/preferences-tab.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/server/api/routers/user/user.tsapps/web/client/src/components/ui/settings-modal/user-delete-section.tsxapps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/preferences-tab.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/components/ui/settings-modal/user-delete-section.tsxapps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsxapps/web/client/src/components/ui/settings-modal/subscription-tab.tsxapps/web/client/src/components/ui/settings-modal/preferences-tab.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx
🧬 Code graph analysis (4)
apps/web/client/src/server/api/routers/user/user.ts (1)
apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(130-149)
apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx (5)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)packages/ui/src/components/sonner.tsx (1)
toast(19-19)apps/web/client/src/utils/constants/index.ts (1)
Routes(1-29)packages/ui/src/components/button.tsx (1)
Button(57-57)packages/ui/src/components/dialog.tsx (6)
Dialog(109-109)DialogContent(111-111)DialogHeader(114-114)DialogTitle(117-117)DialogDescription(112-112)DialogFooter(113-113)
apps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsx (2)
packages/ui/src/components/dialog.tsx (6)
Dialog(109-109)DialogContent(111-111)DialogHeader(114-114)DialogTitle(117-117)DialogDescription(112-112)DialogFooter(113-113)packages/ui/src/components/button.tsx (1)
Button(57-57)
apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (5)
apps/web/client/src/components/store/state/index.ts (1)
useStateManager(6-6)apps/web/client/src/components/ui/pricing-modal/use-subscription.tsx (1)
useSubscription(7-30)apps/web/client/src/trpc/react.tsx (1)
api(23-23)apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx (1)
UserDeleteSection(15-166)apps/web/client/src/components/ui/settings-modal/subscription-cancel-modal.tsx (1)
SubscriptionCancelModal(12-41)
🔇 Additional comments (6)
packages/db/package.json (1)
32-34: Don't remove the "ai" devDependency from packages/db — it's referenced for types.packages/db/src/schema/chat/message.ts contains
import type { LanguageModelUsage } from 'ai';so the devDependency is used for TypeScript type-checking; keep it (or consolidate to the workspace root only if you intend to share it across packages). Also declared in packages/models, packages/ai, packages/ui, and apps/web/client.Likely an incorrect or invalid review comment.
apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx (1)
49-49: Normalize inputs for safer matchingTrim and case‑fold to avoid false negatives when the user types their email.
- const canProceedWithDelete = deleteEmail === user?.email && deleteConfirmText === 'DELETE'; + const canProceedWithDelete = + deleteEmail.trim().toLowerCase() === (user?.email?.trim().toLowerCase() ?? '') && + deleteConfirmText.trim() === 'DELETE';apps/web/client/src/components/ui/settings-modal/subscription-tab.tsx (3)
70-74: Internationalize user‑facing stringsReplace literals with next‑intl (title, subtitles, labels, menu items, button text). This is a repo rule for apps/web/client/src/**/*.
Example (partial):
+import { useTranslations } from 'next-intl'; @@ export const SubscriptionTab = observer(() => { + const t = useTranslations('settings.subscription'); @@ - <h2 className="text-title3 mb-2">Subscription</h2> + <h2 className="text-title3 mb-2">{t('title')}</h2> @@ - Manage + {t('manage')}Also applies to: 79-90, 94-107, 115-116, 136-148
28-36: Harden window.open and surface user feedback on errorsAdd noopener/noreferrer to avoid reverse‑tabnabbing and show a toast on failure.
- const manageSubscriptionMutation = api.subscription.manageSubscription.useMutation({ + const manageSubscriptionMutation = api.subscription.manageSubscription.useMutation({ onSuccess: (session) => { if (session?.url) { - window.open(session.url, '_blank'); + const win = window.open(session.url, '_blank', 'noopener,noreferrer'); + if (!win) window.location.href = session.url; // popup blocked fallback } }, - onError: (error) => { - console.error('Failed to create portal session:', error); - }, + onError: (error) => { + console.error('Failed to create portal session:', error); + toast.error('Unable to open billing portal. Please try again.'); + },Add missing import:
+import { toast } from '@onlook/ui/sonner';
118-126: Wrong handler for “Reactivate subscription”; label and action divergeCall a reactivation handler when cancellation is scheduled, and avoid destructive styling for reactivation.
+ const isCancelling = subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION; @@ - const handleCancelSubscription = () => { + const handleCancelSubscription = () => { setShowCancelModal(true); setIsManageDropdownOpen(false); }; + + const handleReactivateSubscription = () => { + // TODO: wire reactivation mutation + setIsManageDropdownOpen(false); + toast.info('Reactivation coming soon'); + }; @@ - {isPro && ( + {isPro && ( <DropdownMenuItem - onClick={handleCancelSubscription} - className="cursor-pointer text-red-200 hover:text-red-100 group" + onClick={isCancelling ? handleReactivateSubscription : handleCancelSubscription} + className={`cursor-pointer group ${isCancelling ? '' : 'text-red-200 hover:text-red-100'}`} > <Icons.CrossS className="mr-2 h-4 w-4 text-red-200 group-hover:text-red-100" /> - {subscription?.scheduledChange?.scheduledAction === ScheduledSubscriptionAction.CANCELLATION ? 'Reactivate subscription' : 'Cancel subscription'} + {isCancelling ? 'Reactivate subscription' : 'Cancel subscription'} </DropdownMenuItem> )}Also applies to: 24-27, 48-56
apps/web/client/src/server/api/routers/user/user.ts (1)
98-101: Cannot verify ON DELETE CASCADE — confirm DB cascade & PII cleanupSandbox repo search returned no matches; I cannot confirm any foreign-key ON DELETE CASCADE rules for users. If the DB doesn't cascade, deleting auth (supabase.admin.deleteUser) will orphan rows and retain PII — validate soft vs hard delete per GDPR/CCPA.
File: apps/web/client/src/server/api/routers/user/user.ts — delete mutation.
Suggested checks (run from repo root):
# search repo for FK refs and cascade rules rg -nP 'references\s+users\b' -S -C2 rg -nP 'on\s+delete\s+cascade' -S -C2 rg -nP 'foreign key\s*\(.*\)\s*references\s*users' -S -C2# search common schema/migration files if present fd -e sql -e prisma -e yml -e yaml -e toml -E node_modules -x rg -nP 'references\s+users\b|on\s+delete\s+cascade' -S -C2-- Postgres DB-level check (psql) SELECT conname, conrelid::regclass AS child_table, pg_get_constraintdef(oid) FROM pg_constraint WHERE contype = 'f' AND pg_get_constraintdef(oid) ILIKE '%REFERENCES users%';
| import { Icons } from '@onlook/ui/icons'; | ||
| import { observer } from 'mobx-react-lite'; | ||
|
|
||
| export const PreferencesTab = observer(() => { |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm client boundary exists for this tab
tRPC React hooks (and MobX observer if retained) require a client component boundary above. Verify the parent settings modal or entry file has 'use client'.
Run:
🏁 Script executed:
#!/bin/bash
# Verify a client boundary exists in settings modal tree
rg -n --type=ts,tsx -C1 $'^[ \t]*[\'"]use client[\'"]' apps/web/client/src/components/ui/settings-modal
# Also check modal entry points mentioned in the PR
rg -n --type=ts,tsx -C1 $'^[ \t]*[\'"]use client[\'"]' apps/web/client/src/components/ui/settings-modal/{non-project.tsx,with-project.tsx} 2>/dev/null || trueLength of output: 30
🏁 Script executed:
#!/bin/bash
# Verify client boundary exists in settings modal tree (search for 'use client' in ts/tsx)
dir="apps/web/client/src/components/ui/settings-modal"
echo "Checking directory: $dir"
if [ ! -d "$dir" ]; then
echo "DIR_NOT_FOUND"
exit 0
fi
echo "Listing files in $dir"
ls -la "$dir"
echo "Searching for 'use client' in .ts/.tsx under $dir"
grep -nR --include='*.tsx' --include='*.ts' -E "^\s*['\"]use client['\"]" "$dir" -n -H -C1 || true
echo "Also checking specific entry files (non-project.tsx, with-project.tsx)"
for f in non-project.tsx with-project.tsx; do
p="$dir/$f"
if [ -f "$p" ]; then
echo "---- $p (first 120 lines) ----"
sed -n '1,120p' "$p"
echo "grep result:"
grep -nE "^\s*['\"]use client['\"]" "$p" || true
else
echo "MISSING: $p"
fi
doneLength of output: 11899
Add 'use client' to the settings modal entry components
PreferencesTab is a MobX observer (and may use tRPC hooks); the modal entry files lack a client boundary—add a top-line "use client" to one of these entry components (or otherwise ensure a higher client boundary):
- apps/web/client/src/components/ui/settings-modal/non-project.tsx
- apps/web/client/src/components/ui/settings-modal/with-project.tsx
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/settings-modal/preferences-tab.tsx around
line 4, the PreferencesTab is a MobX observer and may use client-only hooks but
the modal entry components lack a client boundary; add a top-line "use client"
directive to one of the entry components (preferably both) —
apps/web/client/src/components/ui/settings-modal/non-project.tsx and
apps/web/client/src/components/ui/settings-modal/with-project.tsx — or ensure a
higher-level parent is already a client component, so the observer and any
tRPC/hooks run in a client boundary.
apps/web/client/src/components/ui/settings-modal/preferences-tab.tsx
Outdated
Show resolved
Hide resolved
| const [deleteEmail, setDeleteEmail] = useState(''); | ||
| const [deleteConfirmText, setDeleteConfirmText] = useState(''); | ||
| const [showFinalDeleteConfirm, setShowFinalDeleteConfirm] = useState(false); | ||
| const { mutate: deleteUser } = api.user.delete.useMutation(); |
There was a problem hiding this comment.
Critical: using mutate (non‑Promise) with await; flow proceeds even if delete fails
useMutation().mutate isn’t async; await deleteUser() is a no‑op. Use mutateAsync, add error handling, and disable while loading.
Apply:
- const { mutate: deleteUser } = api.user.delete.useMutation();
+ const { mutateAsync: deleteUser, isLoading: isDeleting } = api.user.delete.useMutation();
@@
- const handleFinalDeleteAccount = async () => {
- await deleteUser();
- setShowFinalDeleteConfirm(false);
- // Reset form
- setDeleteEmail('');
- setDeleteConfirmText('');
- handleDeleteSuccess();
- };
+ const handleFinalDeleteAccount = async () => {
+ try {
+ await deleteUser();
+ setShowFinalDeleteConfirm(false);
+ // Reset form
+ setDeleteEmail('');
+ setDeleteConfirmText('');
+ await handleDeleteSuccess();
+ } catch (e) {
+ toast.error('Failed to delete account. Please try again.');
+ }
+ };
@@
- <Button
+ <Button
onClick={handleDeleteConfirm}
- disabled={!canProceedWithDelete}
+ disabled={!canProceedWithDelete || isDeleting}
className="order-1 sm:order-2 bg-red-600 hover:bg-red-700 text-white disabled:bg-gray-300 disabled:text-gray-500"
>
- {canProceedWithDelete ? 'Delete Account' : 'Locked'}
+ {isDeleting ? 'Deleting…' : (canProceedWithDelete ? 'Delete Account' : 'Locked')}
</Button>Also applies to: 33-40, 123-129
🤖 Prompt for AI Agents
In apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx
around lines 21 (and similarly at 33-40, 123-129), the code uses
api.user.delete.useMutation()'s mutate (non-Promise) with await which is
ineffective; replace mutate with mutateAsync from the hook, wire loading/error
state from the mutation (isLoading/isError) to disable the delete button and
show feedback, and wrap the mutateAsync call in a try/catch to handle and
log/display errors so the flow only proceeds on successful deletion.
| const handleConfirmCancel = () => { | ||
| // Cancellation logic will be implemented later | ||
| setShowCancelModal(false); | ||
| }; |
There was a problem hiding this comment.
The subscription cancellation implementation is incomplete. The handleConfirmCancel function only closes the modal without actually canceling the subscription. This creates a misleading user experience where users might believe they've canceled their subscription when they haven't. Consider implementing the actual cancellation logic before shipping this feature, or clearly indicating to users that this functionality is coming soon if it must be shipped in this state.
| const handleConfirmCancel = () => { | |
| // Cancellation logic will be implemented later | |
| setShowCancelModal(false); | |
| }; | |
| const handleConfirmCancel = async () => { | |
| try { | |
| // Call the subscription cancellation API | |
| await cancelUserSubscription(); | |
| // Show success message to user | |
| toast.success("Your subscription has been successfully canceled"); | |
| // Close the modal | |
| setShowCancelModal(false); | |
| } catch (error) { | |
| // Handle errors | |
| console.error("Failed to cancel subscription:", error); | |
| toast.error("Failed to cancel your subscription. Please try again or contact support."); | |
| } | |
| }; | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/_components/top-bar/github.tsx (1)
21-51: Avoid “undefined”/NaN UI on GitHub errors; add ok-checks, type guards, timeout, and cleanup.GitHub returns JSON errors on rate limits; without
response.okchecks,stargazers_countcan be undefined and render “undefined”. Add guards and abort to prevent hangs and set safe fallbacks.Apply this diff:
useEffect(() => { - const fetchStats = async () => { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 5000); + const fetchStats = async () => { try { // Stars - const repoResponse = await fetch('https://api.github.com/repos/onlook-dev/onlook'); - const repoData = await repoResponse.json(); - setRaw(repoData.stargazers_count); - setFormatted(formatStarCount(repoData.stargazers_count)); + const repoResponse = await fetch('https://api.github.com/repos/onlook-dev/onlook', { + signal: controller.signal, + headers: { Accept: 'application/vnd.github+json' }, + }); + if (!repoResponse.ok) throw new Error(`GitHub repo fetch failed: ${repoResponse.status}`); + const repoData = await repoResponse.json(); + const stars = + typeof repoData?.stargazers_count === 'number' + ? repoData.stargazers_count + : DEFAULT_STAR_COUNT; + setRaw(stars); + setFormatted(formatStarCount(stars)); // Contributors (use the Link header for pagination) - const contribResponse = await fetch('https://api.github.com/repos/onlook-dev/onlook/contributors?per_page=1&anon=true'); + const contribResponse = await fetch( + 'https://api.github.com/repos/onlook-dev/onlook/contributors?per_page=1&anon=true', + { signal: controller.signal, headers: { Accept: 'application/vnd.github+json' } } + ); + if (!contribResponse.ok) throw new Error(`GitHub contributors fetch failed: ${contribResponse.status}`); const linkHeader = contribResponse.headers.get('Link'); if (linkHeader) { - const match = linkHeader.match(/&page=(\d+)>; rel="last"/); - if (match) { - setContributors(Number(match[1])); - } + const match = linkHeader.match(/[?&]page=(\d+)>; rel="last"/); + if (match) setContributors(Number(match[1])); + else setContributors(DEFAULT_CONTRIBUTORS_COUNT); } else { // fallback: count the single returned contributor const contribData = await contribResponse.json(); setContributors(Array.isArray(contribData) ? contribData.length : DEFAULT_CONTRIBUTORS_COUNT); } } catch (error) { console.error('Failed to fetch GitHub stats:', error); setRaw(DEFAULT_STAR_COUNT); setFormatted(formatStarCount(DEFAULT_STAR_COUNT)); setContributors(DEFAULT_CONTRIBUTORS_COUNT); + } finally { + clearTimeout(timeoutId); } }; fetchStats(); -}, []); + return () => controller.abort(); +}, []);
🧹 Nitpick comments (8)
apps/web/client/src/app/_components/top-bar/github.tsx (2)
9-14: Optional: format millions as “M”.Keeps display compact as stars grow.
const formatStarCount = (count: number): string => { - if (count >= 1000) { + if (count >= 1_000_000) { + return `${(count / 1_000_000).toFixed(1)}M`.replace('.0M', 'M'); + } + if (count >= 1000) { return `${(count / 1000).toFixed(1)}k`.replace('.0k', 'k'); } return count.toString(); };
21-51: Consider moving GitHub fetches server‑side with caching.Expose a tiny API route (revalidate ~1h) and fetch from the client to avoid browser rate limits and improve resilience.
packages/db/src/schema/subscription/usage.ts (2)
12-12: onUpdate: 'cascade' is likely unnecessary for UUID PKs.Primary keys for users shouldn’t change. Dropping onUpdate reduces surprise without losing safety.
Apply:
- userId: uuid('user_id').notNull().references(() => users.id, { onDelete: 'cascade', onUpdate: 'cascade' }), + userId: uuid('user_id').notNull().references(() => users.id, { onDelete: 'cascade' }),
15-16: Nit: avoid reserved-ish column names.Consider renaming
timestamptooccurredAt/recordedAtto reduce confusion with the SQL type name.packages/db/src/schema/subscription/rate-limits.ts (2)
12-15: onUpdate: 'cascade' is unusual here.IDs for users/subscriptions shouldn’t change. Safe to drop the onUpdate to reduce implicit data movement.
- .references(() => users.id, { onDelete: 'cascade', onUpdate: 'cascade' }), + .references(() => users.id, { onDelete: 'cascade' }), ... - .references(() => subscriptions.id, { onDelete: 'cascade', onUpdate: 'cascade' }), + .references(() => subscriptions.id, { onDelete: 'cascade' }),
41-43: Optional index to support subscription-scoped queries.If you frequently fetch limits by subscription/time, add a covering index.
export const rateLimits = pgTable('rate_limits', { ... }, (table) => [ index('rate_limits_user_time_idx').on(table.userId, table.startedAt, table.endedAt), + index('rate_limits_subscription_time_idx').on(table.subscriptionId, table.startedAt, table.endedAt), ])packages/db/src/schema/subscription/subscription.ts (2)
36-36: Make scheduledPriceId nullable on delete.If a Price is removed/archived, the scheduled reference should clear instead of blocking deletion.
- scheduledPriceId: uuid('scheduled_price_id').references(() => prices.id), + scheduledPriceId: uuid('scheduled_price_id').references(() => prices.id, { onDelete: 'set null', onUpdate: 'cascade' }),
15-17: Minor: onUpdate cascades are redundant.UUID PKs aren’t expected to change; consider dropping onUpdate for productId/priceId for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/client/src/app/_components/top-bar/github.tsx(1 hunks)apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx(1 hunks)apps/web/client/src/server/api/routers/user/user.ts(2 hunks)packages/db/src/schema/domain/deployment.ts(1 hunks)packages/db/src/schema/subscription/rate-limits.ts(1 hunks)packages/db/src/schema/subscription/subscription.ts(1 hunks)packages/db/src/schema/subscription/usage.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/client/src/components/ui/settings-modal/user-delete-section.tsx
- apps/web/client/src/server/api/routers/user/user.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/db/src/schema/subscription/usage.tspackages/db/src/schema/domain/deployment.tsapps/web/client/src/app/_components/top-bar/github.tsxpackages/db/src/schema/subscription/rate-limits.tspackages/db/src/schema/subscription/subscription.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/db/src/schema/subscription/usage.tspackages/db/src/schema/domain/deployment.tsapps/web/client/src/app/_components/top-bar/github.tsxpackages/db/src/schema/subscription/rate-limits.tspackages/db/src/schema/subscription/subscription.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/_components/top-bar/github.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/_components/top-bar/github.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/_components/top-bar/github.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/_components/top-bar/github.tsx
🧬 Code graph analysis (4)
packages/db/src/schema/subscription/usage.ts (1)
packages/db/src/schema/user/user.ts (1)
users(12-25)
packages/db/src/schema/domain/deployment.ts (1)
packages/db/src/schema/user/user.ts (1)
users(12-25)
packages/db/src/schema/subscription/rate-limits.ts (2)
packages/db/src/schema/user/user.ts (1)
users(12-25)packages/db/src/schema/subscription/subscription.ts (1)
subscriptions(11-38)
packages/db/src/schema/subscription/subscription.ts (3)
packages/db/src/schema/user/user.ts (1)
users(12-25)packages/db/src/schema/subscription/product.ts (1)
products(6-13)packages/db/src/schema/subscription/price.ts (1)
prices(8-22)
🔇 Additional comments (5)
apps/web/client/src/app/_components/top-bar/github.tsx (1)
6-6: LGTM: refreshed fallback star count.Good to keep the initial UI state current.
packages/db/src/schema/subscription/rate-limits.ts (1)
12-15: Cascades on userId/subscriptionId look correct for lifecycle.Rate limits should disappear with the parent user/subscription. LGTM.
Before enabling account deletion in prod, ensure Stripe subscriptions are cancelled first to avoid orphaned live billing. Do we already do this in the delete flow?
packages/db/src/schema/subscription/subscription.ts (1)
15-17: Change productId/priceId FK onDelete to RESTRICT; keep userId CASCADEProducts/prices are rarely deleted — cascading would remove subscriptions. Keep CASCADE for userId so account deletions still remove a user’s subscriptions.
File: packages/db/src/schema/subscription/subscription.ts (lines 15–17)
- userId: uuid('user_id').notNull().references(() => users.id, { onDelete: 'cascade', onUpdate: 'cascade' }), - productId: uuid('product_id').notNull().references(() => products.id, { onDelete: 'cascade', onUpdate: 'cascade' }), - priceId: uuid('price_id').notNull().references(() => prices.id, { onDelete: 'cascade', onUpdate: 'cascade' }), + userId: uuid('user_id').notNull().references(() => users.id, { onDelete: 'cascade', onUpdate: 'cascade' }), + productId: uuid('product_id').notNull().references(() => products.id, { onDelete: 'restrict', onUpdate: 'cascade' }), + priceId: uuid('price_id').notNull().references(() => prices.id, { onDelete: 'restrict', onUpdate: 'cascade' }),Operational: ensure account-deletion flow cancels Stripe subscriptions before deleting the user to avoid live Stripe subs with missing DB rows.
Verify: quick scan found no code deleting products/prices — re-run to confirm:
rg -nP "delete\s+from\s+products|delete\s+from\s+prices|\bdb\.\s*delete\s*\(\s*products|\bdb\.\s*delete\s*\(\s*prices" -C2packages/db/src/schema/domain/deployment.ts (1)
14-14: Avoid hard-deleting deployments — make requestedBy nullable and use ON DELETE SET NULL.File: packages/db/src/schema/domain/deployment.ts (around line 14)
Deployments/build logs are historical artifacts; cascade deletes will permanently remove them.
- requestedBy: uuid('requested_by').references(() => users.id, { onDelete: 'cascade', onUpdate: 'cascade' }).notNull(), + requestedBy: uuid('requested_by').references(() => users.id, { onDelete: 'set null', onUpdate: 'cascade' }),Do product requirements allow losing deployment history when an account is deleted? If not, adopt SET NULL or reassign to a "System" user.
packages/db/src/schema/subscription/usage.ts (1)
12-12: Confirm data-retention/audit policy — cascading user deletes will remove many child recordsDeleting a user will hard-delete child rows; FK cascades to users.id are present in:
- packages/db/src/schema/user/user-canvas.ts
- packages/db/src/schema/user/user-project.ts
- packages/db/src/schema/subscription/usage.ts
- packages/db/src/schema/subscription/rate-limits.ts
- packages/db/src/schema/subscription/subscription.ts
- packages/db/src/schema/domain/deployment.ts
- packages/db/src/schema/project/invitation.ts
If you need billing/dispute/analytics retention, switch to soft-delete or archive records before user delete, or change FK onDelete to RESTRICT/SET NULL. FK cascades execute as the table owner and bypass RLS.
| await localforage.setItem(LAST_SIGN_IN_METHOD_KEY, method); | ||
| await login(method); | ||
| } catch (error) { | ||
| toast.error('Error signing in with password'); |
There was a problem hiding this comment.
Consider using a generic error message instead of 'Error signing in with password' since this flow may not always involve a password (e.g., OAuth).
| toast.error('Error signing in with password'); | |
| toast.error('Error signing in'); |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/login/actions.tsx (1)
13-15: Do not trust request Origin for OAuth redirectTo (open‑redirect risk).Using the client‑supplied Origin header to build the OAuth callback enables host/header spoofing and can redirect users to attacker‑controlled domains. Always derive the callback from a trusted base URL.
Apply this diff:
- const origin = (await headers()).get('origin') ?? env.NEXT_PUBLIC_SITE_URL; - const redirectTo = `${origin}${Routes.AUTH_CALLBACK}`; + const redirectTo = new URL(Routes.AUTH_CALLBACK, env.NEXT_PUBLIC_SITE_URL).toString();
🧹 Nitpick comments (6)
packages/db/src/seed/db.ts (4)
174-248: DRY up message seed creation with a small factory.Five seeds share the same shape. A helper reduces repetition and centralizes defaults (incl.
usage).Example:
function makeMessage(overrides: Partial<Message>): Message { return { id: uuidv4(), conversationId: conversation0.id, role: 'user', content: '', commitOid: null, createdAt: new Date(), applied: false, context: contexts, checkpoints: [], parts: [], snapshots: null, usage: null, ...overrides, } as Message; }
349-349: Duplicate deletion of userProjects in resetDb.
await tx.delete(userProjects);appears at Line 338 and again here. Drop the second call.Apply:
- await tx.delete(userProjects); await tx.delete(projects);
52-54: Stripe customer ID mismatch between user and subscription.
user0.stripeCustomerIdisnullwhilesubscription0.stripeCustomerIdis set. If future “Manage billing” flows rely onuser.stripeCustomerId, seeds may not reflect real paths.Option: define and reuse a single seeded constant for both.
const STRIPE_CUSTOMER_ID = 'cus_1234567890'; // user0.stripeCustomerId: STRIPE_CUSTOMER_ID // subscription0.stripeCustomerId: STRIPE_CUSTOMER_IDAlso applies to: 272-272
32-32: Avoid importing into another package’s src via alias.
import { db } from '@onlook/db/src/client'relies on aliasing into/src. Prefer the local module entry or a relative path to avoid packaging/TSConfig coupling.apps/web/client/src/app/login/actions.tsx (2)
8-8: Remove now‑unused headers() import.After switching to a trusted base URL, this import is unused and will trip linting.
-import { headers } from 'next/headers';
33-35: Log OAuth errors before redirecting to aid debugging.We swallow details on the error path. Add a server‑side log (or your structured logger) before redirect.
- if (error) { - redirect('/error'); - } + if (error) { + console.error('OAuth sign-in error', { provider, error }); + redirect('/error'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/client/src/app/auth/auth-context.tsx(2 hunks)apps/web/client/src/app/login/actions.tsx(1 hunks)apps/web/client/src/server/api/routers/user/user.ts(2 hunks)packages/db/src/seed/db.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/login/actions.tsxapps/web/client/src/app/auth/auth-context.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/login/actions.tsxapps/web/client/src/server/api/routers/user/user.tsapps/web/client/src/app/auth/auth-context.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/login/actions.tsxapps/web/client/src/app/auth/auth-context.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/login/actions.tsxpackages/db/src/seed/db.tsapps/web/client/src/server/api/routers/user/user.tsapps/web/client/src/app/auth/auth-context.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/login/actions.tsxapps/web/client/src/app/auth/auth-context.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/login/actions.tsxpackages/db/src/seed/db.tsapps/web/client/src/server/api/routers/user/user.tsapps/web/client/src/app/auth/auth-context.tsx
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/user/user.ts
🧠 Learnings (4)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/user/user.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/user/user.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/src/app/auth/auth-context.tsx
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/app/auth/auth-context.tsx
🧬 Code graph analysis (2)
apps/web/client/src/server/api/routers/user/user.ts (2)
apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(131-150)packages/db/src/schema/supabase/user.ts (1)
authUsers(5-10)
apps/web/client/src/app/auth/auth-context.tsx (3)
apps/web/client/src/utils/constants/index.ts (1)
LocalForageKeys(48-50)apps/web/client/src/app/login/actions.tsx (2)
login(11-38)devLogin(40-63)packages/ui/src/components/sonner.tsx (1)
toast(19-19)
🔇 Additional comments (6)
packages/db/src/seed/db.ts (1)
186-186: Message.usage is nullable and consumers handle null — seed null is optional.
- schema: packages/db/src/schema/chat/message.ts —
usage: jsonb("usage")(no.notNull()/.default()→ nullable).- mapper: packages/db/src/mappers/chat/message.ts —
usage: message.metadata?.usage ?? null.- UI: apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/error-message.tsx —
Usage | null.Explicit
usage: nullin seeds is redundant (omitting the field will result in NULL); safe to remove or leave as-is.apps/web/client/src/server/api/routers/user/user.ts (2)
98-100: Replace raw DB delete with Admin delete + app-data cleanup + explicit result; ensurectxpreservessupabase(dup of earlier note)Call
ctx.supabase.auth.admin.deleteUserfirst, then clean up app data (public.users) to trigger your CASCADEs. Handle errors and return a plain object. Also fixprotectedProcedureto spread the originalctxsosupabaseis present.Apply to this router:
- delete: protectedProcedure.mutation(async ({ ctx }) => { - await ctx.db.delete(authUsers).where(eq(authUsers.id, ctx.user.id)); - }), + delete: protectedProcedure.mutation(async ({ ctx }) => { + try { + // 1) Remove auth user and sessions via Supabase Admin API + await ctx.supabase.auth.admin.deleteUser(ctx.user.id); + } catch (err) { + throw new TRPCError({ code: 'INTERNAL_SERVER_ERROR', message: 'Failed to delete auth user' }); + } + + // 2) Cleanup application data; CASCADEs from `users` will remove dependents + await ctx.db.transaction(async (tx) => { + await tx.delete(users).where(eq(users.id, ctx.user.id)); + }); + + return { success: true } as const; + }),And fix
apps/web/client/src/server/api/trpc.tssoctx.supabasesurvivesprotectedProcedure:return next({ - ctx: { - // infers the `session` as non-nullable - user: ctx.user as SetRequiredDeep<User, 'email'>, - db: ctx.db, - }, + ctx: { + ...ctx, + // infers the `session` as non-nullable + user: ctx.user as SetRequiredDeep<User, 'email'>, + }, });Verify FK cascades originate from
public.users(notauth.users) so step (2) wipes dependents:#!/bin/bash # Inspect drizzle schema for CASCADEs hanging off `users` rg -nP -C2 --type=ts "references\(\s*\(\)\s*=>\s*users\.id" packages/db/src/schema | sed -n '1,200p' rg -nP -C2 --type=ts "onDelete\(['\"]cascade['\"]\)" packages/db/src/schema | sed -n '1,200p'
3-3: Prefer Supabase Admin API over directauth.usersdeletes; remove unused importDirectly deleting from
auth.userscan bypass GoTrue cleanup (sessions, identities, factors). Use the Admin API and drop this import.-import { authUsers, fromDbUser, userInsertSchema, users, type User } from '@onlook/db'; +import { fromDbUser, userInsertSchema, users, type User } from '@onlook/db'; +import { TRPCError } from '@trpc/server';apps/web/client/src/app/login/actions.tsx (1)
58-61: Confirm dev-only and error exposure
- devLogin is only defined in apps/web/client/src/app/login/actions.tsx and its sole call site is handleDevLogin in apps/web/client/src/app/auth/auth-context.tsx — the server-side guard (process.env.NODE_ENV !== 'development') prevents the actual supabase sign-in from running in non-development.
- However, auth-context.handleDevLogin catches thrown errors and passes error.message into toast.description, so any message thrown by devLogin will appear in the UI when handleDevLogin is invoked.
Action: either (A) keep as-is if you accept developer-visible error messages in dev flows only, or (B) avoid passing raw error.message to the toast (use a generic message) if you want to ensure no downstream UI ever displays upstream error texts even in development.
Location: apps/web/client/src/app/login/actions.tsx (devLogin) and apps/web/client/src/app/auth/auth-context.tsx (handleDevLogin).
apps/web/client/src/app/auth/auth-context.tsx (2)
5-5: LGTM: toast import in a client componentAppropriate for a client-only context.
37-52: Remove rethrow; internationalize OAuth toast; prefer alias import
- Replace catch block to avoid throwing in a UI handler and use next-intl for an OAuth-specific message:
} catch (error) { - toast.error('Error signing in with password'); - console.error('Error signing in with password:', error); - throw new Error('Error signing in with password'); + toast.error(t('auth.signIn.oauthErrorTitle'), { + description: error instanceof Error ? error.message : t('common.tryAgain'), + }); + console.error('auth.signIn.oauthError', error); + return; } finally {
- Add useTranslations in the component:
import { useTranslations } from 'next-intl'; // ... export const AuthProvider = ({ children }: { children: ReactNode }) => { const t = useTranslations(); // ... }
- Prefer alias import for actions (file: apps/web/client/src/app/auth/auth-context.tsx):
Replace
import { devLogin, login } from '../login/actions';with
import { devLogin, login } from '@/app/login/actions';
- Verification: I confirmed the relative import in apps/web/client/src/app/auth/auth-context.tsx; repository-wide hardcoded toast.error occurrences were not fully verified in this run — execute these locally to finish verification:
rg -n -C1 "toast.error(['"][^'\"]+['"]" apps/web/client/src
rg -n -C1 "from ['"]../login/actions['"]" apps/web/client/src
| const handleDevLogin = async (returnUrl: string | null) => { | ||
| setSigningInMethod(SignInMethod.DEV); | ||
| if (returnUrl) { | ||
| await localforage.setItem(LocalForageKeys.RETURN_URL, returnUrl); | ||
| } | ||
| await devLogin(); | ||
| setTimeout(() => { | ||
| try { | ||
| setSigningInMethod(SignInMethod.DEV); | ||
| if (returnUrl) { | ||
| await localforage.setItem(LocalForageKeys.RETURN_URL, returnUrl); | ||
| } | ||
| await devLogin(); | ||
| } catch (error) { | ||
| toast.error('Error signing in with password', { | ||
| description: error instanceof Error ? error.message : 'Please try again.', | ||
| }); | ||
| console.error('Error signing in with password:', error); | ||
| throw new Error('Error signing in with password'); | ||
| } finally { | ||
| setSigningInMethod(null); | ||
| }, 5000); | ||
| } | ||
| } |
There was a problem hiding this comment.
Align dev login error handling with OAuth; avoid rethrow and hardcoded text
- Don’t rethrow after toasting; return instead.
- Use next-intl; keep the descriptive message.
- Unify console tags.
Apply within this block:
} catch (error) {
- toast.error('Error signing in with password', {
- description: error instanceof Error ? error.message : 'Please try again.',
- });
- console.error('Error signing in with password:', error);
- throw new Error('Error signing in with password');
+ toast.error(t('auth.signIn.passwordErrorTitle'), {
+ description: error instanceof Error ? error.message : t('common.tryAgain'),
+ });
+ console.error('auth.signIn.devError', error);
+ return;
} finally {📝 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.
| const handleDevLogin = async (returnUrl: string | null) => { | |
| setSigningInMethod(SignInMethod.DEV); | |
| if (returnUrl) { | |
| await localforage.setItem(LocalForageKeys.RETURN_URL, returnUrl); | |
| } | |
| await devLogin(); | |
| setTimeout(() => { | |
| try { | |
| setSigningInMethod(SignInMethod.DEV); | |
| if (returnUrl) { | |
| await localforage.setItem(LocalForageKeys.RETURN_URL, returnUrl); | |
| } | |
| await devLogin(); | |
| } catch (error) { | |
| toast.error('Error signing in with password', { | |
| description: error instanceof Error ? error.message : 'Please try again.', | |
| }); | |
| console.error('Error signing in with password:', error); | |
| throw new Error('Error signing in with password'); | |
| } finally { | |
| setSigningInMethod(null); | |
| }, 5000); | |
| } | |
| } | |
| const handleDevLogin = async (returnUrl: string | null) => { | |
| try { | |
| setSigningInMethod(SignInMethod.DEV); | |
| if (returnUrl) { | |
| await localforage.setItem(LocalForageKeys.RETURN_URL, returnUrl); | |
| } | |
| await devLogin(); | |
| } catch (error) { | |
| toast.error(t('auth.signIn.passwordErrorTitle'), { | |
| description: error instanceof Error ? error.message : t('common.tryAgain'), | |
| }); | |
| console.error('auth.signIn.devError', error); | |
| return; | |
| } finally { | |
| setSigningInMethod(null); | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/app/auth/auth-context.tsx around lines 54 to 70, the dev
login catch block currently rethrows, uses a hardcoded English error title, and
prints a console message inconsistent with OAuth handlers; update it to mirror
the OAuth error handling by (1) replacing the hardcoded toast title with
next-intl usage (use intl.formatMessage for the title) while keeping the
descriptive message in the description, (2) remove the throw and simply return
after showing the toast, and (3) standardize the console error tag to match the
OAuth handlers (e.g., use the same prefix/string used elsewhere); preserve the
finally block that clears signingInMethod.
Summary
Changes
New Features
Subscription Tab Component (
subscription-tab.tsx)Subscription Management
Payment Management
Cancel Subscription Modal
Integration
useSubscriptionhook for dataScreenshots
The implementation follows the design inspiration provided while maintaining consistency with Onlook's existing UI patterns.
Test Plan
🤖 Generated with Claude Code
Important
Adds subscription management and account deletion features to the settings panel, with Stripe integration for payments and backend support for user data handling.
SubscriptionTabinsettings-modalfor managing subscription plans and payments.SubscriptionCancelModalfor confirming subscription cancellations.UserDeleteSectioninpreferences-tabfor account deletion with confirmation dialogs.user.ts.avatar-dropdownto open subscription and settings modals.PREFERENCEStoACCOUNTinSettingsTabValue.deployment.ts,rate-limits.ts,subscription.ts, andusage.ts.This description was created by
for ec39c3b. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Changes
Backend
Chores
Bug Fixes