-
Notifications
You must be signed in to change notification settings - Fork 5
refactor: add Settings page #382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds a dedicated authenticated Settings page and routes account-related flows to it; simplifies AccountMenu to a compact header linking to Settings; removes in-place dialog flows (team, API keys) in favor of route-based navigation and updates post-purchase/credit redirect targets to Settings tabs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Sidebar as Sidebar / AccountMenu
participant Router as App Router
participant Settings as Settings Page
participant Billing as Billing API
participant Sections as Settings Sections
User->>Sidebar: Click "Settings"
Sidebar->>Router: Navigate /settings (with search)
Router->>Settings: Render SettingsPage
Settings->>Billing: Fetch billing & credit state
Billing-->>Settings: Billing data
Settings->>Settings: Resolve effectiveTab (tab/team_setup/credits_success)
Settings->>Sections: Render selected tab component
Sections-->>User: Display UI (Team/API/Billing/History/etc.)
sequenceDiagram
actor PaymentProvider
participant Browser
participant Router as App Router
participant Settings as Settings Page
participant Billing as Billing API
PaymentProvider->>Browser: Redirect to /settings?tab=api&credits_success=true
Browser->>Router: Route to /settings
Router->>Settings: Mount SettingsPage
Settings->>Billing: Refresh credit balance
Billing-->>Settings: Updated balance
Settings->>Settings: Show success message (auto-hide 6s)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Deploying maple with
|
| Latest commit: |
04c656c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5fe5a9e4.maple-ca8.pages.dev |
| Branch Preview URL: | https://refactor-settings-page.maple-ca8.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR refactors account management by consolidating all settings into a new dedicated /settings page with tabbed navigation. The AccountMenu dropdown is simplified to a single Settings button that routes to the new page.
Key Changes:
- Created
/settingsauthenticated route with 6 tabs: Account, Billing, Team, API, History, About - Moved all account management dialogs (team, API keys, history deletion) from dropdown to dedicated settings sections
- Preserved deep-link flows: payment success and pricing pages redirect to
/settings?tab=team&team_setup=truefor team setup - API credit purchase success redirects to
/settings?tab=api&credits_success=true - Updated
ApiCreditsSectionsuccess/cancel URLs to return to settings page instead of homepage
Architecture:
The settings page uses TanStack Router's search params for tab selection and flow coordination. Each tab section is a separate component rendered conditionally based on the active tab.
Confidence Score: 4/5
- Generally safe to merge with minor UX issues that won't break functionality
- The refactor successfully moves account management to a dedicated settings page. The code is well-structured with proper authentication guards. However, there are two logic issues: (1) the team setup dialog doesn't close automatically after successful team creation, requiring manual dismissal even though the team exists, and (2) query param navigation timing could cause effectiveTab to evaluate stale params during transition. These are UX issues that won't cause crashes but affect user experience.
- frontend/src/routes/_auth.settings.tsx - Team setup dialog state management after team creation
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| frontend/src/routes/_auth.settings.tsx | 4/5 | New settings page with tabbed UI for account, billing, team, API, history, and about sections. Handles deep-link flows for team setup and credit success. |
| frontend/src/components/AccountMenu.tsx | 5/5 | Simplified to show plan badge, credit usage, and settings link. Removed dropdown menu and all dialog components (moved to settings page). |
| frontend/src/routes/index.tsx | 5/5 | Removed team and API key dialogs. Refactored deep-link flows to redirect to /settings with appropriate query params. |
| frontend/src/routes/payment-success.tsx | 5/5 | Updated team plan redirect to navigate to /settings with team setup params instead of index page. |
| frontend/src/routes/pricing.tsx | 5/5 | Updated team plan success redirect to navigate to /settings with team setup params. |
| frontend/src/components/apikeys/ApiCreditsSection.tsx | 5/5 | Updated success/cancel URLs to redirect to /settings?tab=api instead of index page root. |
| frontend/src/routeTree.gen.ts | 5/5 | Auto-generated route tree file with new /settings route added under _auth parent. |
| frontend/bun.lock | 5/5 | Lock file updated with dependency changes. |
Sequence Diagram
sequenceDiagram
participant User
participant Index as Index Page
participant PaymentSuccess as Payment Success
participant Settings as Settings Page
participant TeamDialog as Team Setup Dialog
participant BillingAPI as Billing API
alt Team Setup Flow (from payment success)
PaymentSuccess->>Settings: Navigate to /settings?tab=team&team_setup=true
Settings->>Settings: effectiveTab = "team" (line 120)
Settings->>Settings: useEffect detects team_setup=true
Settings->>Settings: setAutoOpenTeamSetup(true)
Settings->>Settings: navigate (replace) to /settings?tab=team
Settings->>TeamDialog: Auto-open dialog (line 525)
User->>TeamDialog: Enter team name & submit
TeamDialog->>BillingAPI: createTeam()
BillingAPI-->>TeamDialog: Success
TeamDialog->>TeamDialog: Invalidate teamStatus query
Note over TeamDialog: Dialog stays open (doesn't close itself)
Settings->>Settings: teamStatus refetched (team_created=true)
Note over Settings,TeamDialog: Dialog remains open until manual close
end
alt API Credits Success Flow
Index->>Settings: Navigate to /settings?tab=api&credits_success=true
Settings->>Settings: effectiveTab = "api" (line 121)
Settings->>Settings: useEffect detects credits_success=true
Settings->>Settings: setShowApiCreditSuccessMessage(true)
Settings->>Settings: Invalidate apiCreditBalance query
Settings->>Settings: navigate (replace) to /settings?tab=api
Settings->>Settings: Timer clears success message after 6s
end
alt Regular Settings Access
User->>Index: Click Settings button in AccountMenu
Index->>Settings: Navigate to /settings?tab=account
Settings->>Settings: Render account section
end
| useEffect(() => { | ||
| if (!autoOpenSetup || !needsSetup || hasAutoOpened) return; | ||
| setIsSetupOpen(true); | ||
| setHasAutoOpened(true); | ||
| }, [autoOpenSetup, needsSetup, hasAutoOpened]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After team creation succeeds, TeamSetupDialog doesn't close itself—it invalidates queries and waits for parent control. However, when teamStatus updates to team_created: true, this component won't automatically close the dialog because isSetupOpen state remains true. The dialog will stay open showing stale "create team" UI until manually closed, even though the team now exists.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/_auth.settings.tsx
Line: 523:527
Comment:
After team creation succeeds, `TeamSetupDialog` doesn't close itself—it invalidates queries and waits for parent control. However, when `teamStatus` updates to `team_created: true`, this component won't automatically close the dialog because `isSetupOpen` state remains `true`. The dialog will stay open showing stale "create team" UI until manually closed, even though the team now exists.
How can I resolve this? If you propose a fix, please make it concise.| useEffect(() => { | ||
| if (!team_setup || !os.auth.user) return; | ||
| setAutoOpenTeamSetup(true); | ||
| navigate({ to: "/settings", search: { tab: "team" }, replace: true }); | ||
| }, [team_setup, os.auth.user, navigate]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The navigation here replaces URL history but continues to render with the old team_setup=true query param until the next render. This means effectiveTab (line 120) evaluates team_setup as true even after navigation, potentially causing duplicate tab selection logic. Consider using the navigation result or clearing the search param before computing effectiveTab.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/_auth.settings.tsx
Line: 126:130
Comment:
The navigation here replaces URL history but continues to render with the old `team_setup=true` query param until the next render. This means `effectiveTab` (line 120) evaluates `team_setup` as `true` even after navigation, potentially causing duplicate tab selection logic. Consider using the navigation result or clearing the search param before computing `effectiveTab`.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/routes/_auth.settings.tsx:
- Around line 139-141: The inline comment incorrectly says "5s hide timer" while
the actual timer uses 6000ms; update the comment to match the code (either
change the text to "6s hide timer" or adjust the timeout to 5000ms) so the
description for ApiCreditsSection and the setTimeout(() =>
setShowApiCreditSuccessMessage(false), 6000) timer are consistent.
🧹 Nitpick comments (4)
frontend/src/routes/_auth.settings.tsx (4)
96-105: Side effect in queryFn is unconventional.Calling
setBillingStatus(status)insidequeryFnworks but mixes data fetching with state updates. Consider moving this to anonSuccesscallback or usingselectwith a separate effect.♻️ Alternative using useEffect for state sync
- useQuery({ + const { data: fetchedBillingStatus } = useQuery({ queryKey: ["billingStatus"], queryFn: async () => { const billingService = getBillingService(); const status = await billingService.getBillingStatus(); - setBillingStatus(status); return status; }, enabled: !!os.auth.user }); + + useEffect(() => { + if (fetchedBillingStatus) { + setBillingStatus(fetchedBillingStatus); + } + }, [fetchedBillingStatus, setBillingStatus]);
298-305: Silent failure on verification email request.If
requestNewVerificationEmailfails, the error is only logged. The user gets no feedback that the resend failed. Consider showing an error state or toast notification.♻️ Suggested improvement with error state
+ const [verificationError, setVerificationError] = useState(false); + const handleResendVerification = async () => { try { + setVerificationError(false); await os.requestNewVerificationEmail(); setVerificationStatus("pending"); } catch (error) { console.error("Failed to resend verification email:", error); + setVerificationError(true); } };Then display the error in the UI:
{verificationError && ( <span className="text-xs text-destructive">Failed to send. Try again.</span> )}
425-433: Consider adding a comment explaining the 300ms delay.The
setTimeoutwith 300ms delay after opening URL on mobile isn't immediately obvious. Is this to allow the browser to open before UI state updates? A comment would help maintainability.📝 Suggested documentation
await invoke("plugin:opener|open_url", { url }).catch((err: Error) => { console.error("[Billing] Failed to open external browser:", err); alert("Failed to open browser. Please try again."); }); + // Brief delay to allow external browser to open before resetting loading state await new Promise((resolve) => setTimeout(resolve, 300)); return;
608-628: Silent error handling may leave user confused.Both
clearHistory()anddeleteConversations()catch errors but only log them. If either fails, the user proceeds without knowing their data may not be fully deleted. Consider showing an error toast or alert.♻️ Consider error feedback
const handleDeleteHistory = async () => { + let hasError = false; + try { await clearHistory(); } catch (error) { console.error("Error clearing history:", error); + hasError = true; } try { const conversations = await os.listConversations({ limit: 1 }); if (conversations.data && conversations.data.length > 0) { await os.deleteConversations(); } } catch (error) { console.error("Error deleting conversations:", error); + hasError = true; } queryClient.invalidateQueries({ queryKey: ["chatHistory"] }); queryClient.invalidateQueries({ queryKey: ["conversations"] }); queryClient.invalidateQueries({ queryKey: ["archivedChats"] }); + + if (hasError) { + // Consider showing a toast: "Some data may not have been deleted" + } + navigate({ to: "/" }); };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
frontend/src/components/AccountMenu.tsxfrontend/src/components/apikeys/ApiCreditsSection.tsxfrontend/src/routeTree.gen.tsfrontend/src/routes/_auth.settings.tsxfrontend/src/routes/index.tsxfrontend/src/routes/payment-success.tsxfrontend/src/routes/pricing.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/routes/pricing.tsxfrontend/src/routes/payment-success.tsxfrontend/src/components/apikeys/ApiCreditsSection.tsxfrontend/src/routes/_auth.settings.tsxfrontend/src/routes/index.tsxfrontend/src/components/AccountMenu.tsxfrontend/src/routeTree.gen.ts
🧠 Learnings (1)
📚 Learning: 2025-12-08T19:55:33.330Z
Learnt from: CR
Repo: OpenSecretCloud/Maple PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T19:55:33.330Z
Learning: Applies to **/*.{ts,tsx} : Use React context for global state management and TanStack Query for server state management
Applied to files:
frontend/src/routes/index.tsx
🧬 Code graph analysis (2)
frontend/src/routes/_auth.settings.tsx (5)
frontend/src/state/useLocalState.tsx (1)
useLocalState(4-10)frontend/src/billing/billingService.ts (1)
getBillingService(225-230)frontend/src/types/team.ts (1)
TeamStatus(1-15)frontend/src/components/apikeys/ApiKeyDashboard.tsx (1)
ApiKeyDashboard(36-285)frontend/src/utils/openUrl.ts (1)
openExternalUrlWithConfirmation(96-103)
frontend/src/components/AccountMenu.tsx (2)
frontend/src/components/ui/badge.tsx (1)
Badge(10-12)frontend/src/components/CreditUsage.tsx (1)
CreditUsage(4-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-android
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: build-ios
🔇 Additional comments (14)
frontend/src/components/AccountMenu.tsx (2)
48-56: Good implementation of conditional navigation with alert indicator.The Settings button correctly shows an alert badge when team setup is required and passes the appropriate search parameters to deep-link directly to the team tab. The conditional logic is clear and the UI feedback is appropriate.
33-35: Type safety confirmed — search parameters match the Settings route definition.The
settingsSearchobject correctly usesas constfor type narrowing. Both tab values ("team" and "account") are validSettingsTabenum members, andteam_setup: truematches the expected boolean type. The missingteam_setupproperty in the account branch properly defaults toundefined, which satisfies the optional type definition.frontend/src/routes/pricing.tsx (1)
273-279: LGTM! Consistent redirect pattern for team plan post-purchase flow.The redirect to
/settingswithtab: "team"andteam_setup: truecorrectly routes users to the team setup section after a successful team plan purchase. The use ofreplace: trueprevents back-button confusion, which is appropriate for this flow.frontend/src/routes/payment-success.tsx (1)
47-53: LGTM! Consistent team plan redirect.The redirect to
/settingswith appropriate search parameters maintains consistency with the pricing page flow. The team plan detection logic is clear and the navigation pattern aligns with the PR objectives.frontend/src/routes/index.tsx (1)
81-91: LGTM! Clean route-based navigation replaces dialog flows.The refactor successfully replaces dialog-based flows with route-based navigation to the Settings page. Both the team setup and API credits success flows correctly:
- Check for user authentication before navigating
- Use
replace: trueto avoid back-button issues- Pass appropriate tab parameters to deep-link to the correct section
The dependency arrays are correct and the navigation patterns are consistent with the other updated files in this PR.
frontend/src/routes/_auth.settings.tsx (8)
1-51: LGTM! Imports are well-organized and use path aliases correctly.Imports follow the coding guidelines using
@/*path aliases for local imports. The separation between external libraries and internal components is clear.
52-78: LGTM! Type-safe search parameter validation.The tab validation logic correctly handles unknown input types and uses proper type narrowing. The boolean coercion for
team_setupandcredits_successhandles both actual booleans and string representations from URL query params.
149-165: Sign-out error handling is robust.The nested try-catch properly handles billing token cleanup failures independently from sign-out failures, with a sensible fallback to
window.location.hreffor complete recovery.
377-388: Duplicate log out button exists in header and Account section.There's a "Log out" button in the header (line 223-226) and another in the Account section's Session card. While this may be intentional for discoverability, verify this is the desired UX.
443-505: LGTM! BillingSection UI is well-structured.The conditional rendering for upgrade vs. manage subscription buttons is correct, and the date formatting handles the Unix timestamp conversion properly.
507-575: LGTM! TeamSection handles all states correctly.The auto-open logic with
hasAutoOpenedguard prevents infinite re-renders. The conditional rendering covers non-team, needs-setup, and active-team states appropriately.
577-600: LGTM! Clean conditional rendering for platform-specific UI.The mobile fallback message is user-friendly, and the prop forwarding to
ApiKeyDashboardis correct.
662-711: LGTM! AboutSection with appropriate external link handling.The use of
openExternalUrlWithConfirmationfor external URLs and the Tauri-specific notice are good UX considerations.frontend/src/routeTree.gen.ts (1)
1-10: Auto-generated file — no manual review required.This file is automatically generated by TanStack Router as indicated in the header comments. The changes correctly integrate the new
/_auth/settingsroute. Ensurejust buildor the router codegen script regenerates this file if the route definition changes.
| // Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts. | ||
| const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000); | ||
| return () => clearTimeout(timer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Comment says 5s but timer is 6s.
The comment references "5s hide timer" from ApiCreditsSection, but this timer is 6000ms (6s). This minor discrepancy could cause confusion during maintenance.
📝 Suggested fix
- // Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts.
- const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000);
+ // Hide success message after 6 seconds to prevent re-show on remounts.
+ const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000);📝 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.
| // Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts. | |
| const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000); | |
| return () => clearTimeout(timer); | |
| // Hide success message after 6 seconds to prevent re-show on remounts. | |
| const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000); | |
| return () => clearTimeout(timer); |
🤖 Prompt for AI Agents
In @frontend/src/routes/_auth.settings.tsx around lines 139 - 141, The inline
comment incorrectly says "5s hide timer" while the actual timer uses 6000ms;
update the comment to match the code (either change the text to "6s hide timer"
or adjust the timeout to 5000ms) so the description for ApiCreditsSection and
the setTimeout(() => setShowApiCreditSuccessMessage(false), 6000) timer are
consistent.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @frontend/src/components/apikeys/ApiKeyDashboard.tsx:
- Around line 117-126: The error branch in ApiKeyDashboard returns only
<DashboardHeader /> causing layout mismatch with the loading branch which uses
<div className="space-y-4">; update the error case inside the ApiKeyDashboard
component so it returns the same wrapper (wrap the existing <DashboardHeader
title="API Management" ... /> in a <div className="space-y-4">) to keep
consistent structure and spacing with the loading state.
In @frontend/src/routes/_auth.settings.tsx:
- Around line 125-148: The effect handling credits_success calls
queryClient.invalidateQueries and then immediately navigate, which can cause a
race; make the intent explicit by either awaiting the invalidate/refetch before
calling navigate or marking the fire-and-forget explicitly (prefixing the
invalidateQueries call with void) so it’s clear you intentionally aren’t
waiting; update the useEffect block that references credits_success,
queryClient.invalidateQueries, navigate, and setShowApiCreditSuccessMessage to
use one of these two approaches and add a brief comment clarifying the chosen
behavior for ApiCreditsSection.
🧹 Nitpick comments (4)
frontend/src/components/team/TeamDashboard.tsx (1)
16-29: DashboardHeader is duplicated across files.This exact component implementation appears in both
TeamDashboard.tsxandApiKeyDashboard.tsx. Consider extracting it to a shared location (e.g.,@/components/ui/dashboard-header.tsx) to avoid duplication and ensure consistent styling.♻️ Suggested extraction
Create a new shared component:
// frontend/src/components/ui/dashboard-header.tsx export function DashboardHeader({ title, description }: { title: React.ReactNode; description?: React.ReactNode; }) { return ( <div className="space-y-1"> <h2 className="text-base font-semibold leading-none tracking-tight">{title}</h2> {description ? <div className="text-sm text-muted-foreground">{description}</div> : null} </div> ); }Then import it in both dashboard files:
import { DashboardHeader } from "@/components/ui/dashboard-header";frontend/src/routes/_auth.settings.tsx (3)
453-477: Consider handling the Tauri invoke error more gracefully.The
handleManageSubscriptionfunction usesalert()for error feedback (Line 465), which is not consistent with the rest of the UI that uses toast or in-context messaging. Also, the 300ms delay (Line 467) after Tauri invoke seems arbitrary.♻️ Consider using a toast notification
If a toast system is available in the codebase, consider using it for consistency:
await invoke("plugin:opener|open_url", { url }).catch((err: Error) => { console.error("[Billing] Failed to open external browser:", err); - alert("Failed to open browser. Please try again."); + // Consider using toast notification if available + // toast.error("Failed to open browser. Please try again."); });
638-696: Query invalidations are not awaited, and errors are silently logged.In
handleDeleteHistory, thequeryClient.invalidateQueriescalls (Lines 660-662) are fire-and-forget, and both try-catch blocks only log errors without user feedback. Navigation to "/" happens regardless of success/failure.♻️ Consider awaiting invalidations and providing user feedback
const handleDeleteHistory = async () => { try { await clearHistory(); } catch (error) { console.error("Error clearing history:", error); + // Consider showing error feedback to user } try { const conversations = await os.listConversations({ limit: 1 }); if (conversations.data && conversations.data.length > 0) { await os.deleteConversations(); } } catch (error) { console.error("Error deleting conversations:", error); + // Consider showing error feedback to user } - queryClient.invalidateQueries({ queryKey: ["chatHistory"] }); - queryClient.invalidateQueries({ queryKey: ["conversations"] }); - queryClient.invalidateQueries({ queryKey: ["archivedChats"] }); + await Promise.all([ + queryClient.invalidateQueries({ queryKey: ["chatHistory"] }), + queryClient.invalidateQueries({ queryKey: ["conversations"] }), + queryClient.invalidateQueries({ queryKey: ["archivedChats"] }) + ]); navigate({ to: "/" }); };
1-56: Large file with multiple section components — consider future extraction.This file contains 747 lines with 6 section components (AccountSection, BillingSection, TeamSection, ApiSection, HistorySection, AboutSection). While currently manageable, consider extracting these into separate files under
@/components/settings/if they grow in complexity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/apikeys/ApiKeyDashboard.tsxfrontend/src/components/team/TeamDashboard.tsxfrontend/src/routes/_auth.settings.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/routes/_auth.settings.tsxfrontend/src/components/apikeys/ApiKeyDashboard.tsxfrontend/src/components/team/TeamDashboard.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-08T19:55:33.330Z
Learnt from: CR
Repo: OpenSecretCloud/Maple PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T19:55:33.330Z
Learning: Applies to src/components/ui/**/*.{ts,tsx} : Use existing shadcn/ui components from `src/components/ui/` for UI elements instead of creating custom components
Applied to files:
frontend/src/components/apikeys/ApiKeyDashboard.tsxfrontend/src/components/team/TeamDashboard.tsx
📚 Learning: 2025-12-08T19:55:33.330Z
Learnt from: CR
Repo: OpenSecretCloud/Maple PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T19:55:33.330Z
Learning: Applies to **/*.{ts,tsx} : Use functional components with React hooks instead of class components
Applied to files:
frontend/src/components/apikeys/ApiKeyDashboard.tsxfrontend/src/components/team/TeamDashboard.tsx
🧬 Code graph analysis (2)
frontend/src/routes/_auth.settings.tsx (10)
frontend/src/utils/utils.ts (2)
useIsMobile(16-54)cn(8-10)frontend/src/state/useLocalState.tsx (1)
useLocalState(4-10)frontend/src/billing/billingService.ts (1)
getBillingService(225-230)frontend/src/components/ui/button.tsx (1)
Button(62-62)frontend/src/components/ui/select.tsx (5)
Select(141-141)SelectTrigger(144-144)SelectValue(143-143)SelectContent(145-145)SelectItem(147-147)frontend/src/components/ui/card.tsx (5)
Card(56-56)CardHeader(56-56)CardTitle(56-56)CardDescription(56-56)CardContent(56-56)frontend/src/components/ui/label.tsx (1)
Label(19-19)frontend/src/components/ui/input.tsx (1)
Input(24-24)frontend/src/utils/openUrl.ts (1)
openExternalUrlWithConfirmation(96-103)frontend/src/components/ui/alert.tsx (2)
Alert(49-49)AlertDescription(49-49)
frontend/src/components/apikeys/ApiKeyDashboard.tsx (1)
frontend/src/components/ui/tabs.tsx (1)
Tabs(53-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-linux
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: Cloudflare Pages
🔇 Additional comments (12)
frontend/src/components/team/TeamDashboard.tsx (3)
31-98: LGTM!The TeamDashboard component properly handles:
- Loading state with early return (Line 39-41)
- Team name editing with appropriate validation (empty check, length limit)
- Optimistic query invalidation after successful update
- Error handling with user-friendly messages
100-135: LGTM!Non-admin member view is clean and appropriately simplified, showing only relevant information (team name, membership date, leave team option).
137-272: LGTM!Admin view properly handles:
- Seat limit exceeded warning
- Inline team name editing with keyboard shortcuts (Enter/Escape)
- Visual seat usage progress bar
- Conditional invite button based on seat availability
frontend/src/components/apikeys/ApiKeyDashboard.tsx (3)
50-103: LGTM!Component setup is well-structured:
- Billing and API access checks are clear
- Query configuration with proper
enabledguard- Handler functions for key creation/deletion with refetch
- Proxy API key creation with error handling
129-201: LGTM!The upgrade prompt for users without API access is well-designed with clear feature highlights and a call-to-action to view pricing plans.
203-295: LGTM!Main API access view with tabs is properly implemented:
- Conditional tab rendering for desktop-only Local Proxy
- Proper grid column calculation based on platform
- Clean separation of concerns with dedicated sections
frontend/src/routes/_auth.settings.tsx (6)
58-84: LGTM!Route setup is well-structured:
- Type-safe tab definitions using const assertion
- Proper search param validation with fallback handling
- Clean
parseSettingsTabhelper for type narrowing
155-171: Nested try-catch is overly defensive but acceptable.The nested try-catch in
signOuthandles billing token clearing separately from the main sign-out flow. While verbose, this ensures sign-out completes even if billing service throws. The fallback towindow.location.hrefis a good safety net.
197-317: LGTM!The main layout is well-structured:
- Responsive sidebar handling with toggle
- Mobile-friendly tab navigation via Select dropdown
- Desktop navigation with proper active state styling
- Clean conditional rendering of section components
320-433: LGTM!AccountSection handles:
- Guest vs email user detection for password change eligibility
- Email verification status with resend capability
- Proper dialog state management for preferences, password change, and account deletion
543-611: LGTM!TeamSection properly handles:
- Non-team plan upgrade prompt with link to teams page
- Team setup detection and auto-open behavior
- Prevention of multiple auto-opens via
hasAutoOpenedstate- Clean integration with TeamDashboard and TeamSetupDialog
698-747: LGTM!AboutSection provides clean external link handling with:
- Internal navigation for About page
- External URL handling via
openExternalUrlWithConfirmation- Tauri-specific alert for user awareness about system browser
| if (error) { | ||
| return ( | ||
| <> | ||
| <DialogHeader> | ||
| <DialogTitle className="text-base">API Management</DialogTitle> | ||
| <DialogDescription className="text-destructive"> | ||
| Failed to load API keys. Please try again. | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| </> | ||
| <DashboardHeader | ||
| title="API Management" | ||
| description={ | ||
| <span className="text-destructive">Failed to load API keys. Please try again.</span> | ||
| } | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent error state wrapper structure.
The error state returns just <DashboardHeader /> without a wrapper <div>, unlike the loading state (Lines 107-114) which wraps content in <div className="space-y-4">. This could cause layout inconsistencies.
🐛 Proposed fix
if (error) {
return (
+ <div className="space-y-4">
<DashboardHeader
title="API Management"
description={
<span className="text-destructive">Failed to load API keys. Please try again.</span>
}
/>
+ </div>
);
}🤖 Prompt for AI Agents
In @frontend/src/components/apikeys/ApiKeyDashboard.tsx around lines 117 - 126,
The error branch in ApiKeyDashboard returns only <DashboardHeader /> causing
layout mismatch with the loading branch which uses <div className="space-y-4">;
update the error case inside the ApiKeyDashboard component so it returns the
same wrapper (wrap the existing <DashboardHeader title="API Management" ... />
in a <div className="space-y-4">) to keep consistent structure and spacing with
the loading state.
| const effectiveTab: SettingsTab = useMemo(() => { | ||
| if (team_setup) return "team"; | ||
| if (credits_success) return "api"; | ||
| return tab ?? "account"; | ||
| }, [tab, team_setup, credits_success]); | ||
|
|
||
| // Support legacy/team flow query params by routing into the appropriate settings section. | ||
| useEffect(() => { | ||
| if (!team_setup || !os.auth.user) return; | ||
| setAutoOpenTeamSetup(true); | ||
| navigate({ to: "/settings", search: { tab: "team" }, replace: true }); | ||
| }, [team_setup, os.auth.user, navigate]); | ||
|
|
||
| useEffect(() => { | ||
| if (!credits_success || !os.auth.user) return; | ||
|
|
||
| setShowApiCreditSuccessMessage(true); | ||
| queryClient.invalidateQueries({ queryKey: ["apiCreditBalance"] }); | ||
| navigate({ to: "/settings", search: { tab: "api" }, replace: true }); | ||
|
|
||
| // Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts. | ||
| const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000); | ||
| return () => clearTimeout(timer); | ||
| }, [credits_success, os.auth.user, navigate, queryClient]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition potential between query invalidation and navigation.
In the credits_success effect (Lines 138-148), queryClient.invalidateQueries is called without await, then navigation happens immediately. This could lead to stale data being shown briefly if the query refetches after navigation completes.
🐛 Proposed fix
useEffect(() => {
if (!credits_success || !os.auth.user) return;
setShowApiCreditSuccessMessage(true);
- queryClient.invalidateQueries({ queryKey: ["apiCreditBalance"] });
+ void queryClient.invalidateQueries({ queryKey: ["apiCreditBalance"] });
navigate({ to: "/settings", search: { tab: "api" }, replace: true });
// Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts.
const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000);
return () => clearTimeout(timer);
}, [credits_success, os.auth.user, navigate, queryClient]);Using void makes the intentional fire-and-forget explicit. The current behavior is acceptable since ApiCreditsSection likely handles loading states, but documenting the intent improves clarity.
📝 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 effectiveTab: SettingsTab = useMemo(() => { | |
| if (team_setup) return "team"; | |
| if (credits_success) return "api"; | |
| return tab ?? "account"; | |
| }, [tab, team_setup, credits_success]); | |
| // Support legacy/team flow query params by routing into the appropriate settings section. | |
| useEffect(() => { | |
| if (!team_setup || !os.auth.user) return; | |
| setAutoOpenTeamSetup(true); | |
| navigate({ to: "/settings", search: { tab: "team" }, replace: true }); | |
| }, [team_setup, os.auth.user, navigate]); | |
| useEffect(() => { | |
| if (!credits_success || !os.auth.user) return; | |
| setShowApiCreditSuccessMessage(true); | |
| queryClient.invalidateQueries({ queryKey: ["apiCreditBalance"] }); | |
| navigate({ to: "/settings", search: { tab: "api" }, replace: true }); | |
| // Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts. | |
| const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000); | |
| return () => clearTimeout(timer); | |
| }, [credits_success, os.auth.user, navigate, queryClient]); | |
| const effectiveTab: SettingsTab = useMemo(() => { | |
| if (team_setup) return "team"; | |
| if (credits_success) return "api"; | |
| return tab ?? "account"; | |
| }, [tab, team_setup, credits_success]); | |
| // Support legacy/team flow query params by routing into the appropriate settings section. | |
| useEffect(() => { | |
| if (!team_setup || !os.auth.user) return; | |
| setAutoOpenTeamSetup(true); | |
| navigate({ to: "/settings", search: { tab: "team" }, replace: true }); | |
| }, [team_setup, os.auth.user, navigate]); | |
| useEffect(() => { | |
| if (!credits_success || !os.auth.user) return; | |
| setShowApiCreditSuccessMessage(true); | |
| void queryClient.invalidateQueries({ queryKey: ["apiCreditBalance"] }); | |
| navigate({ to: "/settings", search: { tab: "api" }, replace: true }); | |
| // Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts. | |
| const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000); | |
| return () => clearTimeout(timer); | |
| }, [credits_success, os.auth.user, navigate, queryClient]); |
🤖 Prompt for AI Agents
In @frontend/src/routes/_auth.settings.tsx around lines 125 - 148, The effect
handling credits_success calls queryClient.invalidateQueries and then
immediately navigate, which can cause a race; make the intent explicit by either
awaiting the invalidate/refetch before calling navigate or marking the
fire-and-forget explicitly (prefixing the invalidateQueries call with void) so
it’s clear you intentionally aren’t waiting; update the useEffect block that
references credits_success, queryClient.invalidateQueries, navigate, and
setShowApiCreditSuccessMessage to use one of these two approaches and add a
brief comment clarifying the chosen behavior for ApiCreditsSection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR refactors account management from multiple dialog-based UIs into a unified Settings page with tabbed navigation. The implementation successfully consolidates Account, Billing, Team, API, History, and About sections into a single /settings route.
Key Changes:
- New authenticated
/settingsroute with 6 tabbed sections (Account, Billing, Team, API, History, About) - Simplified
AccountMenucomponent to a single Settings button - Updated payment success and team setup flows to redirect to the appropriate Settings tab
- Refactored
ApiKeyDashboardandTeamDashboardfrom dialog-based to standalone components
Architecture:
The Settings page properly uses the _auth route prefix for authentication protection, supports deep-linking via query params (?tab=team&team_setup=true), and maintains backward compatibility with existing flows.
Critical Issue Found:
Mobile API credit purchases will fail due to a missing /payment-success-credits route. The mobile flow redirects to https://trymaple.ai/payment-success-credits which doesn't exist, causing a 404 error for mobile users after purchasing API credits.
Minor Issues:
- Timer duration mismatch between parent and child components for success messages
- Navigation state management could be more robust
- Sign out error handling could provide better user feedback for partial failures
Overall, this is a well-executed refactor that improves UX by centralizing settings, but requires fixing the mobile payment redirect before merging.
Confidence Score: 3/5
- This PR has a critical bug that breaks mobile API credit purchases, but otherwise implements a solid refactor with good architectural decisions.
- Score reflects one critical issue that will cause production failures for mobile users purchasing API credits. The missing
/payment-success-creditsroute means mobile users (iOS/Android) will encounter a 404 error after completing payment, creating a poor user experience and potential support burden. The rest of the implementation is well-executed with proper authentication, clean refactoring, and good separation of concerns. The issue is straightforward to fix but must be addressed before merging. - frontend/src/components/apikeys/ApiCreditsSection.tsx requires immediate attention to fix the mobile payment redirect URLs (line 105-107). Consider also reviewing DeepLinkHandler.tsx (not in this PR) to ensure it properly handles API credit payment callbacks.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| frontend/src/routes/_auth.settings.tsx | 4/5 | New Settings page with tabbed interface for Account, Billing, Team, API, History, and About. Handles deep-link flows for team setup and API credits. Well-structured but has issues with mobile payment redirects. |
| frontend/src/components/AccountMenu.tsx | 5/5 | Simplified to a single Settings button that redirects to /settings route. Removed dialog-based UI. Clean refactor with team setup alert badge. |
| frontend/src/routes/index.tsx | 5/5 | Removed team and API key dialog management. Now redirects team_setup and credits_success flows to /settings with appropriate query params. Clean simplification. |
| frontend/src/components/apikeys/ApiCreditsSection.tsx | 3/5 | Updated success/cancel URLs to redirect to /settings. Mobile redirect URLs reference non-existent /payment-success-credits route - critical bug. |
Sequence Diagram
sequenceDiagram
participant User
participant Browser
participant AccountMenu
participant SettingsPage
participant TeamDashboard
participant ApiKeyDashboard
participant BillingService
participant PaymentProvider
%% Flow 1: User navigates to Settings
User->>AccountMenu: Clicks Settings button
AccountMenu->>Browser: Navigate to /settings?tab=account
Browser->>SettingsPage: Load Settings page
SettingsPage->>BillingService: Fetch billing status
BillingService-->>SettingsPage: Return billing status
SettingsPage->>User: Display Account section
%% Flow 2: Team Setup Deep Link
User->>PaymentProvider: Purchase Team plan
PaymentProvider-->>Browser: Redirect to /settings?tab=team&team_setup=true
Browser->>SettingsPage: Load with team_setup param
SettingsPage->>SettingsPage: Set autoOpenTeamSetup=true
SettingsPage->>Browser: Navigate (replace) to /settings?tab=team
SettingsPage->>TeamDashboard: Render with autoOpenSetup=true
TeamDashboard->>TeamDashboard: Auto-open TeamSetupDialog
TeamDashboard->>User: Display team creation form
%% Flow 3: API Credits Purchase (Web/Desktop)
User->>ApiKeyDashboard: Click purchase credits
ApiKeyDashboard->>PaymentProvider: Create checkout session
PaymentProvider-->>Browser: Redirect to /settings?tab=api&credits_success=true
Browser->>SettingsPage: Load with credits_success param
SettingsPage->>SettingsPage: Set showApiCreditSuccessMessage=true
SettingsPage->>ApiKeyDashboard: Render with showCreditSuccessMessage=true
ApiKeyDashboard->>User: Display success message
%% Flow 4: API Credits Purchase (Mobile - BROKEN)
User->>ApiKeyDashboard: Click purchase credits (mobile)
ApiKeyDashboard->>PaymentProvider: Create checkout with mobile URLs
Note over PaymentProvider: successUrl: https://trymaple.ai/payment-success-credits
PaymentProvider-->>Browser: Redirect to /payment-success-credits
Browser->>Browser: 404 ERROR - Route does not exist
Browser->>User: Show error page
| // Let ApiCreditsSection manage its own 5s hide timer; we just prevent re-show on remounts. | ||
| const timer = setTimeout(() => setShowApiCreditSuccessMessage(false), 6000); | ||
| return () => clearTimeout(timer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timer duration here (6000ms) is longer than the ApiCreditsSection's internal timer (5000ms at line 52 of ApiCreditsSection.tsx). This means the parent component will hide the success flag 1 second after the child component already hid its success message, which could cause timing inconsistencies if the logic changes.
Consider using the same duration (5000ms) for consistency, or better yet, let only the ApiCreditsSection manage the success message lifetime since it owns the UI rendering.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/_auth.settings.tsx
Line: 145:147
Comment:
The timer duration here (6000ms) is longer than the ApiCreditsSection's internal timer (5000ms at line 52 of ApiCreditsSection.tsx). This means the parent component will hide the success flag 1 second after the child component already hid its success message, which could cause timing inconsistencies if the logic changes.
Consider using the same duration (5000ms) for consistency, or better yet, let only the ApiCreditsSection manage the success message lifetime since it owns the UI rendering.
How can I resolve this? If you propose a fix, please make it concise.| if (!team_setup || !os.auth.user) return; | ||
| setAutoOpenTeamSetup(true); | ||
| navigate({ to: "/settings", search: { tab: "team" }, replace: true }); | ||
| }, [team_setup, os.auth.user, navigate]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This effect doesn't wait for the navigation to complete before setting autoOpenTeamSetup. If navigation is slow or fails, the state will still be set to true. Consider setting the state after navigation completes, or handle the case where navigation might fail.
Also, the replace: true ensures the query param is removed, but the effect will run again if the user navigates back and forth to this page with the param. Consider adding a ref to track if this has already been handled in the current session.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/_auth.settings.tsx
Line: 133:136
Comment:
This effect doesn't wait for the navigation to complete before setting `autoOpenTeamSetup`. If navigation is slow or fails, the state will still be set to true. Consider setting the state after navigation completes, or handle the case where navigation might fail.
Also, the `replace: true` ensures the query param is removed, but the effect will run again if the user navigates back and forth to this page with the param. Consider adding a ref to track if this has already been handled in the current session.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
The route tree ( Impact: Mobile users (iOS/Android) purchasing API credits will be redirected to a non-existent route, breaking the payment flow. Suggested fix: Either:
Option 3 is simplest and maintains consistency with the web flow. Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src/components/apikeys/ApiCreditsSection.tsx
Line: 105:107
Comment:
The mobile payment redirect URLs reference `/payment-success-credits` which does not exist as a route in the application. This will cause mobile users to see a 404 error after purchasing API credits.
The route tree (`routeTree.gen.ts`) only includes `/payment-success` and `/payment-canceled`, but not `/payment-success-credits`. Additionally, the `DeepLinkHandler.tsx` is hardcoded to redirect payment success deep links to `/pricing?success=true` and doesn't differentiate between subscription and API credit purchases.
**Impact**: Mobile users (iOS/Android) purchasing API credits will be redirected to a non-existent route, breaking the payment flow.
**Suggested fix**: Either:
1. Create a new `/payment-success-credits` route that redirects to `/settings?tab=api&credits_success=true`, OR
2. Update the mobile URLs to use `/payment-success` and modify `DeepLinkHandler.tsx` to detect API credit purchases and redirect appropriately, OR
3. Change the mobile URLs to match the web/desktop flow: `https://trymaple.ai/settings?tab=api&credits_success=true`
Option 3 is simplest and maintains consistency with the web flow.
How can I resolve this? If you propose a fix, please make it concise. |
|
I like this refactor. Having a dedicated page makes it feel more permanent and accessible. |
Closes #380\n\n- Adds new authenticated /settings route with tabbed sections (Account, Billing, Team, API, History, About)\n- Simplifies AccountMenu to a single Settings entrypoint\n- Preserves deep-link flows for team setup + credits success
Summary by CodeRabbit
New Features
Changes
✏️ Tip: You can customize this high-level summary in your review settings.