Feature/persist user settings#403
Feature/persist user settings#403ahmetskilinc merged 12 commits intoMail-0:feature/persist-user-settingsfrom
Conversation
|
@CodyCodes95 is attempting to deploy a commit to the Zero Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a comprehensive user settings management feature. A new settings file defines a schema and functions to retrieve and save settings on the server. The GeneralPage component is refactored to use asynchronous form submission with enhanced state and error handling. A custom hook leverages SWR for fetching settings, providing default values when necessary. Additionally, the database schema is updated with a new userSettings table, utility functions for time zone handling are added, and authentication middleware now initializes default settings for new sessions. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant GP as GeneralPage
participant SS as saveUserSettings API
participant DB as Database
U->>GP: Submit settings form
GP->>SS: Call saveUserSettings(settings)
SS->>DB: Update/Insert settings
DB-->>SS: Return confirmation
SS-->>GP: Return result
GP->>GP: Refresh settings using mutate
sequenceDiagram
participant C as Client Session
participant A as Auth Middleware
participant DB as Database
C->>A: Request /sign-up
A->>A: Check newSession flag
A->>C: Extract timezone from headers
A->>DB: Insert default userSettings
DB-->>A: Acknowledge insertion
A-->>C: Continue authentication flow
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/mail/hooks/use-settings.ts (1)
17-31: Return a more descriptive error or result for advanced handling.This
updateSettingsfunction returns a boolean (trueorfalse) to indicate success or failure. While straightforward, it might be beneficial for more complex scenarios to return or throw an error object containing additional context (e.g., validation errors, partial success). This would allow consumers of this hook to handle different error cases individually.apps/mail/app/(routes)/settings/general/page.tsx (1)
146-155: IntegratingFormItemwith a switch is readable and user-friendly.The condensed layout, including labels and descriptions, clearly communicates what dynamic content entails. You may consider adding a tooltip or inline docs for advanced users who need further context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/mail/actions/settings.ts(1 hunks)apps/mail/app/(routes)/settings/general/page.tsx(6 hunks)apps/mail/hooks/use-settings.ts(1 hunks)packages/db/src/schema.ts(1 hunks)
🔇 Additional comments (17)
apps/mail/hooks/use-settings.ts (3)
1-5: Imports appear well-organized and necessary.All imported modules are clearly referenced, and there is no apparent redundancy. Good job keeping the external dependencies minimal and focused.
6-16: SWR usage and errorhandling approach are sound.Using
useSWRto fetch and cache user settings is well-structured. You handle potential errors gracefully, logging them and rethrowing if necessary. Consider customizing theuseSWRconfiguration for refresh intervals or revalidation if real-time updates become necessary.
33-39: Returning hook state in a consolidated object is clean and intuitive.This pattern makes consuming code simpler and ensures consistent naming. The structure is easy to follow, and pairing
isPendingwithisLoadingis a clear distinction of states.apps/mail/app/(routes)/settings/general/page.tsx (6)
21-21: Excellent introduction of theuseSettingshook.Importing the new custom hook keeps the component lean and maintains separation of concerns for data fetching and state updates.
39-39: Destructuring the hook output is succinct.This simplifies the component’s state management and clearly communicates the available values:
settings,updateSettings, andisPending.
49-49: Binding formvaluesdirectly tosettingsis straightforward.Leveraging
react-hook-form'svaluesfor initialization helps ensure consistency between persisted user settings and the UI.
53-53: Inline call toupdateSettings(values)streamlines the submission flow.By delegating all save logic to the hook, the component remains focused on rendering and event handling.
85-86: Disabling the button withisPendingprevents repeat submissions.Providing feedback to the user (the button text changes to "Saving...") enhances user experience and avoids accidental multiple saves.
161-172: Consistent styling for the external images form control.The same approach for displaying a label, description, and toggle fosters uniformity across the settings page.
packages/db/src/schema.ts (2)
89-90: No functional changes introduced here.These lines appear to be part of the closing bracket and a blank line for the
summarytable definition.
91-102: NewuserSettingstable is nicely structured.You cover all necessary fields, including sensible defaults (e.g., "en" for language, false for dynamic content). This table design is flexible enough to store more user preferences in the future.
apps/mail/actions/settings.ts (6)
1-2: “use server” directive is suitable for server-side logic.This explicitly indicates server-only functions, ensuring that sensitive operations like database queries remain on the server.
3-9: Imports are minimal and well curated.Each import is purposeful for schema handling, authentication, and database access. This keeps dependencies organized.
10-15: Zod schema forsettingsenhances validation clarity.Leveraging Zod ensures consistent data formatting and simplifies debugging by catching invalid data early.
17-17: ExportingUserSettingsfosters type consistency across the codebase.Having a single source of truth for user settings helps avoid mismatched types.
19-60:getUserSettingsfunction handles defaults gracefully.You validate session authenticity, fetch the database entry if available, and fall back to a sensible default. The error handling is also robust.
62-117:saveUserSettingssupports both creation and updates seamlessly.Validating the input schema and checking for existing settings ensures data integrity. Throwing general errors maintains a secure boundary without exposing sensitive details.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/mail/hooks/use-settings.ts (1)
8-11: Consider null safety improvement for userIdWhile the current implementation works, there's an edge case where
userIdmight be undefined initially during authentication.const userId = session?.user.id; const { data, error, isLoading, mutate } = useSWR( - userId ? [`user-settings`, userId] : null, + userId ? [`user-settings-${userId}`] : null, async () => {This alternative approach combines the key into a single string, which is a bit more standard with SWR. Both approaches work correctly, but a single string key can be easier to debug in development tools.
apps/mail/app/(routes)/settings/general/page.tsx (1)
46-53: Consider handling loading state for initial form values.The form has default values that will be overridden by the
settingsfrom the hook, but there's no handling for the initial loading state whensettingsmight be undefined.const form = useForm<z.infer<typeof formSchema>>({ resolver: zodResolver(formSchema), defaultValues: { language: "en", timezone: "UTC", dynamicContent: false, externalImages: true, }, - values: settings, + values: settings || undefined, }); + // Reset form with fetched settings when they become available + React.useEffect(() => { + if (settings) { + form.reset(settings); + } + }, [settings, form]);This approach avoids potential issues when
settingsis undefined during initial render, and ensures the form is properly updated once settings are loaded.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/app/(routes)/settings/general/page.tsx(6 hunks)apps/mail/hooks/use-settings.ts(1 hunks)
🔇 Additional comments (5)
apps/mail/hooks/use-settings.ts (1)
5-27: Good implementation of the settings hook with proper error handling.The implementation of
useSettingsis clean and follows React best practices using SWR for data fetching. I especially like the error handling in the fetcher function, which both logs the error and rethrows it for proper error state management.The hook correctly includes the user ID in the SWR key array, addressing a previous review comment. This ensures that settings are properly cached per user and will refetch if the user changes.
apps/mail/app/(routes)/settings/general/page.tsx (4)
55-67: Well-structured async form submission with proper error handling.The implementation of the async
onSubmitfunction is robust with a good error handling pattern:
- Setting pending state at the beginning
- Using try/catch to handle errors
- Showing appropriate toast messages for success and failure cases
- Properly cleaning up the pending state in a finally block
The use of
mutate()after saving is crucial for keeping the UI in sync with the backend.
42-42: Good use of the settings hook.The destructuring of
settingsandmutatefrom theuseSettingshook is clean and effective. This approach centralizes the settings management logic in the hook rather than duplicating it in each component.
97-98: Improved button state management.The button's disabled state and text now properly reflect the form submission state using
isPending. This provides good user feedback during the save operation.
159-160: Minor style improvement for form items.The styling of the form items has been improved with
bg-popoverand better flex layout, making the UI more consistent and visually appealing.Also applies to: 174-175
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/mail/utils/timezones.ts (1)
5-89: Consider adding a fallback for the browser timezone function.While the timezone implementation is comprehensive, the
getBrowserTimezone()function might return unexpected values in environments with limited locale support.-export const getBrowserTimezone = () => Intl.DateTimeFormat().resolvedOptions().timeZone; +export const getBrowserTimezone = () => { + try { + return Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC'; + } catch (e) { + console.warn('Could not detect browser timezone:', e); + return 'UTC'; + } +};apps/mail/actions/settings.ts (1)
49-52: Consider more specific error handling.The error handling is secure by not leaking implementation details, but for debugging purposes, you might want to add more specific error types or codes.
catch (error) { console.error("Failed to fetch user settings:", error); - throw new Error("Failed to fetch user settings"); + // Add error type or code for better debugging + throw new Error(`Failed to fetch user settings: ${error instanceof Error ? error.name : 'Unknown error'}`); }Also applies to: 107-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/mail/actions/settings.ts(1 hunks)apps/mail/app/(routes)/settings/general/page.tsx(6 hunks)apps/mail/hooks/use-settings.ts(1 hunks)apps/mail/utils/timezones.ts(1 hunks)
🔇 Additional comments (17)
apps/mail/utils/timezones.ts (2)
3-3: Nice use of Intl API for browser timezone detection.The
getBrowserTimezonefunction uses the standard Intl API which is well-supported across modern browsers.
5-89: Comprehensive timezone coverage with good organization.The timezone list is well-organized by region with appropriate GMT offsets. The comment about not overloading the select provides good context for why this isn't an exhaustive list.
apps/mail/hooks/use-settings.ts (3)
6-39: Well-structured custom hook with proper error handling.The hook correctly uses SWR for data fetching with the user ID in the cache key, provides sensible defaults when no settings exist, and properly handles errors.
11-11: Good cache key implementation with user ID.Including the user ID in the SWR cache key is essential for proper cache invalidation when switching between users.
16-23: Sensible defaults with browser timezone detection.The hook provides sensible default values when no user settings are found, including detecting the browser's timezone.
apps/mail/app/(routes)/settings/general/page.tsx (7)
18-20: Good import organization for new settings functionality.The imports for the settings functionality are well-organized with other related imports.
42-43: State management updated to use custom hook.The component now uses the
useSettingshook to manage settings state and loads existing settings from the database.
47-54: Form values properly initialized with persisted settings.The form's default values are properly set with the browser timezone as fallback, and the form values are updated with persisted settings when available.
56-68: Robust error handling in submit function.The submit function now properly handles asynchronous operations, manages loading state, and provides appropriate user feedback through toast notifications.
98-99: Button state properly managed during submission.The submit button is correctly disabled during form submission to prevent multiple submissions.
135-138: Improved layout with responsive width.Changing from fixed width to
w-fitfor the timezone selector improves the UI's responsiveness.
141-145: Dynamic timezone options from centralized source.Using
Object.entries(TIMEZONES)to generate select options is more maintainable than hardcoded values.apps/mail/actions/settings.ts (5)
10-17: Well-defined schema with appropriate types.The Zod schema provides clear validation rules for user settings and exports a TypeScript type for type safety throughout the application.
19-53: Secure implementation of getUserSettings.The function properly validates the user session before retrieving settings and includes appropriate error handling.
55-110: Robust implementation of saveUserSettings.The function includes session validation, input validation with Zod, and proper error handling. The update/insert logic based on existing settings is well-implemented.
70-71: Good input validation with Zod schema.Using the Zod schema to validate settings before saving ensures data integrity.
94-96: Good practice using UUID for database records.Using crypto.randomUUID() for new database records is a secure approach for generating unique identifiers.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/mail/lib/auth.ts (1)
146-171: Consider adding error handling to the settings insertion logic.The implementation for creating default user settings during sign-up is well-structured and aligns with the PR objective of persisting user settings. However, there's no error handling around the database operation. If the insert fails, it could potentially disrupt the sign-up flow.
Consider adding a try/catch block:
hooks: { after: createAuthMiddleware(async (ctx) => { // all hooks that run on sign-up routes if (ctx.path.startsWith("/sign-up")) { // only true if this request is from a new user const newSession = ctx.context.newSession; if (newSession) { // get timezone from vercel's header const headerTimezone = ctx.headers?.get("x-vercel-ip-timezone"); // get the timezone from the TIMEZONES object, fallback to UTC const timezone = TIMEZONES[headerTimezone as keyof typeof TIMEZONES] ?? "UTC"; // write default settings against the user + try { await db.insert(userSettings).values({ id: crypto.randomUUID(), userId: newSession.user.id, timezone, language: "en", dynamicContent: true, externalImages: true, createdAt: new Date(), updatedAt: new Date(), }); + } catch (error) { + console.error("Failed to insert default user settings:", error); + // Consider how to handle this error - should it abort the sign-up or continue? + } } } }), },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/lib/auth.ts(3 hunks)
🔇 Additional comments (3)
apps/mail/lib/auth.ts (3)
2-3: Appropriate imports added for the new functionality.The imports for
userSettings,createAuthMiddleware, andTIMEZONESare correctly added to support the new feature for persisting user settings during sign-up.Also applies to: 6-6
32-32: Minor formatting change for consistency.The formatting of the
tenantIdvalue has been adjusted, likely for consistency with other string values in the codebase.
156-167: Well-implemented default user settings with sensible defaults.The code correctly uses the detected timezone from Vercel headers (with UTC fallback) and sets reasonable defaults for language and content preferences. This implementation aligns well with the PR objective of persisting user settings.
|
LGTM, conflicts please |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (71)
apps/mail/app/api/auth/early-access/route.ts (1)
38-38: Consider removing or simplifying debug logging in productionWhile comprehensive logging helps during development, consider conditionally logging this detailed IP information only in development environments to avoid cluttering production logs.
- console.log("Request from IP:", ip, req.headers.get("x-forwarded-for"), req.headers.get('CF-Connecting-IP')); + if (process.env.NODE_ENV === "development") { + console.log("Request from IP:", ip, req.headers.get("x-forwarded-for"), req.headers.get('CF-Connecting-IP')); + }apps/mail/app/api/auth/early-access/count/route.ts (2)
17-37: Well-implemented rate limiting with proper error handlingThe rate limiting implementation includes:
- Proper IP extraction with fallback validation
- Appropriate error responses for missing IPs
- Well-structured rate limit headers in responses
- Clear error messaging for rate-limited requests
However, the same detailed IP logging issue is present here as in the other file.
- console.log("Request from IP:", ip, req.headers.get("x-forwarded-for"), req.headers.get('CF-Connecting-IP')); + if (process.env.NODE_ENV === "development") { + console.log("Request from IP:", ip, req.headers.get("x-forwarded-for"), req.headers.get('CF-Connecting-IP')); + }
41-41: Consider adding rate limit headers to successful responsesFor consistency in API behavior, consider adding the rate limit headers to successful responses as well. This allows clients to properly track their rate limit usage.
- return NextResponse.json({ count: signupCount }, { status: 200 }); + return NextResponse.json({ count: signupCount }, { + status: 200, + headers + });.github/workflows/ci.yml (4)
1-5: Workflow Naming & Trigger Configuration:The workflow is named
autofix.ciand appropriately triggers on both pull requests and pushes. Definingpermissions: {}is a good security practice to grant minimal privileges. However, consider pinning tools (such as Bun) to a specific version rather than usinglatestto avoid potential breaking changes in future releases.
13-17: Bun Setup Configuration:The step that sets up Bun with
oven-sh/setup-bun@v1is configured correctly. However, settingbun-versiontolatestmight introduce unexpected updates. It is recommended to pin to a known stable version for consistency and to prevent unforeseen breakages.
18-20: Execution of Formatting Commands:Running
bun installfollowed bybun run formatin a multi-line run step is clear and straightforward. Just ensure that these commands correctly exit with a non-zero status when formatting issues are found so that the workflow can fail as expected if needed.
24-24: Extraneous Line at File End:The file appears to contain an extraneous line with just "24" at the end. Please verify that this is not an unintended artifact and remove it if unnecessary.
apps/mail/app/(auth)/zero/signup/page.tsx (3)
12-16: Consider stricter validations.
Current Zod checks for minimum lengths look fine, but you might consider validating thenamefield for special characters or spacing to improve data quality.
18-37: Implement actual signup logic.
TheonSubmitfunction only displays a toast but does not actually handle signup. Consider integrating sign-up logic or a call to an API endpoint to store user details.Would you like help in adding an API call for user registration?
39-121: Update the prompt for existing users.
In a signup form, the text "Don't have an account?" is typically replaced with "Already have an account?" to direct users to the login page if they are not new.Apply this diff to align with expected user flows:
- Don't have an account?{' '} + Already have an account?{' '}apps/mail/app/(auth)/login/login-client.tsx (1)
59-124: Optional chaining consistency.
At line 98, consider usingsession?.connectionIdinstead of(session && session.connectionId)to match the syntax in line 93 and reduce redundant checks.-if (isPending || (session && session.connectionId)) return null; +if (isPending || session?.connectionId) return null;🧰 Tools
🪛 Biome (1.9.4)
[error] 98-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/mail/.env.example (1)
27-31: Add documentation for AI-related environment variables.The newly added OpenAI environment variables appear unrelated to the PR's primary objective of persisting user settings. Consider adding more specific comments to explain what the
AI_SYSTEM_PROMPTis used for and its expected format to help other developers.# OpenAI API Key OPENAI_API_KEY= -#AI PROMPT -AI_SYSTEM_PROMPT="" +# AI System Prompt - Instructions provided to the AI model when generating responses +AI_SYSTEM_PROMPT="You are a helpful email assistant that helps with email composition."apps/mail/app/error.tsx (1)
6-6: Avoid shadowing the global Error object.The parameter name
errorshadows the globalErrorconstructor, which can lead to confusion in the codebase.-export default function Error({ error, reset }: { error: Error; reset: () => void }) { +export default function Error({ error: errorObj, reset }: { error: Error; reset: () => void }) {Then update the references to
errorin the component to useerrorObjinstead.🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
apps/mail/components/create/editor.link-selector.tsx (1)
39-40: Optimize the conditional check using optional chainingThe current code has a redundant check on
inputRef.current. This can be simplified using optional chaining for better readability.- inputRef.current && inputRef.current?.focus(); + inputRef.current?.focus();🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/mail/app/globals.css (1)
192-208: Enhancement to gradient animationYou've replaced the static gradient with an animated one by adding keyframes and animation properties. This creates a more engaging visual effect.
One observation is that there are now two similar animations (
gradient-flowandgradient-animation) that do essentially the same thing. Consider consolidating them for better maintainability.For better code organization, consider consolidating the similar animations:
@keyframes gradient-flow { 0% { background-position: 0% 50%; } 50% { background-position: 100% 50%; } 100% { background-position: 0% 50%; } } .compose-gradient { background: linear-gradient(90deg, rgba(255, 213, 208, 1), rgba(219, 255, 228, 1), rgba(226, 214, 255, 1), rgba(255, 213, 208, 1)); background-size: 400% 400%; animation: gradient-flow 3s ease infinite; }Instead of having both
gradient-flowandgradient-animation(on lines 218-222) with identical keyframes, you could reuse the same keyframe definition.apps/mail/app/(routes)/settings/appearance/page.tsx (1)
31-37: Consider making onSubmit async and handling form errorsWhile the current implementation correctly shows loading state during form submission, it uses a setTimeout as a placeholder. When implementing the actual API call, make sure to properly handle errors and provide user feedback.
- function onSubmit(values: z.infer<typeof formSchema>) { - setIsSaving(true); - setTimeout(() => { - console.log(values); - setIsSaving(false); - }, 1000); - } + async function onSubmit(values: z.infer<typeof formSchema>) { + try { + setIsSaving(true); + // Implement actual API call here + console.log(values); + } catch (error) { + // Handle error with proper user feedback + console.error(error); + } finally { + setIsSaving(false); + } + }apps/mail/app/api/driver/types.ts (1)
4-26: Consider using specific types instead of 'any' for API methods.While the draft-related methods added to the MailManager interface align with the PR objective of adding persistence for settings, using
anytype decreases type safety. Consider defining specific interfaces for the draft data structure and responses.- createDraft(data: any): Promise<any>; - getDraft: (id: string) => Promise<any>; - listDrafts: (q?: string, maxResults?: number, pageToken?: string) => Promise<any>; + createDraft(data: DraftData): Promise<DraftResponse>; + getDraft: (id: string) => Promise<Draft>; + listDrafts: (q?: string, maxResults?: number, pageToken?: string) => Promise<DraftsListResponse>;Also, for consistency, consider unifying the method declaration styles - either use method syntax for all or property with function type for all.
apps/mail/components/draft/drafts.tsx (2)
43-44: Remove TypeScript type assertion.The
@ts-expect-errorcomment is used to suppress a TypeScript error, but it's better to fix the underlying type issue. Consider properly typing the event parameter.- useHotKey('Esc', (event) => { - // @ts-expect-error - event.preventDefault(); + useHotKey('Esc', (event: KeyboardEvent) => { + event.preventDefault();
81-121: Eliminate unnecessary Fragment.The code contains an unnecessary React Fragment that can be removed to simplify the code structure, as noted by the static analysis tools.
- {!searchMode && ( - <> - {mail.bulkSelected.length > 0 ? ( + {!searchMode && ( + mail.bulkSelected.length > 0 ? ( <> <div className="flex flex-1 items-center justify-center"> <span className="text-sm font-medium tabular-nums"> {t('common.mail.selected', { count: mail.bulkSelected.length })} </span> <Tooltip> <TooltipTrigger asChild> <Button variant="ghost" size="sm" className="text-muted-foreground ml-1.5 h-8 w-fit px-2" onClick={() => setMail({ ...mail, bulkSelected: [] })} > <X /> </Button> </TooltipTrigger> <TooltipContent>{t('common.mail.clearSelection')}</TooltipContent> </Tooltip> </div> <BulkSelectActions /> - ) : ( - <> + </> + ) : ( <div className="flex items-center gap-1.5"> <Button variant="ghost" className="md:h-fit md:px-2" onClick={() => setSearchMode(true)} onMouseEnter={() => searchIconRef.current?.startAnimation?.()} onMouseLeave={() => searchIconRef.current?.stopAnimation?.()} > <SearchIcon ref={searchIconRef} className="h-4 w-4" /> </Button> </div> - </> - )} - </> + ) )}🧰 Tools
🪛 Biome (1.9.4)
[error] 82-120: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
apps/mail/app/(routes)/settings/shortcuts/page.tsx (1)
27-30: Consider using a type-safe approach for dynamic translation keys.The type casting
as MessageKeyis a workaround that bypasses TypeScript's type checking. Consider defining an explicit map of action keys to translation keys to catch potential typos during development.- {t(`pages.settings.shortcuts.actions.${shortcut.action}` as MessageKey)} + {t(shortcutActionToTranslationKey(shortcut.action))}Where you would define:
function shortcutActionToTranslationKey(action: string): MessageKey { const keyMap: Record<string, MessageKey> = { newEmail: 'pages.settings.shortcuts.actions.newEmail', sendEmail: 'pages.settings.shortcuts.actions.sendEmail', // other mappings }; return keyMap[action] || ('pages.settings.shortcuts.actions.unknown' as MessageKey); }apps/mail/components/cookies/cookie-trigger.tsx (1)
32-43: Simplify unnecessary nested fragments.The fragments in this section can be simplified.
- {children || ( - <> - {variant === 'icon' ? ( - <Cookie className="h-3 w-3" /> - ) : ( - <> - <Cookie className="mr-2 h-4 w-4" /> - Cookie Settings - </> - )} - </> - )} + {children || ( + variant === 'icon' ? ( + <Cookie className="h-3 w-3" aria-label="Cookie Settings" /> + ) : ( + <> + <Cookie className="mr-2 h-4 w-4" /> + Cookie Settings + </> + ) + )}Note the addition of
aria-labelfor accessibility in the icon-only version.🧰 Tools
🪛 Biome (1.9.4)
[error] 33-42: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
apps/mail/components/draft/drafts-list.tsx (7)
1-19: Consider removing unused imports or re-checking dependencies.
The import list looks appropriate for virtual scrolling, hooks, and toast notifications. However, confirm that all imports are actively used.
20-38: Check edge cases in regex-based highlights.
While thehighlightTextfunction works for many search cases, consider potential issues with special regex characters or large inputs.
40-57: Ensure accessibility attributes for interactive elements.
In theDraftcomponent, ensure relevant ARIA attributes or roles for interactivity, especially for keyboard users.
102-114: Validate the default page size usage.
Confirm thatdefaultPageSizeis sufficiently balanced for performance and user experience. If large, it could cause performance overhead.Do you want a performance breakdown script to test various page sizes?
176-198: Consolidate repeated markAsUnread calls.
The logic in the hotkeys for marking as unread is repeated. Extracting it into a helper function would reduce duplication and make code more maintainable.
224-230: Commented-out code for junk mail.
Remove or convert these sections into feature flags. Keeping commented-out blocks for a long time can reduce code clarity.
297-344: Consider clarifying "selectAllBelow" for user.
Selecting all items below might benefit from an explicit user prompt or highlight of the selected range to reduce confusion.apps/mail/components/create/ai-assistant.tsx (7)
1-12: Check lazy loading of large dependencies.
The combination oflucide-react,framer-motion, and other libraries might increase bundle size. Consider dynamic imports or code splitting.
13-33: Potential additions for type definitions.
These well-defined interfaces are helpful, but consider adding JSDoc or TSDoc comments for improved readability.
130-140: Consider dynamic content length.
When displaying generated content inContentPreview, consider limiting displayed lines or using a “show more” approach for large outputs.
210-265: Assess conversation state resets.
Frequent resets might remove valuable context from prior messages. Evaluate storing partial conversation history if valuable for subsequent AI requests.
346-360: Confirm subject suggestions.
Auto-generating subjects can be beneficial. Also consider letting the user confirm or override your suggestions.
362-399: Keep user interactions consistent.
Both “Reject” and “Refresh” create new states. Evaluate adding confirmation or short transition for clarity.
418-428: Accessibility for the Sparkles icon button.
Includearia-labelor relevant text for screen readers. Currently, the user only has a tooltip.apps/mail/components/create/editor-buttons.tsx (3)
1-19: Separate styling from logic.
The custom 'control-group' and 'button-group' classes might be better organized in external CSS or a styled component.
20-47: Potential plugin usage.
The Tiptap community offers official formatting and extension plugins. Consider using them to reduce boilerplate code for toggling marks.
207-220: Consider drag-and-drop.
Adding drag-and-drop for file attachments could further enhance usability if your UI requires frequent file uploads.apps/mail/components/cookies/toggle.tsx (2)
9-13: Consider adding prop defaults and improved documentation.
Currently, you rely on the parent to passchecked,disabled, andcategory. Adding default values (e.g.,checked = false,disabled = false) and basic JSDoc or TS doc comments for these props can help ensure clarity and reduce potential errors if parent components omit certain props.
17-21: Handle potential errors in the asynchronous state update.
updateCookiePreferencesmight fail unexpectedly (e.g., network issues or a server error). Consider adding a try/catch around the await call or otherwise providing user feedback for error scenarios (e.g., rolling back the toggle state if the call fails).apps/mail/components/cookies/cookie-dialog.tsx (4)
38-44: Validate the 1-second delay for the cookie consent banner.
A delayed banner can lead to a jarring user experience if it appears after a slight pause. Confirm that this timing aligns with design requirements, or consider presenting the banner immediately.
71-77: Consider reducing complexity of the<Dialog>block.
The dialog structure is large, mixing layout, text content, toggles, and logic. Breaking out sub-components (like a<CookieCategoryAccordion>or<CookieBanner>) can improve readability, maintainability, and testability.
124-145: Evaluate user feedback for partial acceptance of cookies.
Currently, you offer “Reject All” or “Accept All” plus a custom toggle for each category. Confirm if you need partial acceptance flows (e.g., only analytics or only functional cookies) for compliance. If so, ensure toggles and final states reflect partial acceptance accurately before saving.
189-191: Add an error handling fallback.
In case the user attempts to save preferences or handle cookies with an unexpected error, consider displaying an error alert or fallback for a better user experience. This is particularly important if network or storage operations fail.apps/mail/app/(auth)/login/early-access/page.tsx (1)
103-123: Ensure consistent password validation.
You’re requiring a minimum of 6 characters. If your security requirements change (e.g., 8 characters, special characters), ensure that logic matches. Additionally, consider server-side checks for strong password policies.apps/mail/app/layout.tsx (2)
34-37: Handle potential header unavailability.
headers()may not always returnx-user-countryorx-user-eu-region. You do handle a default'false', but be cautious if headers are missing, asget()can still returnnull. Confirm the desired fallback is correct.
55-55: Revisit the commented-out<CookieConsent>usage.
You have conditionally commented out<CookieConsent>ifisEuRegionis true. Confirm whether it should be part of your final flow or if features remain incomplete.Would you like a code snippet to conditionally render
<CookieConsent>more explicitly?apps/mail/app/api/notes/db.ts (2)
23-55: Potential concurrency note when setting note order.
createNotecorrectly calculates the next note order by referencing the highest order of existing user notes. Depending on the app's usage, consider if multiple concurrent insertions might cause unexpected ordering. Right now, the approach is sufficient for typical use cases.
105-157: Transaction-based reorder strategy is robust.
reorderNotesvalidates ownership of each note inside a transaction, preventing partial updates. This approach is well structured, but be mindful of simultaneous reorder requests, as concurrency might require row-level locking for perfect consistency.README.md (1)
126-283: Refined environment and database instructions.
- Language-specific fenced code blocks: Consider specifying the language (e.g.,
bash,env) to satisfy markdownlint requirements (e.g., line ~247).- Minor repeated phrase: If you want to address Style or Grammar per line ~113, break up or reword duplicates like “Access the App” to avoid repetition.
-``` +```bash🧰 Tools
🪛 LanguageTool
[uncategorized] ~218-~218: A comma might be missing here.
Context: ...ion string for migrations ``` For local development a connection string example is provided...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🪛 markdownlint-cli2 (0.17.2)
247-247: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
apps/mail/actions/ai.ts (2)
18-66: Consider refining the handling of multiple responses.If the AI returns multiple
"email"or"question"responses, they’re either concatenated or only the first match is used. You may want to handle each response type more systematically, especially if future expansions require multi-part replies. Also note that theisEditparameter is declared but not utilized in the function.
68-80: Ensure i18n support for fallback messages.
createJsonContentuses a hardcoded string for the fallback paragraph. Consider referencing a translated message from your i18n framework.apps/mail/components/create/create-email.tsx (2)
67-130: Consider preserving paragraph formatting from draft data.When loading a draft, the code inserts all content into a single paragraph node. This may lose formatting or styling from multi-paragraph drafts. If possible, store richer content in the database and reconstruct multi-paragraph data for a better user experience.
421-482: Partial subject override.
onContentGeneratedonly updates the subject if the current subject is empty, which may skip a relevant AI-provided subject if the user typed a placeholder. Confirm if preserving the original subject is the intended behavior.apps/mail/components/connection/add.tsx (1)
54-90: Validate external links and enhance accessibility.Consider adding
rel="noopener noreferrer"ortarget="_blank"to external links for better security and user experience. Additionally, includingaria-labelor descriptive text on the provider icons would improve accessibility.apps/mail/app/(routes)/settings/general/page.tsx (2)
51-60: Consider updating default values handlingThe form has both
defaultValuesandvaluesproperties set. While this works as thevaluesproperty overridesdefaultValues, it could lead to confusion for future maintenance.Consider simplifying by using only the
valuesproperty and handling loading states:const form = useForm<z.infer<typeof formSchema>>({ resolver: zodResolver(formSchema), - defaultValues: { - language: locale, - timezone: getBrowserTimezone(), - dynamicContent: false, - externalImages: true, - }, - values: settings, + values: settings || { + language: locale, + timezone: getBrowserTimezone(), + dynamicContent: false, + externalImages: true, + }, });
145-157: Consider extracting the switch form item to a reusable componentYou have duplicate code structure for the two switch form items. They follow the exact same pattern and only differ in the field name and labels.
Extract this pattern into a reusable component to reduce duplication:
type SettingsSwitchProps = { name: 'dynamicContent' | 'externalImages'; label: string; description: string; control: Control<z.infer<typeof formSchema>>; }; function SettingsSwitch({ name, label, description, control }: SettingsSwitchProps) { return ( <FormField control={control} name={name} render={({ field }) => ( <FormItem className="bg-popover flex flex-row items-center justify-between rounded-lg border p-4"> <div className="space-y-0.5"> <FormLabel className="text-base">{label}</FormLabel> <FormDescription>{description}</FormDescription> </div> <FormControl className="ml-4"> <Switch checked={field.value} onCheckedChange={field.onChange} /> </FormControl> </FormItem> )} /> ); }Then use it like:
<SettingsSwitch control={form.control} name="dynamicContent" label={t('pages.settings.general.dynamicContent')} description={t('pages.settings.general.dynamicContentDescription')} />Also applies to: 164-176
apps/mail/actions/mail.ts (1)
25-25: Consider consolidating error handling logicThe same error handling pattern is repeated in six different places. This could lead to maintenance issues if the logic needs to change.
Consider creating a helper function to handle these common error patterns:
function handleMailError(error: unknown, context: string): never { if (FatalErrors.includes((error as Error).message)) { await deleteActiveConnection(); } console.error(`Error ${context}:`, error); throw error; }Then use it like:
} catch (error) { handleMailError(error, "getting threads"); }Also applies to: 39-39, 51-51, 63-63, 74-74, 109-109
.github/CONTRIBUTING.md (2)
166-167: Missing word in TypeScript guidelinesThere seems to be a missing word in the guidelines sentence.
- Ensure all code passes type checking, as builds will check for types in the future + Ensure all code passes type checking, as builds will fail to check for types in the future🧰 Tools
🪛 LanguageTool
[uncategorized] ~167-~167: Possible missing preposition found.
Context: ...ict"no-explicit-any"in the future - Ensure all code passes type checking, as build...(AI_HYDRA_LEO_MISSING_TO)
140-141: Add syntax highlighting to code blockThe code block for the database connection string lacks a language specifier for proper syntax highlighting.
- ``` + ```env DATABASE_URL="postgresql://postgres:postgres@localhost:5432/zerodotemail"<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 140-140: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </blockquote></details> <details> <summary>apps/mail/app/api/notes/index.ts (3)</summary><blockquote> `23-31`: **Consider adding request validation** While the underlying `notesManager` likely performs validation, adding API-level validation would provide an additional layer of security and improve error messages. Consider using Zod schemas to validate incoming parameters before passing them to the notes manager: ```typescript import { z } from 'zod'; // Example schema for createNote const createNoteSchema = z.object({ threadId: z.string().min(1), content: z.string().min(1), color: z.string().default('default'), isPinned: z.boolean().default(false) }); // Then in the createNote method: async createNote( threadId: string, content: string, color: string = 'default', isPinned: boolean = false ): Promise<Note> { try { const validated = createNoteSchema.parse({ threadId, content, color, isPinned }); const userId = await getCurrentUserId(); return await notesManager.createNote( userId, validated.threadId, validated.content, validated.color, validated.isPinned ); } catch (error) { // Handle validation errors specifically if (error instanceof z.ZodError) { console.error(`Validation error creating note:`, error); throw new Error(`Invalid note data: ${error.message}`); } console.error(`Error creating note in thread ${threadId}:`, error); throw error; } }Also applies to: 33-41, 43-56, 58-69, 71-79, 81-91
5-5: Redundant type exportsYou're exporting the
Notetype twice - once explicitly on line 5 and then again with the wildcard export on line 94.- export type { Note } from './types'; // ...later in the file export * from './types';Keep only the wildcard export at the bottom since it already includes all types, including
Note.Also applies to: 94-94
27-30: Consider centralized error loggingEach method has similar error logging code that could be centralized for consistency and easier maintenance.
Create a helper function for error logging:
function logApiError(operation: string, details: string, error: unknown): void { console.error(`Error ${operation}${details ? ` ${details}` : ''}:`, error); } // Then use it in each method: } catch (error) { logApiError('getting notes', '', error); throw error; } // With details: } catch (error) { logApiError('creating note', `in thread ${threadId}`, error); throw error; }Also applies to: 37-40, 53-55, 66-68, 76-78, 88-90
apps/mail/app/api/driver/google.ts (4)
1-10: Consolidate documentation for imports andfromBase64Urlhelper.
The imports look consistent and thefromBase64Urlhelper function is logically correct. Consider adding a brief JSDoc comment to describe its purpose in converting base64 URL variants into standard base64 format.
23-36: Improve type safety and logging strategy infindHtmlBody.
Usingany[]forpartsreduces type safety. If possible, define a more specific type to help prevent runtime errors. Also, consider switching to a more structured logging approach or using error/warning logs only if the absence of HTML body is critical.
81-99: Handle missing environment variables more robustly.
When initializing OAuth2 credentials, consider validating thatGOOGLE_CLIENT_ID,GOOGLE_CLIENT_SECRET, andGOOGLE_REDIRECT_URIare present. Throwing an explicit error or logging a warning can prevent silent failures.
149-238: Refine consistency in MailManager sub-methods.
This block introduces multiple sub-methods (e.g.,getAttachment,markAsRead,markAsUnread, etc.). They mostly handle their own error scenarios well. For consistency and clarity, consider standardizing the structure for logging errors, especially in try/catch blocks, and ensure that each method returns consistent data or error objects.apps/mail/components/create/editor.tsx (1)
373-588: LargeEditorcomponent with sophisticated event handling.
The overall design is robust, handling image uploads, AI menu toggling, command shortcuts, etc. As more features are integrated (e.g., slash commands, AI suggestions, attachments), consider breaking out specialized sub-components or hooks to improve maintainability and testability.apps/mail/app/(full-width)/contributors/page.tsx (3)
142-150: Consider retrieving additional contributor pages beyond page 2.
Currently, the logic only fetches the second page if there are exactly 100 contributors on the first page, but some repositories might exceed 200 contributors. Handling subsequent pages would ensure comprehensive data coverage.if (initialContributors?.length === 100) { - ? `https://api.github.com/repos/${REPOSITORY}/contributors?per_page=100&page=2` + ? `https://api.github.com/repos/${REPOSITORY}/contributors?per_page=100&page=2` // or beyond
195-203: Enhance error handling for rate-limiting scenarios.
When fetching multiple endpoints from GitHub’s public API without authentication, requests may be rate-limited. Consider detecting HTTP 403 status codes (or X-RateLimit headers) to differentiate rate-limit from other errors, and handle them appropriately (e.g., by showing a dedicated “rate-limit reached” message or a retry strategy).
274-307: Revisit random fallback data in production.
Using random fallback data for stars, forks, etc., may confuse end-users if the GitHub API fails in non-development environments. Consider static placeholders or a more user-friendly message in production builds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (116)
.devcontainer/Dockerfile(1 hunks).github/CONTRIBUTING.md(4 hunks).github/TRANSLATION.md(1 hunks).github/workflows/ci.yaml(0 hunks).github/workflows/ci.yml(1 hunks).gitignore(1 hunks).husky/pre-commit(1 hunks).npmrc(1 hunks).prettierignore(1 hunks).prettierrc(1 hunks)LICENSE(1 hunks)README.md(5 hunks)apps/mail/.env.example(1 hunks)apps/mail/.gitignore(1 hunks)apps/mail/LICENSE(0 hunks)apps/mail/actions/ai.ts(1 hunks)apps/mail/actions/brain.ts(1 hunks)apps/mail/actions/connections.ts(1 hunks)apps/mail/actions/cookies.ts(1 hunks)apps/mail/actions/drafts.ts(1 hunks)apps/mail/actions/mail.ts(8 hunks)apps/mail/actions/notes.ts(1 hunks)apps/mail/actions/utils.ts(1 hunks)apps/mail/app/(auth)/login/early-access/page.tsx(1 hunks)apps/mail/app/(auth)/login/login-client.tsx(1 hunks)apps/mail/app/(auth)/login/page.tsx(1 hunks)apps/mail/app/(auth)/signup/page.tsx(0 hunks)apps/mail/app/(auth)/signup/verify/page.tsx(0 hunks)apps/mail/app/(auth)/zero/login/page.tsx(1 hunks)apps/mail/app/(auth)/zero/signup/page.tsx(1 hunks)apps/mail/app/(error)/not-found.tsx(1 hunks)apps/mail/app/(full-width)/contributors/page.tsx(1 hunks)apps/mail/app/(full-width)/layout.tsx(1 hunks)apps/mail/app/(full-width)/privacy/page.tsx(1 hunks)apps/mail/app/(full-width)/terms/page.tsx(1 hunks)apps/mail/app/(routes)/create/layout.tsx(1 hunks)apps/mail/app/(routes)/create/page.tsx(1 hunks)apps/mail/app/(routes)/developer/page.tsx(1 hunks)apps/mail/app/(routes)/layout.tsx(1 hunks)apps/mail/app/(routes)/mail/[folder]/page.tsx(1 hunks)apps/mail/app/(routes)/mail/create/page.tsx(1 hunks)apps/mail/app/(routes)/mail/draft/page.tsx(1 hunks)apps/mail/app/(routes)/mail/layout.tsx(1 hunks)apps/mail/app/(routes)/mail/page.tsx(1 hunks)apps/mail/app/(routes)/mail/under-construction/[path]/back-button.tsx(1 hunks)apps/mail/app/(routes)/mail/under-construction/[path]/page.tsx(1 hunks)apps/mail/app/(routes)/settings/[...settings]/page.tsx(1 hunks)apps/mail/app/(routes)/settings/appearance/page.tsx(1 hunks)apps/mail/app/(routes)/settings/connections/page.tsx(1 hunks)apps/mail/app/(routes)/settings/general/page.tsx(1 hunks)apps/mail/app/(routes)/settings/layout.tsx(1 hunks)apps/mail/app/(routes)/settings/notifications/page.tsx(1 hunks)apps/mail/app/(routes)/settings/security/page.tsx(1 hunks)apps/mail/app/(routes)/settings/shortcuts/page.tsx(1 hunks)apps/mail/app/api/auth/early-access/count/route.ts(1 hunks)apps/mail/app/api/auth/early-access/route.ts(3 hunks)apps/mail/app/api/driver/google.ts(1 hunks)apps/mail/app/api/driver/index.ts(1 hunks)apps/mail/app/api/driver/types.ts(1 hunks)apps/mail/app/api/notes/db.ts(1 hunks)apps/mail/app/api/notes/index.ts(1 hunks)apps/mail/app/api/notes/types.ts(1 hunks)apps/mail/app/api/og/home/route.tsx(1 hunks)apps/mail/app/api/v1/mail/auth/[providerId]/callback/route.ts(2 hunks)apps/mail/app/api/v1/mail/auth/[providerId]/init/route.ts(1 hunks)apps/mail/app/error.tsx(1 hunks)apps/mail/app/global-error.tsx(1 hunks)apps/mail/app/globals.css(1 hunks)apps/mail/app/layout.tsx(1 hunks)apps/mail/app/not-found.tsx(1 hunks)apps/mail/app/page.tsx(1 hunks)apps/mail/components/connection/add.tsx(1 hunks)apps/mail/components/context/command-palette-context.tsx(1 hunks)apps/mail/components/context/sidebar-context.tsx(1 hunks)apps/mail/components/context/thread-context.tsx(1 hunks)apps/mail/components/cookies/cookie-dialog.tsx(1 hunks)apps/mail/components/cookies/cookie-trigger.tsx(1 hunks)apps/mail/components/cookies/toggle.tsx(1 hunks)apps/mail/components/create/ai-assistant.tsx(1 hunks)apps/mail/components/create/ai-chat.tsx(1 hunks)apps/mail/components/create/ai-textarea.tsx(1 hunks)apps/mail/components/create/create-email.tsx(1 hunks)apps/mail/components/create/editor-buttons.tsx(1 hunks)apps/mail/components/create/editor-menu.tsx(1 hunks)apps/mail/components/create/editor.colors.tsx(1 hunks)apps/mail/components/create/editor.css(0 hunks)apps/mail/components/create/editor.link-selector.tsx(1 hunks)apps/mail/components/create/editor.node-selector.tsx(1 hunks)apps/mail/components/create/editor.text-buttons.tsx(1 hunks)apps/mail/components/create/editor.tsx(1 hunks)apps/mail/components/create/prosemirror.css(2 hunks)apps/mail/components/create/selectors/link-selector.tsx(1 hunks)apps/mail/components/create/selectors/math-selector.tsx(1 hunks)apps/mail/components/create/selectors/node-selector.tsx(1 hunks)apps/mail/components/create/selectors/text-buttons.tsx(1 hunks)apps/mail/components/create/slash-command.tsx(1 hunks)apps/mail/components/create/uploaded-file-icon.tsx(1 hunks)apps/mail/components/draft/draft.tsx(0 hunks)apps/mail/components/draft/draftHooks/useDrafts.ts(0 hunks)apps/mail/components/draft/drafts-list.tsx(1 hunks)apps/mail/components/draft/drafts.tsx(1 hunks)apps/mail/components/home/footer.tsx(1 hunks)apps/mail/components/home/hero-image.tsx(1 hunks)apps/mail/components/home/hero.tsx(1 hunks)apps/mail/components/home/navbar.tsx(1 hunks)apps/mail/components/icons/animated/align-vertical.tsx(1 hunks)apps/mail/components/icons/animated/archive.tsx(1 hunks)apps/mail/components/icons/animated/arrow-left.tsx(1 hunks)apps/mail/components/icons/animated/bell.tsx(1 hunks)apps/mail/components/icons/animated/book-text.tsx(1 hunks)apps/mail/components/icons/animated/cart.tsx(1 hunks)apps/mail/components/icons/animated/check-check.tsx(1 hunks)apps/mail/components/icons/animated/expand.tsx(1 hunks)apps/mail/components/icons/animated/filter.tsx(1 hunks)apps/mail/components/icons/animated/inbox.tsx(1 hunks)apps/mail/components/icons/animated/keyboard.tsx(1 hunks)
💤 Files with no reviewable changes (7)
- apps/mail/components/create/editor.css
- apps/mail/app/(auth)/signup/page.tsx
- apps/mail/components/draft/draft.tsx
- apps/mail/app/(auth)/signup/verify/page.tsx
- .github/workflows/ci.yaml
- apps/mail/components/draft/draftHooks/useDrafts.ts
- apps/mail/LICENSE
✅ Files skipped from review due to trivial changes (52)
- .prettierignore
- apps/mail/app/not-found.tsx
- .npmrc
- apps/mail/app/(routes)/mail/draft/page.tsx
- apps/mail/components/icons/animated/align-vertical.tsx
- .gitignore
- apps/mail/app/api/v1/mail/auth/[providerId]/init/route.ts
- apps/mail/app/(routes)/mail/under-construction/[path]/back-button.tsx
- apps/mail/app/api/driver/index.ts
- apps/mail/components/create/selectors/math-selector.tsx
- apps/mail/app/(routes)/mail/create/page.tsx
- apps/mail/app/page.tsx
- apps/mail/components/icons/animated/bell.tsx
- apps/mail/.gitignore
- apps/mail/components/create/uploaded-file-icon.tsx
- apps/mail/app/(routes)/settings/notifications/page.tsx
- apps/mail/app/(routes)/mail/page.tsx
- apps/mail/actions/connections.ts
- apps/mail/components/home/hero-image.tsx
- apps/mail/app/(routes)/create/layout.tsx
- apps/mail/components/icons/animated/book-text.tsx
- apps/mail/components/icons/animated/arrow-left.tsx
- .github/TRANSLATION.md
- apps/mail/components/create/ai-textarea.tsx
- apps/mail/components/create/editor.text-buttons.tsx
- apps/mail/components/context/sidebar-context.tsx
- apps/mail/app/(routes)/create/page.tsx
- apps/mail/components/home/hero.tsx
- apps/mail/components/create/slash-command.tsx
- apps/mail/components/icons/animated/keyboard.tsx
- apps/mail/app/(routes)/mail/under-construction/[path]/page.tsx
- apps/mail/app/(routes)/developer/page.tsx
- apps/mail/components/create/selectors/link-selector.tsx
- .husky/pre-commit
- LICENSE
- apps/mail/app/(full-width)/terms/page.tsx
- apps/mail/components/icons/animated/inbox.tsx
- apps/mail/components/icons/animated/cart.tsx
- apps/mail/components/create/editor.colors.tsx
- apps/mail/components/icons/animated/filter.tsx
- apps/mail/app/(full-width)/layout.tsx
- apps/mail/app/(routes)/settings/layout.tsx
- apps/mail/components/icons/animated/archive.tsx
- apps/mail/app/(full-width)/privacy/page.tsx
- apps/mail/app/api/og/home/route.tsx
- apps/mail/components/icons/animated/expand.tsx
- apps/mail/components/create/editor-menu.tsx
- apps/mail/app/(routes)/mail/[folder]/page.tsx
- apps/mail/components/create/editor.node-selector.tsx
- apps/mail/components/create/ai-chat.tsx
- apps/mail/components/create/selectors/node-selector.tsx
- apps/mail/components/icons/animated/check-check.tsx
🧰 Additional context used
🧬 Code Definitions (26)
apps/mail/app/(auth)/zero/login/page.tsx (1)
apps/mail/components/ui/input.tsx (1) (1)
Input(31-31)
apps/mail/app/(routes)/settings/appearance/page.tsx (2)
apps/mail/components/ui/form.tsx (1) (1)
Form(163-163)apps/mail/components/theme/theme-switcher.tsx (1) (1)
ModeToggle(21-91)
apps/mail/app/api/notes/types.ts (2)
apps/mail/hooks/use-notes.tsx (1) (1)
Note(16-16)apps/mail/app/api/notes/index.ts (2) (2)
Note(5-5)notes(22-92)
apps/mail/actions/cookies.ts (1)
apps/mail/lib/cookies.ts (3) (3)
CookiePreferences(12-17)COOKIE_CONSENT_KEY(47-47)CookieCategory(3-3)
apps/mail/components/create/ai-assistant.tsx (2)
apps/mail/lib/ai.ts (1) (1)
generateConversationId(16-18)apps/mail/actions/ai.ts (1) (1)
generateAIEmailContent(18-66)
apps/mail/app/(auth)/login/page.tsx (2)
apps/mail/lib/auth-providers.ts (3) (3)
authProviders(29-79)isProviderEnabled(81-92)customProviders(18-27)apps/mail/app/(auth)/login/login-client.tsx (1) (1)
LoginClient(59-354)
apps/mail/components/context/command-palette-context.tsx (3)
apps/mail/types/index.ts (1) (1)
NavItem(13-19)apps/mail/components/ui/command.tsx (8) (8)
CommandDialog(140-140)CommandInput(143-143)CommandList(145-145)CommandEmpty(141-141)CommandGroup(142-142)CommandItem(144-144)CommandShortcut(147-147)CommandSeparator(146-146)apps/mail/components/ui/dialog.tsx (2) (2)
DialogTitle(97-97)DialogDescription(98-98)
apps/mail/components/home/footer.tsx (2)
apps/mail/components/icons/icons.tsx (2) (2)
Discord(30-38)apps/mail/components/theme/theme-toggle.tsx (1) (1)
ThemeToggle(13-47)
apps/mail/app/(auth)/zero/signup/page.tsx (1)
apps/mail/components/ui/input.tsx (1) (1)
Input(31-31)
apps/mail/components/draft/drafts-list.tsx (4)
apps/mail/types/index.ts (4) (4)
ThreadProps(96-102)MailListProps(90-92)MailSelectMode(94-94)InitialThread(64-79)apps/mail/lib/notes-utils.ts (1) (1)
formatDate(72-89)apps/mail/hooks/use-drafts.ts (1) (1)
useDrafts(53-114)apps/mail/actions/mail.ts (2) (2)
markAsUnread(57-67)markAsRead(45-55)
apps/mail/components/draft/drafts.tsx (3)
packages/db/src/schema.ts (1) (1)
session(17-28)apps/mail/hooks/use-drafts.ts (1) (1)
useDrafts(53-114)apps/mail/components/draft/drafts-list.tsx (1) (1)
DraftsList(102-400)
apps/mail/components/create/editor-buttons.tsx (3)
apps/mail/components/ui/popover.tsx (3) (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)apps/mail/components/ui/separator.tsx (1) (1)
Separator(26-26)apps/mail/components/create/uploaded-file-icon.tsx (1) (1)
UploadedFileIcon(35-67)
apps/mail/components/cookies/toggle.tsx (3)
apps/mail/lib/cookies.ts (1) (1)
CookieCategory(3-3)apps/mail/hooks/use-geo-location.ts (1) (1)
useGeoLocation(3-19)apps/mail/actions/cookies.ts (1) (1)
updateCookiePreferences(34-52)
apps/mail/app/(routes)/settings/security/page.tsx (1)
apps/mail/components/ui/form.tsx (6) (6)
Form(163-163)FormField(169-169)FormItem(164-164)FormLabel(165-165)FormDescription(167-167)FormControl(166-166)
apps/mail/actions/drafts.ts (1)
apps/mail/app/api/driver/google.ts (1) (1)
driver(81-502)
apps/mail/app/(routes)/layout.tsx (2)
apps/mail/components/context/command-palette-context.tsx (1) (1)
CommandPaletteProvider(210-216)apps/mail/lib/idb.ts (1) (1)
dexieStorageProvider(24-67)
apps/mail/app/(routes)/settings/general/page.tsx (4)
apps/mail/hooks/use-settings.ts (1) (1)
useSettings(6-39)apps/mail/utils/timezones.ts (2) (2)
getBrowserTimezone(3-3)TIMEZONES(6-89)apps/mail/actions/settings.ts (2) (2)
saveUserSettings(55-110)UserSettings(17-17)apps/mail/i18n/config.ts (2) (2)
Locale(18-18)availableLocales(26-29)
apps/mail/actions/ai.ts (1)
apps/mail/lib/ai.ts (1) (1)
generateEmailContent(21-120)
apps/mail/app/(auth)/login/early-access/page.tsx (3)
apps/mail/components/ui/input.tsx (1) (1)
Input(31-31)apps/mail/components/ui/input-otp.tsx (3) (3)
InputOTP(71-71)InputOTPGroup(71-71)InputOTPSlot(71-71)apps/mail/lib/utils.ts (1) (1)
cn(47-47)
apps/mail/components/connection/add.tsx (4)
apps/mail/components/ui/dialog.tsx (6) (6)
Dialog(89-89)DialogTrigger(93-93)DialogContent(94-94)DialogHeader(95-95)DialogTitle(97-97)DialogDescription(98-98)apps/mail/components/ui/button.tsx (1) (1)
Button(55-55)apps/mail/lib/utils.ts (1) (1)
cn(47-47)apps/mail/lib/constants.ts (1) (1)
emailProviders(100-106)
apps/mail/app/(routes)/settings/shortcuts/page.tsx (1)
apps/mail/config/shortcuts.ts (1) (1)
keyboardShortcuts(2-24)
apps/mail/components/create/create-email.tsx (3)
apps/mail/actions/drafts.ts (2) (2)
getDraft(35-49)createDraft(22-33)apps/mail/components/create/editor.tsx (1) (1)
Editor(373-587)apps/mail/components/create/ai-assistant.tsx (1) (1)
AIAssistant(210-478)
apps/mail/app/(auth)/login/login-client.tsx (2)
apps/mail/lib/auth-providers.ts (1) (1)
EnvVarInfo(1-5)apps/mail/components/icons/icons.tsx (2) (2)
GitHub(55-63)
apps/mail/components/create/editor.tsx (6)
apps/mail/components/create/uploaded-file-icon.tsx (1) (1)
UploadedFileIcon(35-67)apps/mail/lib/utils.ts (1) (1)
truncateFileName(82-89)apps/mail/components/ui/dialog.tsx (6) (6)
Dialog(89-89)DialogContent(94-94)DialogHeader(95-95)DialogTitle(97-97)DialogDescription(98-98)DialogFooter(96-96)apps/mail/components/ui/input.tsx (1) (1)
Input(31-31)apps/mail/components/create/slash-command.tsx (1) (1)
suggestionItems(19-75)apps/mail/components/create/editor-menu.tsx (1) (1)
EditorMenu(12-34)
apps/mail/app/(full-width)/contributors/page.tsx (2)
apps/mail/components/theme/theme-toggle.tsx (1) (1)
ThemeToggle(13-47)apps/mail/components/icons/icons.tsx (2) (2)
Discord(30-38)
apps/mail/app/api/notes/index.ts (2)
apps/mail/app/api/notes/types.ts (1) (1)
Note(1-11)apps/mail/app/api/notes/db.ts (1) (1)
notesManager(6-157)
🪛 Biome (1.9.4)
apps/mail/components/cookies/cookie-trigger.tsx
[error] 33-42: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
apps/mail/components/draft/drafts.tsx
[error] 82-120: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
apps/mail/components/create/editor-buttons.tsx
[error] 187-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/mail/app/(auth)/login/login-client.tsx
[error] 98-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/mail/app/error.tsx
[error] 6-6: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
apps/mail/components/create/editor.link-selector.tsx
[error] 39-39: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 LanguageTool
README.md
[grammar] ~113-~113: This phrase is duplicated. You should probably use “the App” only once.
Context: ...ew minutes the first time) 3. Access the App - The app will be available at [http://localhost:...
(PHRASE_REPETITION)
.github/CONTRIBUTING.md
[uncategorized] ~167-~167: Possible missing preposition found.
Context: ...ict "no-explicit-any" in the future - Ensure all code passes type checking, as build...
(AI_HYDRA_LEO_MISSING_TO)
🪛 markdownlint-cli2 (0.17.2)
README.md
247-247: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
.github/CONTRIBUTING.md
140-140: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (152)
.prettierrc (2)
3-3: Update: Enforcing Tab IndentationThe change
"useTabs": truecommits the project to using tabs for indentation. Please ensure that all team members are aware of this update so that code formatting remains consistent across the codebase.
6-6: Update: Standardizing to Single QuotesThe update
"singleQuote": trueenforces the usage of single quotes for string literals. Verify that this change is fully aligned with your team’s style guidelines and that any related tooling (e.g., ESLint, editor configs) is updated accordingly.apps/mail/app/api/auth/early-access/route.ts (3)
19-29: Well-implemented email validation functionThe new
isEmailfunction properly validates email formats using a comprehensive regex pattern. It correctly handles empty values and returns a boolean result.
33-37: Good enhancement to IP detectionSwitching to
CF-Connecting-IPis appropriate for Cloudflare-proxied applications. The added validation with error handling for missing IP addresses improves robustness.
65-68: Good addition of email format validationAdding format validation before processing improves error handling. This prevents invalid emails from reaching the database and returns appropriate error responses to clients.
apps/mail/app/api/auth/early-access/count/route.ts (3)
1-7: Well-structured imports with rate limitingThe addition of rate limiting imports is well-organized. Including NextRequest properly supports the updated function signature.
8-13: Good rate limiting configuration for public API endpointThe rate limit of 20 requests per minute with sliding window is appropriate for a public-facing endpoint. Using a distinct prefix for this endpoint ensures separate rate limiting from other API routes.
15-15: Function signature appropriately updatedThe GET function now correctly accepts a request parameter to support IP-based rate limiting.
.github/workflows/ci.yml (4)
6-9: Job & Runner Setup:The job named
prettierrunning onubuntu-latestis well-defined. Ensure that this runner environment contains all the necessary dependencies, especially given the mix of Bun and Node.js setups.
10-12: Source Code Checkout:Using
actions/checkout@v4to retrieve the repository code is correct and clearly specified.
17-18: Node.js Setup:Integrating
actions/setup-node@v4next ensures Node.js is available for any necessary tasks. This is appropriately placed following the Bun setup.
21-23: Autofix Commit Action:The step using
autofix-ci/action@v1with a commit message ("Apply Prettier format") is set up correctly to automatically commit code formatting changes. This enhances the CI process by ensuring code style consistency without manual intervention.apps/mail/app/(auth)/zero/signup/page.tsx (2)
1-11: Good use of the Next.js App Router's client-based approach.
Import statements and the'use client';directive appear correct, ensuring the form can use client-side functionality such asreact-hook-formandtoast.
124-141: Footer structure looks good.
The links to Terms of Service and Privacy Policy are placed consistently at the bottom. No issues identified.apps/mail/app/(auth)/zero/login/page.tsx (4)
1-11: Imports and setup appear correct.
The usage of'use client'and the imported libraries are consistent with typical Next.js + React Hook Form patterns.
12-16: Email validation is clear and concise.
Ensuring a valid email format helps catch user input errors early. Current approach withzodis sufficient.
18-33: Toast-based user feedback.
TheonSubmitfunction currently shows a toast but does not pass the user input to an authentication flow. If you plan to add real login logic later, consider a placeholder comment.
35-125: Form layout and styling look consistent.
The design follows the same style as the signup page, which helps maintain a unified user experience. No immediate issues found.apps/mail/app/(auth)/login/page.tsx (3)
1-2: Clear usage of imports.
Merging auth providers and custom providers is organized neatly.
4-39: Environment-based logic is well-structured.
UsingNODE_ENVto toggle production features is standard. All environment-driven checks appear to follow a straightforward pattern.
40-44: Delegation toLoginClientfosters maintainability.
Separating the UI/logic for provider options into its own component helps keep concerns isolated.apps/mail/app/(auth)/login/login-client.tsx (3)
1-34: Interfaces and imports look solid.
DefiningProvider,LoginClientProps, and using typed environment variable info helps ensure clarity and type safety.
36-58: Icon mapping strategy is straightforward.
Switching onproviderIdkeeps icons decoupled and maintainable. This approach is extendable to new providers in the future.
126-355: Comprehensive configuration messaging.
This approach effectively enumerates missing environment variables for required providers, aiding quick debugging. The toast flow for sign-in attempts is also a good user experience.apps/mail/app/(routes)/mail/layout.tsx (2)
1-2: Consistent code style with single quotesThe change from double quotes to single quotes in imports maintains consistency with the project's style guidelines.
7-9: Layout adjustments for user settingsThe padding change from
md:p-3tomd:py-3 md:pr-2provides more precise control over the layout spacing. The removal of theAISidebarcomponent simplifies the layout structure.Could you confirm if this layout change is directly related to the user settings persistence feature? It would help to understand the relationship between these UI changes and the core functionality being implemented.
apps/mail/app/(error)/not-found.tsx (4)
3-6: Added internationalization supportGreat addition of the
useTranslationshook fromnext-intl, which aligns with the user settings persistence feature for language preferences.
9-10: Properly initialized translation hookThe translation hook is correctly initialized and used within the component.
24-28: Replaced hardcoded text with translation keysExcellent implementation of translated content using translation keys instead of hardcoded text, which supports the language preference setting.
37-39: Translated UI control textConsistent implementation of translation for the "Go Back" button text.
apps/mail/app/(routes)/layout.tsx (1)
12-17:Details
❓ Verification inconclusive
Added error handling configuration for SWR
The addition of
shouldRetryOnError: falseto the SWR configuration prevents automatic retry of failed requests, which can improve user experience when settings operations fail.
🏁 Script executed:
#!/bin/bash # Check how settings are being fetched in the codebase echo "Checking for settings-related SWR usage..." rg -A 3 -B 3 "useSettings|getUserSettings" --type=ts --type=tsxLength of output: 188
I'll run a revised search to verify that there aren’t any other settings-related SWR hooks (like
useSettingsorgetUserSettings) elsewhere in the codebase. In particular, I'll try a search that avoids relying on built-in file type definitions for TSX files.If this search still produces no output, it would suggest that the SWR configuration change in
apps/mail/app/(routes)/layout.tsxis isolated. This supports the understanding that the addition ofshouldRetryOnError: falseis solely intended to improve error handling for that specific hook without affecting other parts of the codebase.
Enhanced SWR configuration to prevent automated retries on failed requests
The added
shouldRetryOnError: falseprevents SWR from automatically retrying requests when settings operations fail, thereby avoiding potential user experience issues. Please verify (manually if needed) that no other SWR hooks (e.g.,useSettingsorgetUserSettings) require similar error handling adjustments.
- Verify that this specialized configuration in
apps/mail/app/(routes)/layout.tsxis appropriate and consistent with the rest of the codebase.- Confirm that there are no other settings-related SWR usages that might need similar error handling.
🏁 Script executed:
#!/bin/bash echo "Searching for settings-related SWR usage in .ts and .tsx files..." rg -A 3 -B 3 "useSettings|getUserSettings" -g "*.tsx" -g "*.ts"Length of output: 3144
Verified SWR Global Error Handling Update
The change in
apps/mail/app/(routes)/layout.tsxto includeshouldRetryOnError: falsehas been verified. Our search confirms that settings-related SWR hooks (e.g., inapps/mail/hooks/use-settings.tsand its usage inapps/mail/app/(routes)/settings/general/page.tsx) rely on this global configuration. This update prevents automatic retries on failed requests and appears to be applied consistently, enhancing error handling for settings operations.Please perform manual verification to ensure that all SWR-based settings operations exhibit the intended behavior.
apps/mail/components/context/command-palette-context.tsx (7)
20-20: Good addition of internationalization support.Adding the
useTranslationshook is a necessary step to support the persisted language settings implemented in this PR.
67-67: Initialize translation hook for use throughout the component.This correctly sets up the translation function that will use the persisted language settings.
94-98: Good implementation of dynamic translations for command groups.Using the translation keys for the mail and settings groups ensures the UI properly reflects the user's language preference from persisted settings.
103-103: Check the conditional translation key comparison.When comparing group names with translated values, ensure that translation keys remain consistent across the application. This ensures the command palette will properly filter items based on the user's chosen language.
Also applies to: 115-116
136-137: Good implementation of screen reader accessibility with translations.Using translated values for dialog title and description improves accessibility for users with different language settings.
139-139: Complete internationalization of user-facing strings.All user-facing strings have been properly internationalized, ensuring the command palette fully respects the user's language preference from the newly persisted settings.
Also applies to: 141-141, 144-146, 174-174, 184-184, 187-187, 200-200
120-120: Update dependency array to include translation function.The dependency array correctly includes the translation function, ensuring the commands are recalculated when the language changes.
apps/mail/components/home/navbar.tsx (4)
25-25: UI spacing adjustment for better responsive layout.The padding adjustments (from
px-7topx-3andlg:px-20tolg:px-4) create a more consistent spacing system that will better accommodate different language text lengths from persisted user settings.
28-33: Reduced logo size for better UI balance.The logo has been reduced from
h-9 w-9toh-6 w-6, which creates better visual hierarchy, particularly important when accommodating potential text expansion from different language settings.
39-39: Updated hover styling for dark mode consistency.The class change from
hover:bg-gray-100todark:hover:bg-accentensures the hover state respects the user's theme preference, which aligns with the overall settings persistence goal.
68-68: Shortened 'Contact Us' to 'Contact' for mobile view.This change improves space efficiency on mobile devices, which is particularly helpful for accommodating longer text that may result from different language settings.
apps/mail/components/context/thread-context.tsx (4)
32-32: Added internationalization support via useTranslations hook.This addition is essential for supporting the persisted language settings implemented in this PR.
73-73: Good initialization of translation function.The translation function is properly initialized for use throughout the component.
115-118: Implemented internationalized toast messages.Toast messages now use translation keys, ensuring they reflect the user's language preference from the persisted settings.
157-158: All action labels properly internationalized.Email action labels have been updated to use the translation function, ensuring the context menu properly reflects the user's language preference from persisted settings.
Also applies to: 169-170, 181-182, 193-194, 201-202, 212-213, 222-223, 230-231, 238-239
apps/mail/components/create/prosemirror.css (6)
2-2: Improved editor padding and typography for better readability.The increased padding (
p-3instead ofp-2) and adjusted font weight (400 instead of 500) with a defined line height (1.4) create a more comfortable writing experience, which is important for supporting multilingual content from different language settings.Also applies to: 5-6
10-13: Updated placeholder text with better color contrast.The placeholder text has been changed to "Start your email here" with a specified color of #363636, which provides better accessibility and clearer guidance for users across different language settings.
196-204: Fixed placeholder visibility in list items.These new CSS rules properly handle placeholders within list contexts, ensuring a consistent editing experience regardless of the user's language settings.
217-221: Improved list item spacing and line height.Reducing the margin between list items from 1rem to 0.75rem and setting a consistent line height of 1.4 creates more compact lists that work well across different languages.
224-227: Adjusted paragraph spacing for better text flow.The reduced bottom margin for paragraphs (from 1.5rem to 1.25rem) with a consistent line height creates better vertical rhythm, which is important for different language text layouts.
230-236: Consistent list indentation for better structure.Setting consistent padding for unordered and ordered lists ensures proper indentation regardless of the language, supporting the multilingual capabilities enabled by the persisted language settings.
apps/mail/app/global-error.tsx (1)
1-16: Looks good: Appropriate global error handling implementation.This new global error component correctly follows Next.js conventions for handling application-wide errors. The detailed comment explaining the status code workaround is helpful for future developers.
apps/mail/actions/brain.ts (1)
26-30: Good defensive programming practice.Added proper validation for the environment variable before using it. This prevents potential runtime errors and provides a clear error message.
apps/mail/app/error.tsx (3)
1-5: Code styling improvements.The switch to single quotes and consistent formatting aligns with the project's style guidelines.
7-9: Consistent indentation applied.The indentation change from spaces to tabs aligns with the project's formatting standards.
11-28: Improved class organization.The reordering of class names and formatting of JSX improves readability while maintaining functionality.
apps/mail/app/api/v1/mail/auth/[providerId]/callback/route.ts (2)
1-1: Good use of TypeScripttypeimportUsing the
typekeyword for theNextRequestimport is a good practice as it ensures the type is only used for type checking and not included in the runtime bundle.
62-62:Details
❓ Verification inconclusive
Verify the impact of changing the redirect flow
The redirect URL has been changed from
/settings/connections?success=trueto/onboarding. This changes the user flow after account connection.Could you confirm that redirecting to the onboarding page after connecting an account is the intended behavior? This might impact users who are adding additional connections rather than setting up their first account.
🏁 Script executed:
#!/bin/bash # Check for other places where this redirect URL might be referenced rg -A 1 -B 1 "settings/connections\?success=true" --type ts --type tsxLength of output: 100
Action Required: Confirm the Updated Redirect Flow
The new code now redirects to
/onboardinginstead of/settings/connections?success=true. Note that the initial search for the old URL produced an error (unrecognized file type: tsx), so we couldn’t fully verify if there are any remaining references in the codebase. Please manually confirm that redirecting to the onboarding page after connecting an account is the intended behavior—especially ensuring that users adding additional connections aren’t adversely impacted by this change.apps/mail/app/(routes)/settings/appearance/page.tsx (1)
10-10: Good internationalization implementationThe addition of the
useTranslationshook and replacing hardcoded strings with translation keys is a great improvement for internationalization support. This will allow for easier localization of the application.Also applies to: 22-22, 42-47, 54-54
apps/mail/actions/cookies.ts (2)
6-32: Well-implemented cookie preferences retrieval with proper safeguardsThe implementation of
getCookiePreferencesfollows best practices:
- Establishes sensible defaults with necessary cookies always enabled
- Uses proper error handling for JSON parsing
- Ensures that necessary cookies remain enabled even if parsed preferences indicate otherwise
34-52: Good security implementation for cookie preference updatesThe
updateCookiePreferencesfunction properly:
- Prevents users from disabling necessary cookies (line 38-40)
- Reuses the existing
getCookiePreferencesfunction to maintain consistency- Correctly revalidates the related path to ensure UI updates
This implementation aligns with the PR objective of persisting user settings in a secure manner.
apps/mail/app/api/notes/types.ts (3)
1-11: Well-structured Note interface definitionThe
Noteinterface clearly defines all required properties with appropriate types. The use ofboolean | nullforisPinnedallows for three states which gives flexibility in the data model.
13-38: Comprehensive NotesManager interface with well-typed methodsThe interface provides a complete set of CRUD operations for notes management. The use of
Partial<Omit<...>>in theupdateNotemethod is particularly good as it:
- Allows for partial updates (only changing the fields that need updating)
- Restricts updates to only the mutable fields, preventing accidental modification of IDs and timestamps
1-38: Verify relevance to the PR objectiveWhile this is well-implemented, this notes-related type definition doesn't seem directly related to the PR objective of persisting settings for "language, timezone, dynamic content, and external images" as mentioned in the PR description.
If this file is intended to be part of this PR, could you clarify how it relates to the user settings persistence feature? Or is this file possibly included unintentionally?
apps/mail/actions/utils.ts (2)
9-9: Good practice to define error constantsDefining the
FatalErrorsarray as a constant improves code maintainability by centralizing error types that require special handling.
11-27: Well-structured error handling for connection managementThe
deleteActiveConnectionfunction:
- Properly checks for session validity before attempting operations
- Includes appropriate error logging
- Uses revalidation to ensure UI consistency
Is there a connection between this functionality and the PR objective of persisting user settings? This seems more focused on connection error handling rather than settings persistence.
apps/mail/app/(routes)/settings/[...settings]/page.tsx (2)
9-9: Good internationalization implementationAdding the
useTranslationshook enables localized error messages, which aligns with the PR objective of persisting language settings.Also applies to: 24-24
28-30: Improved error message localizationReplacing the hardcoded error message with a translation key improves the user experience by providing localized error messages based on user language preferences.
apps/mail/components/home/footer.tsx (3)
3-6: Well-organized imports with proper component separation.The organized imports with clear separation between component-specific imports, icons, and Next.js elements improves code readability and maintainability.
9-26: GitHub repository URL updated correctly.The GitHub repository URL has been updated from "Mail-0" to "Zero", reflecting the correct project repository. This change is consistent with the project branding shown in the footer copyright notice.
30-86: Good implementation of ThemeToggle with responsive layout.The footer has been nicely restructured with a responsive grid layout and the addition of the ThemeToggle component enhances user experience by providing easy theme switching. The use of consistent styling with dark mode support across all elements is excellent.
apps/mail/app/api/driver/types.ts (1)
33-38: Reformatted IConfig interface looks good.The IConfig interface has been reformatted for better readability while maintaining its functionality.
apps/mail/components/create/selectors/text-buttons.tsx (3)
1-6: Imports updated for new AI chat functionality.The imports have been appropriately updated to support the new AI chat functionality, including the addition of the Image component from Next.js.
24-32: AI chat button configuration looks good.The items array is well-structured for the AI chat functionality, with appropriate properties for the name, label, action, and image display.
34-61: Well-structured UI component with proper image handling.The component's rendering logic effectively displays the AI chat button with an image, maintaining accessibility with proper alt text and dimensions.
apps/mail/components/draft/drafts.tsx (5)
3-21: Good organization of imports.The imports are well-organized and comprehensive, covering all the necessary UI components, hooks, and icons needed for the drafts functionality.
30-34: Proper session handling with redirect.The component correctly checks for user authentication and redirects to the login page if the user is not authenticated, ensuring protected access to drafts.
38-48: Effective keyboard shortcut implementation.The implementation of keyboard shortcuts for search functionality enhances user experience. The use of
useHotKeyhook for '/' to start search and 'Esc' to exit search mode follows intuitive patterns.
124-142: Good use of skeleton loaders.The implementation of skeleton loaders during the loading state provides excellent user feedback and prevents layout shifts when data is loaded. The skeletons accurately reflect the expected content structure.
153-176: Well-implemented bulk actions with internationalization.The
BulkSelectActionscomponent is well-structured with proper tooltip implementation and internationalization support. This provides a good user experience for managing multiple drafts at once.apps/mail/app/(routes)/settings/security/page.tsx (4)
3-18: Good implementation of internationalization support!You've successfully added the
useTranslationshook from next-intl and properly organized your imports.
27-28: Well-structured state management and translation setup.The state management for the saving indicator and translation setup look good.
50-58: Successfully migrated text content to use translation keys.You've properly replaced all hardcoded strings with translation keys, making the component fully localizable.
70-96: Form field labels and descriptions now properly internationalized.All form labels and descriptions use translation keys consistently.
apps/mail/app/(routes)/settings/shortcuts/page.tsx (3)
3-7: Good addition of internationalization support.The imports of
useTranslationsandMessageKeytype are appropriate for the changes made.
11-12: Nice enhancement with translations.The use of the
useTranslationshook for localization is well implemented.
17-22: Successfully migrated card title, description, and button texts to use translation keys.All UI text is now properly internationalized.
apps/mail/actions/drafts.ts (2)
1-3: Good use of the 'use server' directive and imports.The "use server" directive correctly marks these functions as server actions, and importing the active driver is the right approach.
35-49: Good implementation of the getDraft function.The function properly validates the draft ID, handles errors, and includes appropriate logging.
One minor suggestion would be to remove or make the console.log statement more informative for production use:
- console.log("getting email:", id); + console.debug("Retrieving draft with ID:", id);apps/mail/components/cookies/cookie-trigger.tsx (1)
5-10: Well-defined props interface.The
CookieTriggerPropsinterface is clear and provides good flexibility for the component.apps/mail/components/draft/drafts-list.tsx (5)
50-56: Avoid possible re-render inefficiencies.
The inlineonClickhandler might trigger re-rendering of child elements. Consider using a callback reference to optimize performance if needed.
150-154: Prevent potential mode conflicts.
TheresetSelectModefunction sets all modes to false, but ensure no race conditions occur if toggled in quick succession with multiple keys pressed concurrently.
346-349: Check correctness of route push.
Ensure that/create?draftId=${message.id}is a valid route and properly handles existing or missingdraftId.
361-374: Evaluate infinite scroll triggers.
When callingendReached={handleScroll}, confirm thathandleScrollis reliably invoked for all edge cases (e.g., partial container overlaps, extremely large content).
377-401: Overall structure is solid.
The usage ofVirtuosowith integrated hotkeys for selection is well-organized and user-friendly. Great job!apps/mail/components/create/ai-assistant.tsx (6)
60-100: Animation variants are well-structured.
The structured approach to storing variants is clear. Great to see separation of container, content, input, button, and card states.
102-124: Avoid re-render inside loading spinner.
The spinner is stateless and works well. No issues here.
142-207: Prevent accidental re-generation loops.
Ensure user is aware that pressing “Regenerate response” can override or discard unsaved user edits. Confirm with user or store previous versions in case of accidental clicks.
280-339: Robust error handling is commendable.
Catching errors and employing fallback messages is a good user-friendly approach. Well done.
431-474: User experience for input focus.
Expanding the input and focusing is great. Check if user input is preserved if they interact with other page elements.
476-479: Overall usage is well structured.
The newAIAssistantis a polished approach to generating email content with minimal friction for the user. Great addition!apps/mail/components/create/editor-buttons.tsx (4)
48-141: Code block and undo/redo logic is consistent.
Toggle actions look consistent with Tiptap’s recommended approach, and disabling buttons ensures a robust user experience.
149-205: Good user feedback for attachments.
Displaying the number of attached files and their details in a popover is a clean, user-friendly approach.🧰 Tools
🪛 Biome (1.9.4)
[error] 187-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
224-226: Empty EditorProvider default.
Currently theEditorProvideris returned withslotBefore={<MenuBar/>}. Confirm whether the editor usage is extended or if additional testing is needed.
1-227: Overall editor buttons are well-structured.
The toolbar is comprehensive, including headings, lists, code blocks, and attachments. Great job bridging functionality and user-friendliness!🧰 Tools
🪛 Biome (1.9.4)
[error] 187-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/mail/components/cookies/toggle.tsx (2)
23-26: Confirm user experience for non-EU regions.
Returningnullif the user is outside the EU region (and the category is not 'necessary') hides the toggle, but this might confuse or limit users in non-EU regions who still may want control. Ensure this is intended behavior, or consider showing a disabled toggle with explanatory text.
28-35: Validate forced enabling of 'necessary' cookies.
You’re forcingchecked={true}for 'necessary' cookies. Confirm that this accurately represents a scenario in which necessary cookies cannot be disabled, and update your UI or logic accordingly if you ever want to provide visually distinct states for forced vs. user-enabled cookies.apps/mail/app/(auth)/login/early-access/page.tsx (2)
68-101: Verify confetti performance and consistency.
This repeated confetti launch within a set interval can be CPU-intensive on slower devices. Confirm it meets user experience requirements, or consider an immediate short burst if performance becomes an issue.
193-205: Confirm the verification flow for partial or expired OTP.
Currently, if the OTP is less than 6 digits, it sets an error. Consider if you also need to handle expired codes or invalid codes of length 6. If your back end eventually checks codes, you’ll want better error messages than just “Please enter a valid code.”apps/mail/app/layout.tsx (1)
27-33: Validate the addedcookiesprop usage.
You accept acookiesprop but only render it directly in the JSX. Confirm that the caller is always passing a valid react node and that no fallback logic is needed if cookies are undefined.apps/mail/app/api/notes/db.ts (5)
1-5: Imports & type references look good.The imports from drizzle-orm and the custom types are properly organized, offering a clear structure for database operations.
7-13: Ordering logic is consistent and well-structured.
getNotessorts notes by pinned status (descending), order (ascending), and creation date (descending). Ensure this ordering meets business logic expectations for displaying notes.
15-21: Consistent approach for thread notes retrieval.
getThreadNotesfollows the same ordering strategy and includes a thread-based filter. No issues found.
57-85: Update logic is sound and includes ownership checks.
updateNoteproperly fetches the note to ensure it belongs to the user before updating. IncludingupdatedAtis helpful for tracking changes.
87-103: Delete operation with ownership validation is secure.
deleteNoteensures the user owns the note, preventing unauthorized deletions. Consider whether soft-deletion or an audit log is needed long-term, but current logic is clear.README.md (3)
1-5: Logo section updates are visually appealing.Centering the logo and setting a custom width clarifies branding. The inline style for padding and background is acceptable for a README.
44-48: System requirements are clearly specified.Listing minimum versions for Node.js, Bun, and Docker helps prevent environment mismatches.
50-124:Details
✅ Verification successful
Setup instructions are thorough and user-friendly.
Splitting the instructions into “Standard Setup” and “Dev Container Setup” is helpful, and the commands (e.g.,
bun docker:up,bun db:dependencies) are well-documented. Ensure these Bun scripts exist in your repository.
🏁 Script executed:
#!/bin/bash # Verify that the referenced Bun scripts exist in your project configs. rg 'docker:up|db:dependencies' -A 3Length of output: 1044
Bun Script Commands Verified – Setup Instructions Are Clear
The README clearly distinguishes between Standard Setup and Dev Container Setup, and the Bun commands (
bun docker:up,bun db:dependencies) referenced have been verified to exist in the repository’s package.json. No further changes are needed.🧰 Tools
🪛 LanguageTool
[grammar] ~113-~113: This phrase is duplicated. You should probably use “the App” only once.
Context: ...ew minutes the first time) 3. Access the App - The app will be available at [http://localhost:...(PHRASE_REPETITION)
apps/mail/app/(routes)/settings/connections/page.tsx (3)
1-13: Concise import usage and minimal overhead.Usage of
'use client'along with necessary component imports is properly organized. The approach to localizing strings usingnext-intlappears consistent.
33-43: Clear disconnect error handling.
disconnectAccountwith success/error toast messages is straightforward. Logging the error to console for debugging is a good practice.
45-178:Details
❓ Verification inconclusive
User interface considerations.
- Tooltip logic on mobile: Toggling via
window.innerWidth <= 768works but may need future updates for responsiveness.- Default icon for connections: Using
emailProviders[0]!is safe if guaranteed non-empty, but consider a fallback icon ifemailProvidersmight be empty.Please confirm
emailProvidersis always a non-empty array, or introduce a fallback icon to avoid a runtime error.
🏁 Script executed:
#!/bin/bash rg --json 'emailProviders\s*=\s*\[.+?\]' -A 5Length of output: 303
Action Required: Verify Default Icon Logic and Mobile Tooltip Behavior
- In
apps/mail/app/(routes)/settings/connections/page.tsx(lines 45–178), the connection item usesemailProviders[0]!for the default icon. Please manually confirm that theemailProvidersarray is always populated. If there's any chance it might be empty, consider adding a fallback icon to prevent runtime errors.- Also, review the mobile tooltip toggle logic that relies on
window.innerWidth <= 768to ensure it meets current responsiveness requirements.apps/mail/actions/ai.ts (1)
1-16: Interfaces and imports appear consistent.No major issues found; the definitions for
UserContextandAIEmailResponseare clear and well-structured.apps/mail/components/create/create-email.tsx (3)
21-33: State initialization looks good.No immediate issues found. The multiple
useStatecalls are well-labeled and effectively manage the local form state.
168-205: Auto-save logic is generally effective.The approach to auto-save after a 3-second pause is clear. As an improvement, consider a debounced approach to avoid frequent saves when users type quickly.
211-264:Details
❓ Verification inconclusive
Verify the default content check.
You compare
(messageContent === JSON.stringify(defaultValue))to detect an empty message, which might yield false positives if the user typed only a single line or if the doc structure slightly changed. Consider checking the actual text content.
🏁 Script executed:
#!/bin/bash # Searching for usage of JSON.stringify(defaultValue) across the codebase rg "JSON\.stringify\(defaultValue\)"Length of output: 166
Action Required: Revisit Default Content Validation Logic
The current check using
(!messageContent.trim() || messageContent === JSON.stringify(defaultValue))is fragile. Comparing the editor’s content to the JSON string of the default value can lead to false positives if, for instance, the user enters content that—by coincidence or due to slight structure changes—matches the JSON string representation of the default value. I recommend extracting and validating the actual text content rather than relying on JSON string comparison.
- Location:
apps/mail/components/create/create-email.tsx(Lines 211-264)apps/mail/components/connection/add.tsx (1)
28-39: Conditional trigger usage is clear.No issues found. The fallback button with translated text is well implemented, ensuring a consistent user interface.
apps/mail/app/(routes)/settings/general/page.tsx (3)
25-25: Great implementation of the new useSettings hook!Your implementation elegantly replaces local state with the custom hook. Using SWR under the hood (as seen in the relevant code snippet) provides caching and revalidation benefits, making the user settings management more robust.
Also applies to: 46-46, 47-47, 59-59, 65-66
62-75: Well-structured async form submission with proper error handlingThe form submission logic is well-implemented with:
- Clear state management using
isPending- Proper error handling with try/catch
- Successful persistence feedback with toast
- Correctly awaiting both the save operation and the mutate function
This ensures user settings are properly persisted and UI feedback is accurate.
129-133: Good use of dynamic timezone optionsUsing
Object.entries(TIMEZONES)to dynamically generate timezone options improves maintainability compared to hardcoded values. This approach ensures timezone changes only need to be updated in one place (TIMEZONESobject).apps/mail/actions/mail.ts (3)
2-2: Improved error handling with fatal error detectionGood enhancement to the error handling by checking for fatal errors and cleaning up connections when they occur. This prevents connection leaks and helps maintain application stability by properly handling destructive error conditions.
Also applies to: 25-25, 39-39, 51-51, 63-63, 74-74, 109-109
53-53: More consistent error handlingThe change from returning error objects to throwing errors provides a more consistent error handling pattern throughout the codebase. This ensures that errors propagate correctly up the call stack rather than being silently returned as objects.
Also applies to: 65-65, 111-111
90-91: Consistent log formattingThe log statements now consistently use string template literals for array joining, which improves readability and maintainability.
.github/CONTRIBUTING.md (5)
5-21: Improved Table of Contents organizationThe updated Table of Contents provides better organization and navigation with additional sections for Database Management and Internationalization. This helps contributors quickly find relevant information.
29-36: Clear development environment setup instructionsThe updated setup instructions now clearly specify the use of Bun and include all necessary steps to get a working development environment. This is particularly important for new contributors.
106-148: Comprehensive Database Management sectionThe new Database Management section provides valuable information about the database structure, common tasks, connection details, and troubleshooting. This will help contributors understand how to work with the database effectively.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
140-140: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
166-167: Stricter TypeScript guidelinesThe added TypeScript guidelines about avoiding
anytype and ensuring all code passes type checking align with best practices for maintainable TypeScript codebases.🧰 Tools
🪛 LanguageTool
[uncategorized] ~167-~167: Possible missing preposition found.
Context: ...ict"no-explicit-any"in the future - Ensure all code passes type checking, as build...(AI_HYDRA_LEO_MISSING_TO)
177-202: Thorough i18n documentationThe new Internationalization section provides clear guidelines for adding translations, following formatting standards, and ensuring quality. This is critical for maintaining a consistent multilingual application.
apps/mail/app/api/notes/index.ts (2)
7-20: Well-implemented user authentication helperThe
getCurrentUserIdfunction is well-structured with proper error handling and clear error messages. It uses the auth API correctly to retrieve the user's session and check for authorization.
22-92: Consistent API implementation with proper error handlingThe notes API implementation follows a consistent pattern across all methods:
- Get the current user ID
- Call the appropriate notes manager method
- Handle and log errors appropriately
This approach ensures proper authentication and error handling throughout the API.
apps/mail/app/api/driver/google.ts (2)
38-79: Validate optional fields inParsedDraftandparseDraft.
Overall, this interface and function are well structured. Sinceto,subject, andcontentare optional, ensure that all usage sites handle undefined values gracefully. Otherwise, this approach looks good.
103-147: Strengthen error handling inparsefor missing headers.
The function is well-structured, but some headers can be missing or malformed. Consider adding fallback handling or logging for cases like an absentDateorFromheader, ensuring your downstream logic does not break.apps/mail/actions/notes.ts (3)
1-10: Initial setup andActionResulttype definition look good.
TheActionResult<T>pattern is clear and helps standardize responses from the notes API. No issues spotted.
12-59: Solid error handling strategy infetchNotes,fetchThreadNotes, andcreateNote.
These functions consistently usetry/catchand return structured error responses. This improves reliability and clarity for upstream consumers. You could consider logging more contextual info about the user or thread ID if relevant.
61-111: Ensure concurrency or transaction safety when updating, deleting, or reordering notes.
While the methods here are straightforward, multiple simultaneous calls (e.g., reordering notes and deleting a note simultaneously) might cause conflicts. Consider whether database transactions or locking are needed for data integrity.apps/mail/components/create/editor.tsx (5)
1-26: Enhanced imports for editor functionality.
These icons and editor-related imports make sense for a richer user interface. Keep an eye on bundle size if more icons or libraries are added.
28-45: Additional UI components and popover integration.
The usage ofPopoverand other UI utilities is consistent with your existing design patterns. No immediate issues spotted.
55-57: Improvised extension types for Tiptap + Markdown.
Casting toany[]is a practical workaround, but consider refining type declarations in the future to take advantage of Tiptap’s extension system.
68-77: New optional event-handling props inEditorProps.
AddingonFocus,onBlur,onCommandEnter, etc. makes the editor more flexible. This extension ofEditorPropsis straightforward and beneficial for broader use cases.
107-371:MenuBarcomponent is comprehensive but watch for complexity.
Providing link insertion, inline formatting, and attachments in a single toolbar is convenient. However, the inline state management for links, attachments, and UI dialogs can lead to a large component over time. Consider splitting out advanced functionalities into smaller sub-components if growth continues.apps/mail/app/(full-width)/contributors/page.tsx (2)
224-230: Verify time zone handling when counting commits.
Substring matching on dates will skip any commit authored in a different local date. Consider normalizing date strings more robustly (e.g., parsing them into Date objects) to avoid missing or double-counting commits due to time zone differences.
641-714: Core Team display looks great.
Highlighting core contributors with their personalized roles, Twitter links, and styled animations beautifully showcases the team behind the project.
| nvm use --lts && \ | ||
| nvm alias default 'lts/*' && \ | ||
| npm install -g yarn pnpm | ||
| npm install -g bun |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Package manager transition to Bun
The change from using yarn and pnpm to bun as the primary package manager appears unrelated to the PR's objective of persisting user settings. While the change itself is correctly implemented, it would be clearer if separated into its own PR focused on tooling changes.
🏁 Script executed:
#!/bin/bash
# Check if this package manager change is referenced in any related configuration files
echo "Checking for related package manager configuration changes..."
grep -r "bun" --include="*.json" --include="*.md" --include="*.yml" .Length of output: 2606
Action Required: Isolate the Package Manager Transition
The new Bun installation command in .devcontainer/Dockerfile (line 47) is consistent with the repository-wide adoption of Bun—as seen in configuration files like .github/CONTRIBUTING.md, .github/workflows/ci.yml, README.md, and package.json. While the change is correctly implemented, the package manager transition isn't directly related to the PR's objective of persisting user settings. For clearer commit history and to maintain focus on the primary functionality, please consider separating this tooling change into its own PR.
| <MailPlus className="mr-2.5 h-4 w-4" /> | ||
| Create New Label | ||
| </ContextMenuItem> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing internationalization for label management texts.
The "Create New Label" and "No labels available" texts are still hardcoded and not using the translation function.
<ContextMenuItem className="font-normal">
<MailPlus className="mr-2.5 h-4 w-4" />
- Create New Label
+ {t('common.mail.createNewLabel')}
</ContextMenuItem>
<ContextMenuSeparator />
<ContextMenuItem disabled className="text-muted-foreground italic">
- No labels available
+ {t('common.mail.noLabelsAvailable')}
</ContextMenuItem>Also applies to: 289-290
| <Tag className="mr-2.5 h-4 w-4" /> | ||
| Labels | ||
| </ContextMenuSubTrigger> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing internationalization for Labels text.
The "Labels" text in the context menu sub-trigger is still hardcoded and not using the translation function.
<ContextMenuSubTrigger className="font-normal">
<Tag className="mr-2.5 h-4 w-4" />
- Labels
+ {t('common.mail.labels')}
</ContextMenuSubTrigger>📝 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.
| <Tag className="mr-2.5 h-4 w-4" /> | |
| Labels | |
| </ContextMenuSubTrigger> | |
| <ContextMenuSubTrigger className="font-normal"> | |
| <Tag className="mr-2.5 h-4 w-4" /> | |
| {t('common.mail.labels')} | |
| </ContextMenuSubTrigger> |
| // Define AI action handlers | ||
| const handleChatWithAI = () => { | ||
| // Get selected text | ||
| const selection = editor.state.selection; | ||
| const selectedText = selection.empty | ||
| ? '' | ||
| : editor.state.doc.textBetween(selection.from, selection.to); | ||
|
|
||
| console.log('Chat with AI about:', selectedText); | ||
| // Implement chat with AI functionality | ||
| }; |
There was a problem hiding this comment.
Implement AI chat handler functionality.
The handleChatWithAI function currently only logs the selected text to the console. As this appears to be a placeholder for future implementation, make sure to complete this functionality to enable actual AI chat interaction.
Consider connecting this with your AI assistant component to provide a working chat experience:
const handleChatWithAI = () => {
// Get selected text
const selection = editor.state.selection;
const selectedText = selection.empty
? ''
: editor.state.doc.textBetween(selection.from, selection.to);
// Set the selected text in your AI context or open the AI chat dialog
openAIAssistant(selectedText);
};| export const getDrafts = async ({ | ||
| q, | ||
| max, | ||
| pageToken, | ||
| }: { | ||
| q?: string; | ||
| max?: number; | ||
| pageToken: string | undefined; | ||
| }) => { | ||
| try { | ||
| const driver = await getActiveDriver(); | ||
| return await driver.listDrafts(q, max, pageToken); | ||
| } catch (error) { | ||
| console.error("Error getting threads:", error); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix inconsistent error message and consider adding input validation.
The error message mentions "threads" instead of "drafts", which is inconsistent with the function's purpose.
- console.error("Error getting threads:", error);
+ console.error("Error getting drafts:", error);Also, consider adding some basic validation for the input parameters:
export const getDrafts = async ({
q,
max,
pageToken,
}: {
q?: string;
max?: number;
pageToken: string | undefined;
}) => {
+ // Validate max if provided
+ if (max !== undefined && (isNaN(max) || max <= 0)) {
+ throw new Error("Invalid max value: must be a positive number");
+ }
+
try {
const driver = await getActiveDriver();
return await driver.listDrafts(q, max, pageToken);
} catch (error) {
console.error("Error getting drafts:", error);
throw error;
}
};📝 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.
| export const getDrafts = async ({ | |
| q, | |
| max, | |
| pageToken, | |
| }: { | |
| q?: string; | |
| max?: number; | |
| pageToken: string | undefined; | |
| }) => { | |
| try { | |
| const driver = await getActiveDriver(); | |
| return await driver.listDrafts(q, max, pageToken); | |
| } catch (error) { | |
| console.error("Error getting threads:", error); | |
| throw error; | |
| } | |
| }; | |
| export const getDrafts = async ({ | |
| q, | |
| max, | |
| pageToken, | |
| }: { | |
| q?: string; | |
| max?: number; | |
| pageToken: string | undefined; | |
| }) => { | |
| // Validate max if provided | |
| if (max !== undefined && (isNaN(max) || max <= 0)) { | |
| throw new Error("Invalid max value: must be a positive number"); | |
| } | |
| try { | |
| const driver = await getActiveDriver(); | |
| return await driver.listDrafts(q, max, pageToken); | |
| } catch (error) { | |
| console.error("Error getting drafts:", error); | |
| throw error; | |
| } | |
| }; |
| const extractSubjectFromContent = (content: string): string | null => { | ||
| const patterns = [ | ||
| /subject:\s*([^\n]+)/i, | ||
| /^RE:\s*([^\n]+)/i, | ||
| /^(Dear|Hello|Hi|Greetings).*?\n\n(.{5,60})[.?!]/i, | ||
| /\b(regarding|about|concerning|reference to|in response to)\b[^.!?]*[.!?]/i, | ||
| ]; | ||
|
|
||
| for (const pattern of patterns) { | ||
| const match = content.match(pattern); | ||
| if (match) { | ||
| if (pattern.toString().includes('Dear|Hello|Hi|Greetings')) { | ||
| return match[2]?.trim() || null; | ||
| } else { | ||
| return match[1]?.trim() || null; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const firstSentence = content.match(/^[^.!?]{5,60}[.!?]/); | ||
| return firstSentence ? firstSentence[0].trim() : null; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refine subject extraction logic.
Regex patterns may fail for certain languages or structures. Consider adding fallback or multi-lingual support if your users are international.
| // Define props interface for AttachmentButtons | ||
| interface AttachmentButtonsProps { | ||
| attachments?: File[]; | ||
| onAttachmentAdd?: () => void; | ||
| onAttachmentRemove?: (index: number) => void; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify optional chaining.
When removing attachments, use onAttachmentRemove?.(index) instead of onAttachmentRemove && onAttachmentRemove(index) to simplify and convey optional calls.
Apply this small diff:
- removeAttachment={(index) => onAttachmentRemove && onAttachmentRemove(index)}
+ removeAttachment={(index) => onAttachmentRemove?.(index)}Committable suggestion skipped: line range outside the PR's diff.
| const currentStep = searchParams.get('step') || 'claim'; | ||
|
|
||
| const [showVerification, setShowVerification] = useState( | ||
| currentStep === 'verify' || currentStep === 'success', | ||
| ); | ||
| const [verified, setVerified] = useState(currentStep === 'success'); | ||
| const [verificationCode, setVerificationCode] = useState(''); | ||
| const [verificationError, setVerificationError] = useState(false); | ||
| const [userEmail, setUserEmail] = useState(searchParams.get('email') || ''); | ||
|
|
There was a problem hiding this comment.
Handle potential race conditions for multiple simultaneous signups.
Your code sets local states for verification flow, but there’s no server-side check to prevent a user from spam-clicking or reusing the signup endpoint. If spam or concurrency is a concern, add checks or gating.
| // Handle form errors with toast notifications | ||
| const onError = (errors: any) => { | ||
| // Define error messages mapping | ||
| const errorMessageMap = [ | ||
| // Email errors | ||
| { | ||
| field: 'email', | ||
| pattern: '@ symbol', | ||
| message: 'Zero email has to have only letters (a-z), numbers (0-9), and periods (.).', | ||
| }, | ||
| { | ||
| field: 'email', | ||
| pattern: 'required', | ||
| message: 'Username is required', | ||
| }, | ||
|
|
||
| // Password errors | ||
| { | ||
| field: 'password', | ||
| pattern: 'at least', | ||
| message: 'Password must be at least 6 characters', | ||
| }, | ||
| { | ||
| field: 'password', | ||
| pattern: '', | ||
| message: 'Password is required', | ||
| }, | ||
|
|
||
| // Name errors | ||
| { | ||
| field: 'name', | ||
| pattern: '', | ||
| message: 'Name is required', | ||
| }, | ||
|
|
||
| // Early access email errors | ||
| { | ||
| field: 'earlyAccessEmail', | ||
| pattern: 'Invalid', | ||
| message: 'Invalid early access email address', | ||
| }, | ||
| { | ||
| field: 'earlyAccessEmail', | ||
| pattern: '', | ||
| message: 'Early access email is required', | ||
| }, | ||
| ]; | ||
|
|
||
| // Find the first matching error and show toast | ||
| for (const [field, fieldError] of Object.entries(errors)) { | ||
| const errorMessage = (fieldError as { message?: string })?.message || ''; | ||
|
|
||
| // Find matching error pattern | ||
| const matchedError = errorMessageMap.find( | ||
| (mapping) => | ||
| mapping.field === field && | ||
| (mapping.pattern === '' || errorMessage.includes(mapping.pattern)), | ||
| ); | ||
|
|
||
| if (matchedError) { | ||
| toast.error(matchedError.message); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Fallback for any other errors | ||
| toast.error('Please fix the form errors'); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor mapping approach for form error messages.
You maintain a large array of field-to-pattern mappings. This can become brittle for future changes. Consider centralizing validation logic with a single reference, or handle each error field’s logic in a more direct manner (e.g., referencing zod parsing codes).
| folder: string, | ||
| q: string, | ||
| maxResults = 20, | ||
| _labelIds: string[] = [], | ||
| pageToken?: string, | ||
| ) => { | ||
| const { folder: normalizedFolder, q: normalizedQ } = normalizeSearch(folder, q ?? ""); | ||
| const labelIds = [..._labelIds]; | ||
| if (normalizedFolder) labelIds.push(normalizedFolder.toUpperCase()); | ||
| const res = await gmail.users.threads.list({ | ||
| userId: "me", | ||
| q: normalizedQ ? normalizedQ : undefined, | ||
| labelIds, | ||
| maxResults, | ||
| pageToken: pageToken ? pageToken : undefined, | ||
| }); | ||
| const threads = await Promise.all( | ||
| (res.data.threads || []) | ||
| .map(async (thread) => { | ||
| if (!thread.id) return null; | ||
| const msg = await gmail.users.threads.get({ | ||
| userId: "me", | ||
| id: thread.id, | ||
| format: "metadata", | ||
| metadataHeaders: ["From", "Subject", "Date"], | ||
| }); | ||
| const threads = await Promise.all( | ||
| (res.data.threads || []) | ||
| .map(async (thread) => { | ||
| if (!thread.id) return null; | ||
| const msg = await gmail.users.threads.get({ | ||
| userId: "me", | ||
| id: thread.id, | ||
| format: "metadata", | ||
| metadataHeaders: ["From", "Subject", "Date"], | ||
| }); | ||
| const message = msg.data.messages?.[0]; | ||
| const parsed = parse(message as any); | ||
| return { | ||
| ...parsed, | ||
| body: "", | ||
| processedHtml: "", | ||
| blobUrl: "", | ||
| totalReplies: msg.data.messages?.length || 0, | ||
| threadId: thread.id, | ||
| }; | ||
| }) | ||
| .filter((msg): msg is NonNullable<typeof msg> => msg !== null), | ||
| ); | ||
|
|
||
| return { ...res.data, threads } as any; | ||
| }, | ||
| get: async (id: string) => { | ||
| const res = await gmail.users.threads.get({ userId: "me", id, format: "full" }); | ||
| const messages = res.data.messages?.map((message) => { | ||
| const bodyData = | ||
| message.payload?.body?.data || | ||
| (message.payload?.parts ? findHtmlBody(message.payload.parts) : "") || | ||
| message.payload?.parts?.[0]?.body?.data || | ||
| ""; // Fallback to first part | ||
|
|
||
| if (!bodyData) { | ||
| console.log("⚠️ Driver: No email body data found"); | ||
| } else { | ||
| console.log("✓ Driver: Found email body data"); | ||
| const message = msg.data.messages?.[0]; | ||
| const parsed = parse(message as any); | ||
| return { | ||
| ...parsed, | ||
| body: "", | ||
| processedHtml: "", | ||
| blobUrl: "", | ||
| totalReplies: msg.data.messages?.length || 0, | ||
| threadId: thread.id, | ||
| }; | ||
| }) | ||
| .filter((msg): msg is NonNullable<typeof msg> => msg !== null), | ||
| ); | ||
|
|
||
| return { ...res.data, threads } as any; | ||
| }, | ||
| get: async (id: string): Promise<ParsedMessage[]> => { | ||
| const res = await gmail.users.threads.get({ userId: "me", id, format: "full" }); | ||
| if (!res.data.messages) return []; | ||
|
|
||
| const messages = await Promise.all( | ||
| res.data.messages.map(async (message) => { | ||
| const bodyData = | ||
| message.payload?.body?.data || | ||
| (message.payload?.parts ? findHtmlBody(message.payload.parts) : "") || | ||
| message.payload?.parts?.[0]?.body?.data || | ||
| ""; | ||
|
|
||
| if (!bodyData) { | ||
| console.log("⚠️ Driver: No email body data found"); | ||
| } else { | ||
| console.log("✓ Driver: Found email body data"); | ||
| } | ||
|
|
||
| console.log("🔄 Driver: Processing email body..."); | ||
| const decodedBody = fromBinary(bodyData); | ||
|
|
||
| console.log("✅ Driver: Email processing complete", { | ||
| hasBody: !!bodyData, | ||
| decodedBodyLength: decodedBody.length, | ||
| }); | ||
|
|
||
| const parsedData = parse(message); | ||
|
|
||
| const attachments = await Promise.all( | ||
| message.payload?.parts | ||
| ?.filter((part) => part.filename && part.filename.length > 0) | ||
| ?.map(async (part) => { | ||
| console.log("Processing attachment:", part.filename); | ||
| const attachmentId = part.body?.attachmentId; | ||
| if (!attachmentId) { | ||
| console.log("No attachment ID found for", part.filename); | ||
| return null; | ||
| } | ||
|
|
||
| // Process the body content | ||
| console.log("🔄 Driver: Processing email body..."); | ||
| const decodedBody = fromBinary(bodyData); | ||
|
|
||
| console.log("✅ Driver: Email processing complete", { | ||
| hasBody: !!bodyData, | ||
| decodedBodyLength: decodedBody.length, | ||
| }); | ||
|
|
||
| // Create the full email data | ||
| const parsedData = parse(message); | ||
| const fullEmailData = { | ||
| ...parsedData, | ||
| body: "", | ||
| processedHtml: "", | ||
| // blobUrl: `data:text/html;charset=utf-8,${encodeURIComponent(decodedBody)}`, | ||
| blobUrl: "", | ||
| decodedBody, | ||
| }; | ||
|
|
||
| // Log the result for debugging | ||
| console.log("📧 Driver: Returning email data", { | ||
| id: fullEmailData.id, | ||
| hasBody: !!fullEmailData.body, | ||
| hasBlobUrl: !!fullEmailData.blobUrl, | ||
| blobUrlLength: fullEmailData.blobUrl.length, | ||
| }); | ||
|
|
||
| return fullEmailData; | ||
| }); | ||
| return messages; | ||
| }, | ||
| create: async (data: any) => { | ||
| const res = await gmail.users.messages.send({ userId: "me", requestBody: data }); | ||
| return res.data; | ||
| }, | ||
| delete: async (id: string) => { | ||
| const res = await gmail.users.messages.delete({ userId: "me", id }); | ||
| return res.data; | ||
| }, | ||
| normalizeIds: (ids) => { | ||
| const normalizedIds: string[] = []; | ||
| const threadIds: string[] = []; | ||
|
|
||
| for (const id of ids) { | ||
| if (id.startsWith('thread:')) { | ||
| threadIds.push(id.substring(7)); | ||
| } else { | ||
| normalizedIds.push(id); | ||
| try { | ||
| if (!message.id) { | ||
| console.error("No message ID found for attachment"); | ||
| return null; | ||
| } | ||
| const attachmentData = await manager.getAttachment(message.id, attachmentId); | ||
| console.log("Fetched attachment data:", { | ||
| filename: part.filename, | ||
| mimeType: part.mimeType, | ||
| size: part.body?.size, | ||
| dataLength: attachmentData?.length || 0, | ||
| hasData: !!attachmentData, | ||
| }); | ||
| return { | ||
| filename: part.filename || "", | ||
| mimeType: part.mimeType || "", | ||
| size: Number(part.body?.size || 0), | ||
| attachmentId: attachmentId, | ||
| headers: part.headers || [], | ||
| body: attachmentData, | ||
| }; | ||
| } catch (error) { | ||
| console.error("Failed to fetch attachment:", part.filename, error); | ||
| return null; | ||
| } | ||
| } | ||
| }) || [], | ||
| ).then((attachments) => | ||
| attachments.filter((a): a is NonNullable<typeof a> => a !== null), | ||
| ); | ||
|
|
||
| return { normalizedIds, threadIds }; | ||
| console.log("ATTACHMENTS:", attachments); | ||
|
|
||
| const fullEmailData = { | ||
| ...parsedData, | ||
| body: "", | ||
| processedHtml: "", | ||
| // blobUrl: `data:text/html;charset=utf-8,${encodeURIComponent(decodedBody)}`, | ||
| blobUrl: "", | ||
| decodedBody, | ||
| attachments, | ||
| }; | ||
|
|
||
| console.log("📧 Driver: Returning email data", { | ||
| id: fullEmailData.id, | ||
| hasBody: !!fullEmailData.body, | ||
| hasBlobUrl: !!fullEmailData.blobUrl, | ||
| blobUrlLength: fullEmailData.blobUrl.length, | ||
| }); | ||
|
|
||
| return fullEmailData; | ||
| }), | ||
| ); | ||
| return messages; | ||
| }, | ||
| create: async (data: any) => { | ||
| const res = await gmail.users.messages.send({ userId: "me", requestBody: data }); | ||
| return res.data; | ||
| }, | ||
| delete: async (id: string) => { | ||
| const res = await gmail.users.messages.delete({ userId: "me", id }); | ||
| return res.data; | ||
| }, | ||
| normalizeIds: (ids: string[]) => { | ||
| const normalizedIds: string[] = []; | ||
| const threadIds: string[] = []; | ||
|
|
||
| for (const id of ids) { | ||
| if (id.startsWith("thread:")) { | ||
| threadIds.push(id.substring(7)); | ||
| } else { | ||
| normalizedIds.push(id); | ||
| } | ||
| } | ||
|
|
||
| return { normalizedIds, threadIds }; | ||
| }, | ||
| async modifyLabels(id: string[], options: { addLabels: string[]; removeLabels: string[] }) { | ||
| await gmail.users.messages.batchModify({ | ||
| userId: "me", | ||
| requestBody: { | ||
| ids: id, | ||
| addLabelIds: options.addLabels, | ||
| removeLabelIds: options.removeLabels, | ||
| }, | ||
| async modifyLabels(id, options) { | ||
| await gmail.users.messages.batchModify({ | ||
| userId: "me", | ||
| requestBody: { | ||
| ids: id, | ||
| addLabelIds: options.addLabels, | ||
| removeLabelIds: options.removeLabels, | ||
| }, | ||
| }); | ||
| }, | ||
| getDraft: async (draftId: string) => { | ||
| try { | ||
| const res = await gmail.users.drafts.get({ | ||
| userId: "me", | ||
| id: draftId, | ||
| format: "full", | ||
| }); | ||
|
|
||
| if (!res.data) { | ||
| throw new Error("Draft not found"); | ||
| } | ||
|
|
||
| const parsedDraft = parseDraft(res.data); | ||
| if (!parsedDraft) { | ||
| throw new Error("Failed to parse draft"); | ||
| } | ||
|
|
||
| return parsedDraft; | ||
| } catch (error) { | ||
| console.error("Error loading draft:", error); | ||
| throw error; | ||
| } | ||
| }, | ||
| listDrafts: async (q?: string, maxResults = 20, pageToken?: string) => { | ||
| const { q: normalizedQ } = normalizeSearch("", q ?? ""); | ||
| const res = await gmail.users.drafts.list({ | ||
| userId: "me", | ||
| q: normalizedQ ? normalizedQ : undefined, | ||
| maxResults, | ||
| pageToken: pageToken ? pageToken : undefined, | ||
| }); | ||
|
|
||
| const drafts = await Promise.all( | ||
| (res.data.drafts || []) | ||
| .map(async (draft) => { | ||
| if (!draft.id) return null; | ||
| const msg = await gmail.users.drafts.get({ | ||
| userId: "me", | ||
| id: draft.id, | ||
| }); | ||
| const message = msg.data.message; | ||
| const parsed = parse(message as any); | ||
| return { | ||
| ...parsed, | ||
| id: draft.id, | ||
| threadId: draft.message?.id, | ||
| }; | ||
| }) | ||
| .filter((msg): msg is NonNullable<typeof msg> => msg !== null), | ||
| ); | ||
|
|
||
| return { ...res.data, drafts } as any; | ||
| }, | ||
| createDraft: async (data: any) => { | ||
| const mimeMessage = [ | ||
| `From: me`, | ||
| `To: ${data.to}`, | ||
| `Subject: ${data.subject}`, | ||
| "Content-Type: text/html; charset=utf-8", | ||
| "", | ||
| data.message, | ||
| ].join("\n"); | ||
|
|
||
| const encodedMessage = Buffer.from(mimeMessage) | ||
| .toString("base64") | ||
| .replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| .replace(/=+$/, ""); | ||
|
|
||
| const requestBody = { | ||
| message: { | ||
| raw: encodedMessage, | ||
| }, | ||
| }; | ||
| }; No newline at end of file | ||
| }; | ||
|
|
||
| let res; | ||
|
|
||
| if (data.id) { | ||
| res = await gmail.users.drafts.update({ | ||
| userId: "me", | ||
| id: data.id, | ||
| requestBody, | ||
| }); | ||
| } else { | ||
| res = await gmail.users.drafts.create({ | ||
| userId: "me", | ||
| requestBody, | ||
| }); | ||
| } | ||
|
|
||
| return res.data; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider splitting large manager methods for maintainability.
list, get, create, delete, modifyLabels, and draft-related methods are all in one large manager object. Splitting them into specialized modules or classes can boost readability and maintainability, especially as this code grows further.
codythatsme
force-pushed
the
feature/persist-user-settings
branch
from
93d17ab to
bac7728
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/mail/app/(routes)/settings/general/page.tsx (1)
49-61: UsedefaultValuesorform.reset()to prevent re-initialization.When
settingschanges dynamically, providing the form values viavaluesmay cause the form to re-initialize every time. Consider usingdefaultValuesalongsideform.reset()if you need to selectively update form fields without re-initializing them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/mail/actions/settings.ts(1 hunks)apps/mail/app/(routes)/settings/general/page.tsx(5 hunks)apps/mail/hooks/use-settings.ts(1 hunks)apps/mail/lib/auth.ts(2 hunks)apps/mail/utils/timezones.ts(1 hunks)packages/db/src/schema.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/mail/hooks/use-settings.ts
- apps/mail/utils/timezones.ts
- packages/db/src/schema.ts
- apps/mail/actions/settings.ts
🧰 Additional context used
🧬 Code Definitions (1)
apps/mail/app/(routes)/settings/general/page.tsx (4)
apps/mail/hooks/use-settings.ts (1) (1)
useSettings(6-39)apps/mail/i18n/config.ts (1) (1)
Locale(18-18)apps/mail/utils/timezones.ts (2) (2)
getBrowserTimezone(3-3)TIMEZONES(6-89)apps/mail/actions/settings.ts (2) (2)
saveUserSettings(55-110)UserSettings(17-17)
🔇 Additional comments (6)
apps/mail/lib/auth.ts (1)
2-3: Imports and references look good.No issues found with these new imports; they correctly match usage in the file.
Also applies to: 6-6
apps/mail/app/(routes)/settings/general/page.tsx (5)
18-21: Appropriate imports for settings handling.These imports correctly accommodate the new persistence workflow.
Also applies to: 24-24
44-45: Neat usage of state and custom hook.Introducing
isPendingand callinguseSettings()for data retrieval is straightforward and consistent.
64-76: Asynchronous form submission is well-handled.The
try/catch/finallyblock ensures error handling and user feedback. ThesetIsPending(false)infinallyis a good practice.
85-86: Button disabling based on submission state is clear.Disabling the button and showing a saving indicator neatly prevents duplicate submissions.
99-99: Logic for language/timezone selection is consistent.Using
field.valueand mapping overTIMEZONESensures correct population of form fields.Also applies to: 123-123, 131-135
| hooks: { | ||
| after: createAuthMiddleware(async (ctx) => { | ||
| // all hooks that run on sign-up routes | ||
| if (ctx.path.startsWith("/sign-up")) { | ||
| // only true if this request is from a new user | ||
| const newSession = ctx.context.newSession; | ||
| if (newSession) { | ||
| // get timezone from vercel's header | ||
| const headerTimezone = ctx.headers?.get("x-vercel-ip-timezone"); | ||
| // get the timezone from the TIMEZONES object, fallback to UTC | ||
| const timezone = TIMEZONES[headerTimezone as keyof typeof TIMEZONES] ?? "UTC"; | ||
| // write default settings against the user | ||
| await db.insert(userSettings).values({ | ||
| id: crypto.randomUUID(), | ||
| userId: newSession.user.id, | ||
| timezone, | ||
| language: "en", | ||
| dynamicContent: true, | ||
| externalImages: true, | ||
| createdAt: new Date(), | ||
| updatedAt: new Date(), | ||
| }); | ||
| } | ||
| } | ||
| }), | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential conflict with pre-existing user settings.
While adding default user settings for brand-new user sessions is appropriate, consider whether the logic may be invoked multiple times and risk inserting duplicate settings. For example, if a user partially signed up before or re-triggers sign-up, a unique constraint violation on userId could occur if the table enforces uniqueness.
Below is a script to verify whether the userSettings table enforces a unique constraint on userId:
🏁 Script executed:
#!/bin/bash
# Description: Check for unique constraint on userSettings.userId
# Searching for table definition or constraints for userSettings
rg -A 10 "create table userSettings"
rg -A 10 "unique" Length of output: 68732
Action: Prevent duplicate user settings for a single user
The current middleware always inserts default settings on sign-up without checking for an existing record. As verified, our database schema does not enforce a unique constraint on userSettings.userId—meaning duplicate settings can be inserted if the sign-up logic is triggered more than once (e.g., a user re-triggers sign-up or a partial sign-up occurs). Please consider one of the following improvements:
- Check for an existing settings record before inserting defaults.
- Use an upsert operation (or similar mechanism) so that if settings already exist for the user, they are updated instead of creating a new record.
|
@MrgSub apologies I did a messy merge from upstream, fixed it with a rebase and should be good to go. |
|
hey @CodyCodes95! im going to help you with this one 🤝 |
ahmetskilinc
changed the base branch from
staging
to
feature/persist-user-settings
ahmetskilinc
merged commit ea2b862
into
Mail-0:feature/persist-user-settings
4 participants
Description
Made general settings located at /settings/general persist. Specifically:
language,
timezone
dynamicContent
externalImages
Options. New db changes are simple and easy to extend for other settings in the future.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Am I stupid I saw no tests here (good)
Security Considerations
For changes involving data or authentication:
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Refactor