[ZERO-170] Toggle for 'Auto-read' emails#1473
Conversation
WalkthroughA new "autoRead" boolean setting was introduced to the mail application's user settings. This setting appears in the general settings UI as a toggle, is included in localization files, and is persisted via schema and database migration updates. The mail list behavior now respects this setting when marking emails as read. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPage
participant Server
participant MailList
User->>SettingsPage: Toggles "Auto Read" setting
SettingsPage->>Server: Updates user settings (autoRead)
Server-->>SettingsPage: Confirms update
User->>MailList: Clicks on an email
MailList->>Server: Fetches user settings (autoRead)
alt autoRead is enabled
MailList->>Server: Marks email as read
else autoRead is disabled
MailList-->>User: Does not mark as read
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (3)
apps/mail/components/mail/mail-list.tsx (3)
797-799: Remove debug console.log statements before production.These debug statements should be removed as they're not suitable for production code and could clutter the browser console.
- console.log("cooking"); const autoRead = settingsData?.settings?.autoRead; - console.log("COOKED!")
798-798: Add defensive null checking for autoRead setting.While the current implementation works, consider adding more explicit null checking for better type safety.
- const autoRead = settingsData?.settings?.autoRead; + const autoRead = settingsData?.settings?.autoRead ?? true; // Default to true if undefinedAlternatively, you could use a more explicit check:
- const autoRead = settingsData?.settings?.autoRead; + const autoRead = Boolean(settingsData?.settings?.autoRead);
816-825: Consider adding settingsData to the dependency array.The handleMailClick callback uses settingsData but doesn't include it in the dependency array. While this might work due to React's optimization, it's better to be explicit about dependencies.
[ getSelectMode, handleSelectMail, handleMouseEnter, setFocusedIndex, optimisticMarkAsRead, setThreadId, setDraftId, setActiveReplyId, + settingsData, ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/mail/app/(routes)/settings/general/page.tsx(1 hunks)apps/mail/components/mail/mail-list.tsx(3 hunks)apps/mail/locales/en.json(1 hunks)apps/server/src/db/migrations/0029_thin_triathlon.sql(1 hunks)apps/server/src/db/migrations/meta/0029_snapshot.json(1 hunks)apps/server/src/db/migrations/meta/_journal.json(1 hunks)apps/server/src/lib/schemas.ts(2 hunks)
🔇 Additional comments (9)
apps/server/src/db/migrations/meta/_journal.json (1)
208-214: Migration journal entry looks correct.The new migration entry follows the established pattern and properly tracks the database schema change for the autoRead feature.
apps/server/src/lib/schemas.ts (2)
47-47: LGTM! Consistent default value.The
autoRead: trueaddition todefaultUserSettingsis properly implemented and aligns with the schema definition.
60-60: LGTM! Proper Zod validation with consistent default.The schema validation for
autoReadis correctly implemented with appropriate boolean type and default value that matchesdefaultUserSettings.apps/mail/app/(routes)/settings/general/page.tsx (1)
263-279: LGTM! Consistent UI implementation.The autoRead toggle follows the established pattern used for other boolean settings in the form. The Switch component, form binding, and localization usage are all properly implemented.
apps/server/src/db/migrations/0029_thin_triathlon.sql (1)
1-1: LGTM! Migration properly adds autoRead with consistent defaults.The SQL migration correctly adds the
autoRead: trueproperty to the default JSONB settings while preserving all existing settings. The structure matches the schema definition.apps/mail/locales/en.json (1)
390-391: LGTM! Clear and informative localization.The localization strings for the autoRead feature are well-written with a concise label and descriptive explanation that clearly communicates the feature's functionality to users.
apps/mail/components/mail/mail-list.tsx (2)
650-650: LGTM - Settings data retrieval looks correct.The useSettings hook is properly imported and used to retrieve settings data.
811-811: Verify the autoRead logic implementation.The conditional logic looks correct - it only marks emails as read when both the message is unread AND autoRead is enabled. This properly respects the user's preference.
apps/server/src/db/migrations/meta/0029_snapshot.json (1)
713-713: Database schema correctly includes autoRead setting.The default JSONB value properly includes the new
autoReadsetting with a sensible default value oftrue. This ensures that existing users will have the autoRead feature enabled by default, which aligns with typical email client behavior.
|
pending approval, should be done from my end |
2 participants
Description
Users can set an "autoRead" toggle to automatically read emails on click or not.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
none
Screenshots/Recordings
none
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit