Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update refactors and enhances the digest email settings feature. It replaces category-based digest preferences with rule-based preferences, introduces new forms for managing digest items and schedules, adds a new API endpoint for digest schedules, and streamlines onboarding and settings flows. Several obsolete components are removed, and the UI is updated for more interactive configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DigestSetting (UI)
participant DigestItemsForm
participant DigestScheduleForm
participant API (digest-schedule)
participant Backend
User->>DigestSetting (UI): Clicks "Edit"
DigestSetting (UI)->>DigestItemsForm: Displays digest items form
DigestSetting (UI)->>DigestScheduleForm: Displays digest schedule form
DigestItemsForm->>Backend: Submit ruleDigestPreferences update
DigestScheduleForm->>API (digest-schedule): Fetch current schedule
DigestScheduleForm->>Backend: Submit updated schedule
Backend-->>DigestItemsForm: Responds with success/error
Backend-->>DigestScheduleForm: Responds with success/error
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/utils/schedule.test.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/utils/schedule.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)`apps/web/**/*.{ts,tsx}`: Use TypeScript with strict null checks Path aliases: U...
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md) List of files the instruction was applied to:
`**/*.{ts,tsx}`: Define validation schemas using Zod Apply the same validation in both client and server Use descriptive error messages
📄 Source: CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc) List of files the instruction was applied to:
🧠 Learnings (1)apps/web/utils/schedule.test.ts (1)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (7)
✨ 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
Documentation and Community
|
|
Claude finished @elie222's task —— View job PR Review: Add digest settingsI'll provide a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
SummaryThis PR adds comprehensive digest settings functionality, allowing users to configure both what gets included in digest emails and when they are sent. The implementation follows the established patterns in the codebase and includes proper form validation, error handling, and data management. Code Quality & Architecture✅ Strengths
|
|
Claude finished @elie222's task —— View job PR Review: Add digest settingsI'll provide a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
SummaryThis PR adds comprehensive digest settings functionality, allowing users to configure both what gets included in digest emails and when they are sent. The implementation follows the established patterns in the codebase and includes proper form validation, error handling, and data management. Code Quality & Architecture✅ Strengths
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/web/hooks/useFeatureFlags.ts (1)
42-42: Remove the commented-out debug line.The commented-out
// return true;line should be removed as it appears to be leftover debugging code that adds unnecessary clutter.export function useDigestEnabled() { return useFeatureFlagEnabled("digest-emails"); - // return true; }apps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx (1)
12-17: Remove unused imports and variables.The
useDigestEnabledhook import anddigestEnabledvariable are no longer used since the conditional navigation logic was simplified. These should be removed to clean up the code.- import { useDigestEnabled } from "@/hooks/useFeatureFlags"; export default function DraftRepliesPage() { const router = useRouter(); const { emailAccountId } = useAccount(); - const digestEnabled = useDigestEnabled();apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (1)
140-140: Complex bit manipulation for day of weekThe day of week conversion
1 << (6 - Number.parseInt(dayOfWeek, 10))is complex and could benefit from a comment explaining the bit manipulation logic.Add a comment to explain the bit manipulation:
+// Convert day of week to bitmask (Sunday=0 -> bit 6, Monday=1 -> bit 5, etc.) daysOfWeek: 1 << (6 - Number.parseInt(dayOfWeek, 10)),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/digest-frequency/page.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/settings/DigestMailFrequencySection.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/settings/SchedulePicker.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/settings/page.tsx(2 hunks)apps/web/app/api/user/digest-schedule/route.ts(1 hunks)apps/web/hooks/useFeatureFlags.ts(1 hunks)apps/web/utils/actions/settings.ts(3 hunks)apps/web/utils/actions/settings.validation.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/digest-frequency/page.tsx
- apps/web/app/(app)/[emailAccountId]/settings/DigestMailFrequencySection.tsx
- apps/web/app/(app)/[emailAccountId]/settings/SchedulePicker.tsx
🧰 Additional context used
📓 Path-based instructions (6)
`apps/web/**/*.{ts,tsx}`: Use TypeScript with strict null checks Path aliases: U...
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/hooks/useFeatureFlags.tsapps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsxapps/web/utils/actions/settings.validation.tsapps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsxapps/web/app/(app)/[emailAccountId]/settings/page.tsxapps/web/app/api/user/digest-schedule/route.tsapps/web/utils/actions/settings.tsapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx
`**/*.{ts,tsx}`: Define validation schemas using Zod Apply the same validation in both client and server Use descriptive error messages
**/*.{ts,tsx}: Define validation schemas using Zod
Apply the same validation in both client and server
Use descriptive error messages
📄 Source: CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
List of files the instruction was applied to:
apps/web/hooks/useFeatureFlags.tsapps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsxapps/web/utils/actions/settings.validation.tsapps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsxapps/web/app/(app)/[emailAccountId]/settings/page.tsxapps/web/app/api/user/digest-schedule/route.tsapps/web/utils/actions/settings.tsapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx
`apps/web/app/**/*`: NextJS app router structure with (app) directory
apps/web/app/**/*: NextJS app router structure with (app) directory
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsxapps/web/app/(app)/[emailAccountId]/settings/page.tsxapps/web/app/api/user/digest-schedule/route.tsapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx
`apps/web/**/*.tsx`: Follow tailwindcss patterns with prettier-plugin-tailwindcs...
apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss
Prefer functional components with hooks
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Follow consistent naming conventions (PascalCase for components)
Use LoadingContent component for async data
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsxapps/web/app/(app)/[emailAccountId]/settings/page.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx
`**/*.tsx`: For API GET requests to server, use the `swr` package Use `result?.serverError` with `toastError` and `toastSuccess` for error handling; success toast is optional
**/*.tsx: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorandtoastSuccessfor error handling; success toast is optional
📄 Source: CodeRabbit Inference Engine (.cursor/rules/data-fetching.mdc)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsxapps/web/app/(app)/[emailAccountId]/settings/page.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx
`**/*.tsx`: Use React Hook Form with Zod for validation Validate form inputs before submission Show validation errors inline next to form fields
**/*.tsx: Use React Hook Form with Zod for validation
Validate form inputs before submission
Show validation errors inline next to form fields
📄 Source: CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsxapps/web/app/(app)/[emailAccountId]/settings/page.tsxapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsxapps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx
🧠 Learnings (10)
apps/web/hooks/useFeatureFlags.ts (3)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/posthog-feature-flags.mdc:0-0
Timestamp: 2025-06-23T12:26:51.119Z
Learning: Centralize all feature flag hooks in a single file (e.g., 'useFeatureFlags.ts') to ensure maintainability and discoverability.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/posthog-feature-flags.mdc:0-0
Timestamp: 2025-06-23T12:26:51.119Z
Learning: For A/B test feature flags, always define a TypeScript type for the possible variants and ensure the hook returns a default fallback value (such as 'control') if the flag is not set.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/posthog-feature-flags.mdc:0-0
Timestamp: 2025-06-23T12:26:51.119Z
Learning: When implementing feature flags using PostHog in a TypeScript React codebase, always use kebab-case for flag keys (e.g., 'inbox-cleaner', 'pricing-options-2') to maintain consistency.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsx (1)
Learnt from: elie222
PR: elie222/inbox-zero#537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
apps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsx (6)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Use React Hook Form with Zod for validation
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Validate form inputs before submission
Learnt from: CR
PR: elie222/inbox-zero#0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-03T12:02:16.137Z
Learning: Applies to apps/web/**/*.tsx : Prefer functional components with hooks
Learnt from: elie222
PR: elie222/inbox-zero#537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-06-23T12:27:33.499Z
Learning: For form text inputs in React using Shadcn UI, use the `Input` component with `registerProps` from a form library (such as react-hook-form) and pass any validation errors to the `error` prop.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-06-23T12:26:16.769Z
Learning: Custom React hooks should encapsulate reusable stateful logic, especially for data fetching or complex UI interactions.
apps/web/utils/actions/settings.validation.ts (1)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.{ts,tsx} : Define validation schemas using Zod
apps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsx (8)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-06-23T12:26:59.468Z
Learning: For components with onClick handlers in Next.js App Router, ensure they are client components by including the 'use client' directive at the top of the file.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-06-23T12:26:16.769Z
Learning: Custom React hooks should encapsulate reusable stateful logic, especially for data fetching or complex UI interactions.
Learnt from: CR
PR: elie222/inbox-zero#0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-03T12:02:16.137Z
Learning: Applies to apps/web/**/*.tsx : Prefer functional components with hooks
Learnt from: aryanprince
PR: elie222/inbox-zero#210
File: apps/web/app/(app)/stats/NewsletterModal.tsx:2-4
Timestamp: 2024-08-23T11:37:26.779Z
Learning: `MoreDropdown` is a React component and `useUnsubscribeButton` is a custom React hook, and they should not be imported using `import type`.
Learnt from: elie222
PR: elie222/inbox-zero#485
File: apps/web/app/(landing)/login/page.tsx:41-43
Timestamp: 2025-06-05T09:49:12.168Z
Learning: In Next.js App Router, components that use the `useSearchParams` hook require a Suspense boundary to handle the asynchronous nature of search parameter access. The Suspense wrapper is necessary and should not be removed when a component uses useSearchParams.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-06-23T12:27:33.499Z
Learning: For form text inputs in React using Shadcn UI, use the `Input` component with `registerProps` from a form library (such as react-hook-form) and pass any validation errors to the `error` prop.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-06-23T12:26:16.769Z
Learning: Each custom React hook should have a single responsibility and be kept as simple as possible.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Use React Hook Form with Zod for validation
apps/web/app/(app)/[emailAccountId]/settings/page.tsx (6)
Learnt from: elie222
PR: elie222/inbox-zero#537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/posthog-feature-flags.mdc:0-0
Timestamp: 2025-06-23T12:26:51.119Z
Learning: Centralize all feature flag hooks in a single file (e.g., 'useFeatureFlags.ts') to ensure maintainability and discoverability.
Learnt from: CR
PR: elie222/inbox-zero#0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-03T12:02:16.137Z
Learning: Applies to apps/web/**/*.tsx : Prefer functional components with hooks
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/posthog-feature-flags.mdc:0-0
Timestamp: 2025-06-23T12:26:51.119Z
Learning: When implementing feature flags using PostHog in a TypeScript React codebase, always use kebab-case for flag keys (e.g., 'inbox-cleaner', 'pricing-options-2') to maintain consistency.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-06-23T12:27:33.499Z
Learning: To handle loading and error states in UI components, wrap content with a `LoadingContent` component that takes `loading` and `error` props, and only renders the main content when data is available.
Learnt from: CR
PR: elie222/inbox-zero#0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-03T12:02:16.137Z
Learning: Applies to apps/web/**/*.tsx : Use LoadingContent component for async data
apps/web/app/api/user/digest-schedule/route.ts (5)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-06-23T12:26:11.276Z
Learning: In Next.js App Router, GET API route handlers should be wrapped with either `withAuth` or `withEmailAccount` middleware to ensure consistent error handling and authentication.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-06-23T12:27:17.978Z
Learning: In Next.js/TypeScript API routes, always use authentication middleware (withAuth for user-level, withEmailAccount for account-level) to protect endpoints that handle user data.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/security-audit.mdc:0-0
Timestamp: 2025-06-23T12:27:05.686Z
Learning: In Next.js API routes under apps/web/app/api/, always use authentication middleware such as withAuth or withEmailAccount, or ensure custom authentication logic is present if using withError.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-06-23T12:26:11.276Z
Learning: Response types for GET API routes in Next.js can be inferred and exported using `Awaited<ReturnType<typeof getData>>`, eliminating the need for manual type definitions.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-06-23T12:26:11.276Z
Learning: Responses from GET API routes in Next.js should be returned using `NextResponse.json()` for consistency and proper response formatting.
apps/web/utils/actions/settings.ts (8)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: Input validation schemas for server actions should be defined using Zod in dedicated .validation.ts files, which can be reused on the client for form validation.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/data-fetching.mdc:0-0
Timestamp: 2025-07-03T12:02:38.024Z
Learning: For mutating data, use Next.js server actions
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: In Next.js projects, server actions should be implemented using the next-safe-action library to ensure type safety, input validation, context management, and centralized error handling.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/prisma.mdc:0-0
Timestamp: 2025-06-23T12:26:53.882Z
Learning: In this project, Prisma should be imported using 'import prisma from "@/utils/prisma";' in TypeScript files.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: Server actions must only be used for mutations (create, update, delete operations) and never for data fetching (GET operations); data fetching should be handled via dedicated GET API routes and SWR hooks.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: Sentry instrumentation for server actions is automatically applied via withServerActionInstrumentation in the safe action clients, and meaningful action names should be provided using the .metadata({ name: "actionName" }) method.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-06-23T12:26:36.694Z
Learning: When implementing LLM-related functions in TypeScript, follow a standard structure: create a scoped logger, define a Zod schema for output validation, perform early input validation and returns, separate system and user prompts, log inputs and outputs, call the LLM with proper configuration, and return the validated result.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/security-audit.mdc:0-0
Timestamp: 2025-06-23T12:27:05.686Z
Learning: All Prisma queries in API routes must be scoped to the current user or account by including emailAccountId or userId in the WHERE clause to prevent unauthorized data access (IDOR vulnerabilities).
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (4)
Learnt from: elie222
PR: elie222/inbox-zero#537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Use React Hook Form with Zod for validation
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Validate form inputs before submission
Learnt from: CR
PR: elie222/inbox-zero#0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-03T12:02:16.137Z
Learning: Applies to apps/web/**/*.tsx : Prefer functional components with hooks
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx (2)
Learnt from: elie222
PR: elie222/inbox-zero#537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-06-23T12:26:11.276Z
Learning: In Next.js App Router, GET API route handlers should be wrapped with either `withAuth` or `withEmailAccount` middleware to ensure consistent error handling and authentication.
🧬 Code Graph Analysis (6)
apps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsx (1)
apps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsx (1)
DigestSetting(23-77)
apps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsx (5)
apps/web/providers/EmailAccountProvider.tsx (1)
useAccount(70-80)apps/web/utils/actions/settings.validation.ts (2)
UpdateDigestItemsBody(57-57)updateDigestItemsBody(54-56)apps/web/utils/actions/settings.ts (1)
updateDigestItemsAction(93-141)apps/web/components/Input.tsx (1)
Label(116-132)apps/web/components/Toggle.tsx (1)
Toggle(19-70)
apps/web/app/(app)/[emailAccountId]/settings/page.tsx (3)
apps/web/providers/EmailAccountProvider.tsx (1)
useAccount(70-80)apps/web/components/Typography.tsx (1)
SectionDescription(114-114)apps/web/app/(app)/[emailAccountId]/settings/ResetAnalyticsSection.tsx (1)
ResetAnalyticsSection(10-43)
apps/web/app/api/user/digest-schedule/route.ts (1)
apps/web/utils/middleware.ts (1)
withEmailAccount(190-194)
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (7)
apps/web/providers/EmailAccountProvider.tsx (1)
useAccount(70-80)apps/web/app/api/user/digest-schedule/route.ts (1)
GetDigestScheduleResponse(5-7)apps/web/utils/actions/settings.ts (1)
updateDigestScheduleAction(53-91)apps/web/utils/schedule.ts (1)
createCanonicalTimeOfDay(12-14)apps/web/components/Input.tsx (1)
Label(116-132)apps/web/components/ui/form.tsx (1)
FormItem(173-173)apps/web/components/ui/select.tsx (3)
SelectTrigger(116-116)SelectContent(117-117)SelectItem(119-119)
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx (1)
apps/web/utils/path.ts (1)
prefixPath(1-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (26)
apps/web/utils/actions/settings.validation.ts (1)
54-57: Well-structured refactoring from categories to rule-based preferences.The change from fixed category fields to a flexible
z.record(z.string(), z.boolean())structure is a good architectural improvement that allows for dynamic rule-based digest preferences. The schema follows Zod best practices and the naming is clear and consistent.apps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsx (2)
13-13: Verify the component integration with existing state management.The import change from
SchedulePickertoDigestUpdateSectionFormlooks correct.
81-81: EnsureDigestUpdateSectionFormis exported and integrated with dialog state
- In apps/web/app/(app)/[emailAccountId]/assistant/onboarding/DigestFrequencyDialog.tsx you’re importing
DigestUpdateSectionFormfrom
@/app/(app)/[emailAccountId]/settings/DigestScheduleForm,
but that file only exportsDigestScheduleForm. Please confirm that:
DigestUpdateSectionFormis defined and exported from the correct module.- The import path is updated to point at its actual location.
- Once the component is available, reconcile the dialog’s local
digestScheduleValuestate andhandleSavelogic:
- Either remove the dialog’s internal state entirely, passing its value and save callback into
DigestUpdateSectionForm,- Or adjust your dialog to consume and forward props from the form component’s API (e.g.
initialValue,onSave).- After fixing exports and wiring up props, verify that saving and validation flow works end-to-end.
apps/web/app/(app)/[emailAccountId]/assistant/SettingsTab.tsx (2)
2-2: Import looks good.The
DigestSettingcomponent import is properly added.
33-33: Component properly positioned in the settings layout.The
DigestSettingcomponent is correctly placed alongside other settings components outside the tabs section. The component handles its own feature flag logic internally, so no conditional rendering is needed here.apps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx (1)
31-35: Navigation logic simplified correctly.The unconditional redirect to the completed page aligns with the removal of the digest frequency onboarding step, simplifying the user flow.
apps/web/app/(app)/[emailAccountId]/settings/DigestItemsForm.tsx (5)
1-17: LGTM: Proper imports and dependenciesThe imports follow the coding guidelines with correct use of TypeScript types, React Hook Form, Zod validation, and proper path aliases using
@/.
18-33: LGTM: Correct hook usage and form setupThe component properly uses the
useAccounthook for context access and sets up React Hook Form with Zod resolver as required by the coding guidelines. The default values structure matches the validation schema.
38-50: LGTM: Proper initialization of form values from API dataThe useEffect correctly initializes form preferences based on the rules data, checking for DIGEST actions on each rule. The dependency array is properly set with
[rules, reset].
52-74: LGTM: Proper form submission with error handlingThe form submission follows the coding guidelines with proper error handling using
result?.serverErrorwithtoastErrorandtoastSuccess. Themutate()call refreshes the data after successful updates.
76-104: LGTM: Proper UI structure and accessibilityThe component uses
LoadingContentfor async data handling and provides a clean UI with proper accessibility attributes. The toggle components have appropriate names and the form structure is well-organized.apps/web/app/(app)/[emailAccountId]/assistant/DigestSetting.tsx (3)
3-21: LGTM: Proper imports and client-side setupThe component correctly uses the "use client" directive and imports all necessary dependencies including React state management, Next.js Image component, and dialog components.
23-28: LGTM: Proper state initialization and feature flag usageThe component correctly initializes state for dialog visibility and uses the feature flag for conditional rendering.
30-76: LGTM: Well-structured modal dialog implementationThe dialog implementation properly uses shadcn/ui components with appropriate accessibility attributes. The nested example dialog functionality is well-implemented with proper state management.
apps/web/app/(app)/[emailAccountId]/settings/page.tsx (2)
14-17: LGTM: Proper use of context hookThe component correctly uses the
useAccounthook from the EmailAccountProvider context as shown in the relevant code snippets.
42-59: LGTM: Simplified conditional renderingThe conditional rendering based on
emailAccountpresence is appropriate. TheResetAnalyticsSectionis properly placed within the conditional block.apps/web/app/api/user/digest-schedule/route.ts (3)
1-7: LGTM: Proper imports and type definitionThe imports follow the coding guidelines and the type alias uses the recommended pattern of
Awaited<ReturnType<typeof function>>for API response typing.
9-14: LGTM: Proper authentication and response handlingThe route correctly uses
withEmailAccountmiddleware for authentication as required by the coding guidelines. The response is properly formatted usingNextResponse.json().
16-35: LGTM: Proper database query and field selectionThe database query uses appropriate field selection and the
findUniqueoperation is correct for fetching a single schedule per email account.apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (4)
1-22: LGTM: Proper imports and dependenciesThe component correctly imports all necessary dependencies and follows the coding guidelines for TypeScript imports and path aliases.
86-103: LGTM: Proper action handling with error managementThe
useActionhook is properly configured with success and error callbacks, following the coding guidelines for error handling with toasts.
149-306: LGTM: Well-structured form UI with proper accessibilityThe form UI is well-structured with proper labels, error messages, and accessibility attributes. The conditional rendering for different schedule types is appropriate.
127-129: Potential bug in 12-hour to 24-hour conversionThe hour conversion logic has a potential issue. Line 127 uses
Number.parseInt(hour, 10) % 12, but this will convert "12" to 0, which is incorrect for 12 PM.Fix the 12-hour to 24-hour conversion logic:
-let hour24 = Number.parseInt(hour, 10) % 12; -if (ampm === "PM") hour24 += 12; -if (ampm === "AM" && hour24 === 12) hour24 = 0; +let hour24 = Number.parseInt(hour, 10); +if (ampm === "PM" && hour24 !== 12) { + hour24 += 12; +} else if (ampm === "AM" && hour24 === 12) { + hour24 = 0; +}Likely an incorrect or invalid review comment.
apps/web/utils/actions/settings.ts (3)
17-33: LGTM!The function correctly updates email settings with proper authentication context and follows the established patterns.
35-51: LGTM!The conditional logic for clearing AI settings when DEFAULT_PROVIDER is selected is well-implemented.
56-91: Verify email account exists before schedule operations.The function doesn't verify that the
emailAccountIdexists before creating or updating schedules. This could lead to orphaned schedule records if an invalid emailAccountId is provided.Consider adding an email account existence check:
export const updateDigestScheduleAction = actionClient .metadata({ name: "updateDigestSchedule" }) .schema(saveDigestScheduleBody) .action(async ({ ctx: { emailAccountId }, parsedInput: { schedule } }) => { try { + // Verify email account exists + const emailAccount = await prisma.emailAccount.findUnique({ + where: { id: emailAccountId }, + select: { id: true }, + }); + + if (!emailAccount) { + throw new SafeError("Email account not found", 404); + } + if (schedule) { // Create or update the ScheduleAdditionally, the error message on line 89 is too generic. Consider making it more specific:
- throw new SafeError("Failed to update settings", 500); + throw new SafeError("Failed to update digest schedule", 500);
| }) { | ||
| const { data, isLoading, error, mutate } = useEmailAccountFull(); | ||
| const digestEnabled = useDigestEnabled(); | ||
| export default function SettingsPage() { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider error handling for missing emailAccount
The component no longer uses LoadingContent for error handling. While useAccount will throw if used outside the provider, consider what happens if emailAccount is null/undefined within the provider context.
🏁 Script executed:
#!/bin/bash
# Description: Check how EmailAccountProvider handles loading and error states
ast-grep --pattern $'export function EmailAccountProvider($$$) {
$$$
}'Length of output: 4027
🏁 Script executed:
#!/bin/bash
# Display the SettingsPage component to inspect its current loading/error handling
rg -n "export default function SettingsPage" -A20 -B5 "apps/web/app/(app)/[emailAccountId]/settings/page.tsx"Length of output: 1104
Ensure loading and missing-account are handled in SettingsPage
The SettingsPage should guard against the provider still loading or failing to resolve an emailAccount. Without this, users could see a broken UI or runtime errors if emailAccount is ever undefined.
• File: apps/web/app/(app)/[emailAccountId]/settings/page.tsx (around line 16)
• Import and use isLoading from useAccount, and render a loading state and an error/fallback when no account is found.
Suggested diff:
--- a/apps/web/app/(app)/[emailAccountId]/settings/page.tsx
+++ b/apps/web/app/(app)/[emailAccountId]/settings/page.tsx
@@ -14,7 +14,10 @@
import { useAccount } from "@/providers/EmailAccountProvider";
-export default function SettingsPage() {
- const { emailAccount } = useAccount();
+export default function SettingsPage() {
+ const { emailAccount, isLoading } = useAccount();
+
+ if (isLoading) {
+ return <LoadingContent loading />;
+ }
+ if (!emailAccount) {
+ return <ErrorMessage message="Email account not found." />;
+ }
return (
<Tabs defaultValue="user">• Don’t forget to add the necessary imports for LoadingContent and ErrorMessage (or your app’s equivalent).
🤖 Prompt for AI Agents
In apps/web/app/(app)/[emailAccountId]/settings/page.tsx around line 16, the
SettingsPage component currently does not handle the loading state or the case
when emailAccount is undefined. To fix this, import isLoading from useAccount
and also import LoadingContent and ErrorMessage components. Inside SettingsPage,
check if isLoading is true and render LoadingContent, and if emailAccount is
undefined render ErrorMessage or a fallback UI. This will prevent broken UI or
runtime errors when the account data is not yet available or missing.
| const hour12 = (Number.parseInt(initHour24, 10) % 12 || 12) | ||
| .toString() |
There was a problem hiding this comment.
Potential bug in 24-hour to 12-hour conversion
In the getInitialScheduleProps function, the conversion from 24-hour to 12-hour format has the same issue as the submission logic.
Fix the 24-hour to 12-hour conversion:
-const hour12 = (Number.parseInt(initHour24, 10) % 12 || 12)
- .toString()
- .padStart(2, "0");
+const hour24 = Number.parseInt(initHour24, 10);
+const hour12 = (hour24 === 0 ? 12 : hour24 > 12 ? hour24 - 12 : hour24)
+ .toString()
+ .padStart(2, "0");Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx around
lines 353 to 354, the conversion from 24-hour to 12-hour format incorrectly
handles the hour 12 and midnight cases. Update the conversion logic to correctly
convert 0 to 12 for midnight and keep 12 as 12 for noon, ensuring the hour is
properly represented in 12-hour format. Adjust the calculation to handle these
edge cases correctly.
| export const updateDigestItemsAction = actionClient | ||
| .metadata({ name: "updateDigestItems" }) | ||
| .schema(updateDigestItemsBody) | ||
| .action( | ||
| async ({ | ||
| ctx: { emailAccountId }, | ||
| parsedInput: { | ||
| toReply, | ||
| newsletter, | ||
| marketing, | ||
| calendar, | ||
| receipt, | ||
| notification, | ||
| coldEmail, | ||
| }, | ||
| parsedInput: { ruleDigestPreferences }, | ||
| }) => { | ||
| const promises: Promise<any>[] = []; | ||
|
|
||
| // Update cold email digest setting | ||
| if (coldEmail !== undefined) { | ||
| promises.push( | ||
| prisma.emailAccount.update({ | ||
| where: { id: emailAccountId }, | ||
| data: { coldEmailDigest: coldEmail }, | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| // Update rule digest settings | ||
| const systemTypeMap = { | ||
| toReply: SystemType.TO_REPLY, | ||
| newsletter: SystemType.NEWSLETTER, | ||
| marketing: SystemType.MARKETING, | ||
| calendar: SystemType.CALENDAR, | ||
| receipt: SystemType.RECEIPT, | ||
| notification: SystemType.NOTIFICATION, | ||
| }; | ||
|
|
||
| for (const [key, systemType] of Object.entries(systemTypeMap)) { | ||
| const value = { | ||
| toReply, | ||
| newsletter, | ||
| marketing, | ||
| calendar, | ||
| receipt, | ||
| notification, | ||
| }[key as keyof typeof systemTypeMap]; | ||
|
|
||
| if (value !== undefined) { | ||
| const promise = async () => { | ||
| const rule = await prisma.rule.findUnique({ | ||
| const promises = Object.entries(ruleDigestPreferences).map( | ||
| async ([ruleId, enabled]) => { | ||
| // Verify the rule belongs to this email account | ||
| const rule = await prisma.rule.findUnique({ | ||
| where: { | ||
| id: ruleId, | ||
| emailAccountId, | ||
| }, | ||
| select: { id: true, actions: true }, | ||
| }); | ||
|
|
||
| if (!rule) return; | ||
|
|
||
| const hasDigestAction = rule.actions.some( | ||
| (action) => action.type === ActionType.DIGEST, | ||
| ); | ||
|
|
||
| if (enabled && !hasDigestAction) { | ||
| // Add DIGEST action | ||
| await prisma.action.create({ | ||
| data: { | ||
| ruleId: rule.id, | ||
| type: ActionType.DIGEST, | ||
| }, | ||
| }); | ||
| } else if (!enabled && hasDigestAction) { | ||
| // Remove DIGEST action | ||
| await prisma.action.deleteMany({ | ||
| where: { | ||
| emailAccountId_systemType: { | ||
| emailAccountId, | ||
| systemType, | ||
| }, | ||
| ruleId: rule.id, | ||
| type: ActionType.DIGEST, | ||
| }, | ||
| select: { id: true, actions: true }, | ||
| }); | ||
|
|
||
| if (!rule) return; | ||
|
|
||
| const hasDigestAction = rule.actions.some( | ||
| (action) => action.type === ActionType.DIGEST, | ||
| ); | ||
|
|
||
| if (value && !hasDigestAction) { | ||
| // Add DIGEST action | ||
| await prisma.action.create({ | ||
| data: { | ||
| ruleId: rule.id, | ||
| type: ActionType.DIGEST, | ||
| }, | ||
| }); | ||
| } else if (!value && hasDigestAction) { | ||
| // Remove DIGEST action | ||
| await prisma.action.deleteMany({ | ||
| where: { | ||
| ruleId: rule.id, | ||
| type: ActionType.DIGEST, | ||
| }, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| promises.push(promise()); | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| ); | ||
|
|
||
| await Promise.all(promises); | ||
| return { success: true }; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for individual rule updates.
The current implementation uses Promise.all which will fail the entire operation if any single rule update fails. This could leave digest preferences in a partially updated state.
Consider using Promise.allSettled to handle individual failures gracefully:
- const promises = Object.entries(ruleDigestPreferences).map(
+ const results = await Promise.allSettled(
+ Object.entries(ruleDigestPreferences).map(
async ([ruleId, enabled]) => {
// Verify the rule belongs to this email account
const rule = await prisma.rule.findUnique({
where: {
id: ruleId,
emailAccountId,
},
select: { id: true, actions: true },
});
- if (!rule) return;
+ if (!rule) {
+ // Log or track skipped rules for debugging
+ console.warn(`Rule ${ruleId} not found or not owned by account ${emailAccountId}`);
+ return;
+ }
const hasDigestAction = rule.actions.some(
(action) => action.type === ActionType.DIGEST,
);
if (enabled && !hasDigestAction) {
// Add DIGEST action
await prisma.action.create({
data: {
ruleId: rule.id,
type: ActionType.DIGEST,
},
});
} else if (!enabled && hasDigestAction) {
// Remove DIGEST action
await prisma.action.deleteMany({
where: {
ruleId: rule.id,
type: ActionType.DIGEST,
},
});
}
},
- );
+ ));
- await Promise.all(promises);
+ // Check for any failures
+ const failures = results.filter(r => r.status === 'rejected');
+ if (failures.length > 0) {
+ console.error('Some digest preferences failed to update:', failures);
+ // Optionally throw or return partial success status
+ }
+
return { success: true };📝 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 updateDigestItemsAction = actionClient | |
| .metadata({ name: "updateDigestItems" }) | |
| .schema(updateDigestItemsBody) | |
| .action( | |
| async ({ | |
| ctx: { emailAccountId }, | |
| parsedInput: { | |
| toReply, | |
| newsletter, | |
| marketing, | |
| calendar, | |
| receipt, | |
| notification, | |
| coldEmail, | |
| }, | |
| parsedInput: { ruleDigestPreferences }, | |
| }) => { | |
| const promises: Promise<any>[] = []; | |
| // Update cold email digest setting | |
| if (coldEmail !== undefined) { | |
| promises.push( | |
| prisma.emailAccount.update({ | |
| where: { id: emailAccountId }, | |
| data: { coldEmailDigest: coldEmail }, | |
| }), | |
| ); | |
| } | |
| // Update rule digest settings | |
| const systemTypeMap = { | |
| toReply: SystemType.TO_REPLY, | |
| newsletter: SystemType.NEWSLETTER, | |
| marketing: SystemType.MARKETING, | |
| calendar: SystemType.CALENDAR, | |
| receipt: SystemType.RECEIPT, | |
| notification: SystemType.NOTIFICATION, | |
| }; | |
| for (const [key, systemType] of Object.entries(systemTypeMap)) { | |
| const value = { | |
| toReply, | |
| newsletter, | |
| marketing, | |
| calendar, | |
| receipt, | |
| notification, | |
| }[key as keyof typeof systemTypeMap]; | |
| if (value !== undefined) { | |
| const promise = async () => { | |
| const rule = await prisma.rule.findUnique({ | |
| const promises = Object.entries(ruleDigestPreferences).map( | |
| async ([ruleId, enabled]) => { | |
| // Verify the rule belongs to this email account | |
| const rule = await prisma.rule.findUnique({ | |
| where: { | |
| id: ruleId, | |
| emailAccountId, | |
| }, | |
| select: { id: true, actions: true }, | |
| }); | |
| if (!rule) return; | |
| const hasDigestAction = rule.actions.some( | |
| (action) => action.type === ActionType.DIGEST, | |
| ); | |
| if (enabled && !hasDigestAction) { | |
| // Add DIGEST action | |
| await prisma.action.create({ | |
| data: { | |
| ruleId: rule.id, | |
| type: ActionType.DIGEST, | |
| }, | |
| }); | |
| } else if (!enabled && hasDigestAction) { | |
| // Remove DIGEST action | |
| await prisma.action.deleteMany({ | |
| where: { | |
| emailAccountId_systemType: { | |
| emailAccountId, | |
| systemType, | |
| }, | |
| ruleId: rule.id, | |
| type: ActionType.DIGEST, | |
| }, | |
| select: { id: true, actions: true }, | |
| }); | |
| if (!rule) return; | |
| const hasDigestAction = rule.actions.some( | |
| (action) => action.type === ActionType.DIGEST, | |
| ); | |
| if (value && !hasDigestAction) { | |
| // Add DIGEST action | |
| await prisma.action.create({ | |
| data: { | |
| ruleId: rule.id, | |
| type: ActionType.DIGEST, | |
| }, | |
| }); | |
| } else if (!value && hasDigestAction) { | |
| // Remove DIGEST action | |
| await prisma.action.deleteMany({ | |
| where: { | |
| ruleId: rule.id, | |
| type: ActionType.DIGEST, | |
| }, | |
| }); | |
| } | |
| }; | |
| promises.push(promise()); | |
| } | |
| } | |
| } | |
| }, | |
| ); | |
| await Promise.all(promises); | |
| return { success: true }; | |
| }, | |
| ); | |
| export const updateDigestItemsAction = actionClient | |
| .metadata({ name: "updateDigestItems" }) | |
| .schema(updateDigestItemsBody) | |
| .action( | |
| async ({ | |
| ctx: { emailAccountId }, | |
| parsedInput: { ruleDigestPreferences }, | |
| }) => { | |
| const results = await Promise.allSettled( | |
| Object.entries(ruleDigestPreferences).map( | |
| async ([ruleId, enabled]) => { | |
| // Verify the rule belongs to this email account | |
| const rule = await prisma.rule.findUnique({ | |
| where: { | |
| id: ruleId, | |
| emailAccountId, | |
| }, | |
| select: { id: true, actions: true }, | |
| }); | |
| if (!rule) { | |
| // Log or track skipped rules for debugging | |
| console.warn( | |
| `Rule ${ruleId} not found or not owned by account ${emailAccountId}` | |
| ); | |
| return; | |
| } | |
| const hasDigestAction = rule.actions.some( | |
| (action) => action.type === ActionType.DIGEST | |
| ); | |
| if (enabled && !hasDigestAction) { | |
| // Add DIGEST action | |
| await prisma.action.create({ | |
| data: { | |
| ruleId: rule.id, | |
| type: ActionType.DIGEST, | |
| }, | |
| }); | |
| } else if (!enabled && hasDigestAction) { | |
| // Remove DIGEST action | |
| await prisma.action.deleteMany({ | |
| where: { | |
| ruleId: rule.id, | |
| type: ActionType.DIGEST, | |
| }, | |
| }); | |
| } | |
| } | |
| ) | |
| ); | |
| // Check for any failures | |
| const failures = results.filter((r) => r.status === "rejected"); | |
| if (failures.length > 0) { | |
| console.error("Some digest preferences failed to update:", failures); | |
| // Optionally throw or return partial success status | |
| } | |
| return { success: true }; | |
| } | |
| ); |
🤖 Prompt for AI Agents
In apps/web/utils/actions/settings.ts between lines 93 and 141, the
updateDigestItemsAction uses Promise.all which causes the entire update to fail
if any single rule update fails, risking partial updates. Replace Promise.all
with Promise.allSettled to allow all rule updates to proceed independently and
handle individual failures gracefully. After Promise.allSettled, check the
results for any rejected promises and handle or log errors accordingly to ensure
robustness.
|
Claude finished @elie222's task —— View job PR Review: Add digest settingsI'll provide a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
SummaryThis PR adds comprehensive digest settings functionality, allowing users to configure both what gets included in digest emails and when they are sent. The implementation follows the established patterns in the codebase and includes proper form validation, error handling, and data management. Code Quality & Architecture✅ Strengths
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/utils/actions/settings.ts (1)
80-128: Address error handling for individual rule updates.The current implementation has the Promise.all issue flagged in previous reviews, which can leave digest preferences in a partially updated state.
🧹 Nitpick comments (2)
apps/web/utils/actions/settings.ts (2)
99-99: Add logging for skipped rules to aid debugging.When a rule is not found, the function silently returns without indication. This makes it difficult to debug why certain digest preferences might not be applied.
- if (!rule) return; + if (!rule) { + console.warn(`Rule ${ruleId} not found or not owned by account ${emailAccountId}`); + return; + }
88-128: Consider wrapping rule updates in a transaction for consistency.The multiple database operations could benefit from transaction wrapping to ensure all-or-nothing updates and prevent partial state inconsistencies.
export const updateDigestItemsAction = actionClient .metadata({ name: "updateDigestItems" }) .schema(updateDigestItemsBody) .action( async ({ ctx: { emailAccountId }, parsedInput: { ruleDigestPreferences }, }) => { + await prisma.$transaction(async (tx) => { const promises = Object.entries(ruleDigestPreferences).map( async ([ruleId, enabled]) => { // Verify the rule belongs to this email account - const rule = await prisma.rule.findUnique({ + const rule = await tx.rule.findUnique({ where: { id: ruleId, emailAccountId, }, select: { id: true, actions: true }, }); if (!rule) { console.warn(`Rule ${ruleId} not found or not owned by account ${emailAccountId}`); return; } const hasDigestAction = rule.actions.some( (action) => action.type === ActionType.DIGEST, ); if (enabled && !hasDigestAction) { // Add DIGEST action - await prisma.action.create({ + await tx.action.create({ data: { ruleId: rule.id, type: ActionType.DIGEST, }, }); } else if (!enabled && hasDigestAction) { // Remove DIGEST action - await prisma.action.deleteMany({ + await tx.action.deleteMany({ where: { ruleId: rule.id, type: ActionType.DIGEST, }, }); } }, ); await Promise.all(promises); + }); return { success: true }; }, );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx(1 hunks)apps/web/utils/actions/settings.ts(2 hunks)apps/web/utils/actions/settings.validation.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx
- apps/web/utils/actions/settings.validation.ts
🧰 Additional context used
📓 Path-based instructions (2)
`apps/web/**/*.{ts,tsx}`: Use TypeScript with strict null checks Path aliases: U...
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/utils/actions/settings.ts
`**/*.{ts,tsx}`: Define validation schemas using Zod Apply the same validation in both client and server Use descriptive error messages
**/*.{ts,tsx}: Define validation schemas using Zod
Apply the same validation in both client and server
Use descriptive error messages
📄 Source: CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
List of files the instruction was applied to:
apps/web/utils/actions/settings.ts
🧠 Learnings (1)
apps/web/utils/actions/settings.ts (4)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/data-fetching.mdc:0-0
Timestamp: 2025-07-03T12:02:38.024Z
Learning: For mutating data, use Next.js server actions
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: Input validation schemas for server actions should be defined using Zod in dedicated .validation.ts files, which can be reused on the client for form validation.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: In Next.js projects, server actions should be implemented using the next-safe-action library to ensure type safety, input validation, context management, and centralized error handling.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: The next-safe-action library provides different clients for context management: actionClientUser for authenticated user context, actionClient for user plus resource-specific context (like emailAccountId), and adminActionClient for admin-restricted actions.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/web/utils/actions/settings.ts (2)
8-8: LGTM - Imports support the new rule-based digest preferences.The new imports correctly support the refactored digest preferences functionality with proper validation and database type safety.
Also applies to: 14-14
55-78: LGTM - Clean refactoring of schedule upsert logic.The explicit separation of create and update objects improves code clarity, and correctly excluding
emailAccountIdfrom updates follows database best practices.
|
Claude finished @elie222's task —— View job PR Review: Add digest settingsI'll provide a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
SummaryThis PR adds comprehensive digest settings functionality, allowing users to configure both what gets included in digest emails and when they are sent. The implementation follows the established patterns in the codebase and includes proper form validation, error handling, and data management. Code Quality & Architecture✅ Strengths
|
|
Claude finished @elie222's task —— View job PR Review: Add digest settingsI'll provide a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
SummaryThis PR adds comprehensive digest settings functionality, allowing users to configure both what gets included in digest emails and when they are sent. The implementation follows the established patterns in the codebase and includes proper form validation, error handling, and data management. Code Quality & Architecture✅ Strengths
|
|
Claude finished @elie222's task —— View job PR Review: Add digest settingsI'll provide a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
SummaryThis PR adds comprehensive digest settings functionality, allowing users to configure both what gets included in digest emails and when they are sent. The implementation follows the established patterns in the codebase and includes proper form validation, error handling, and data management. Code Quality & Architecture✅ Strengths
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/web/utils/actions/settings.ts (2)
93-136: Address potential partial failure issue with Promise.allThe current implementation uses
Promise.allwhich will fail the entire operation if any single rule update fails, potentially leaving digest preferences in an inconsistent state.Consider using
Promise.allSettledto handle individual failures gracefully:- const promises = Object.entries(ruleDigestPreferences).map( + const results = await Promise.allSettled( + Object.entries(ruleDigestPreferences).map( async ([ruleId, enabled]) => { // ... existing logic }, - ); + )); - await Promise.all(promises); + // Check for any failures + const failures = results.filter(r => r.status === 'rejected'); + if (failures.length > 0) { + logger.error('Some digest preferences failed to update:', failures); + // Consider returning partial success status + } + return { success: true };
93-136: Use Promise.allSettled for robust error handling.The current implementation uses
Promise.allwhich will fail the entire operation if any single rule update fails, potentially leaving digest preferences in a partially updated state. This matches the concern raised in the past review comment.-const promises = Object.entries(ruleDigestPreferences).map( +const results = await Promise.allSettled( + Object.entries(ruleDigestPreferences).map( async ([ruleId, enabled]) => { // Verify the rule belongs to this email account const rule = await prisma.rule.findUnique({ where: { id: ruleId, emailAccountId, }, select: { id: true, actions: true }, }); if (!rule) { logger.error("Rule not found", { ruleId }); return; } const hasDigestAction = rule.actions.some( (action) => action.type === ActionType.DIGEST, ); if (enabled && !hasDigestAction) { // Add DIGEST action await prisma.action.create({ data: { ruleId: rule.id, type: ActionType.DIGEST, }, }); } else if (!enabled && hasDigestAction) { // Remove DIGEST action await prisma.action.deleteMany({ where: { ruleId: rule.id, type: ActionType.DIGEST, }, }); } }, - ); + )); -await Promise.all(promises); +// Check for any failures +const failures = results.filter(r => r.status === 'rejected'); +if (failures.length > 0) { + logger.error('Some digest preferences failed to update', { failures }); + // Could throw or return partial success status +}apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (1)
360-362: Fix the 24-hour to 12-hour conversion logic.The conversion from 24-hour to 12-hour format has the same issue flagged in the past review comment. The logic
Number.parseInt(initHour24, 10) % 12 || 12incorrectly handles midnight (0 hours) and noon (12 hours).-const hour12 = (Number.parseInt(initHour24, 10) % 12 || 12) - .toString() - .padStart(2, "0"); +const hour24 = Number.parseInt(initHour24, 10); +const hour12 = (hour24 === 0 ? 12 : hour24 > 12 ? hour24 - 12 : hour24) + .toString() + .padStart(2, "0");
🧹 Nitpick comments (4)
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (3)
140-142: Remove redundant code in time conversionThe condition on line 142 will never execute because after the
% 12operation on line 140,hour24can never equal 12. This line is redundant and should be removed for clarity.let hour24 = Number.parseInt(hour, 10) % 12; if (ampm === "PM") hour24 += 12; -if (ampm === "AM" && hour24 === 12) hour24 = 0;
284-302: Improve error display for time fieldsThe current error handling displays all time-related errors (hour, minute, AM/PM) in a single block. Consider displaying each error next to its respective field for better user experience.
- {(errors.hour || errors.minute || errors.ampm) && ( - <div className="space-y-1"> - {errors.hour && ( - <ErrorMessage - message={errors.hour.message || "Please select an hour"} - /> - )} - {errors.minute && ( - <ErrorMessage - message={errors.minute.message || "Please select minutes"} - /> - )} - {errors.ampm && ( - <ErrorMessage - message={errors.ampm.message || "Please select AM or PM"} - /> - )} - </div> - )} + {/* Move error messages next to each respective field */}
284-302: Improve error message display for time fields.The error handling for time fields could be improved by showing all validation errors at once instead of conditionally displaying them.
-{(errors.hour || errors.minute || errors.ampm) && ( - <div className="space-y-1"> - {errors.hour && ( - <ErrorMessage - message={errors.hour.message || "Please select an hour"} - /> - )} - {errors.minute && ( - <ErrorMessage - message={errors.minute.message || "Please select minutes"} - /> - )} - {errors.ampm && ( - <ErrorMessage - message={errors.ampm.message || "Please select AM or PM"} - /> - )} - </div> -)} +<div className="space-y-1"> + {errors.hour && ( + <ErrorMessage + message={errors.hour.message || "Please select an hour"} + /> + )} + {errors.minute && ( + <ErrorMessage + message={errors.minute.message || "Please select minutes"} + /> + )} + {errors.ampm && ( + <ErrorMessage + message={errors.ampm.message || "Please select AM or PM"} + /> + )} +</div>apps/web/utils/actions/settings.ts (1)
104-107: Improve error handling for missing rules.Instead of just logging and returning, consider whether this should be treated as an error condition that affects the response.
if (!rule) { logger.error("Rule not found", { ruleId }); - return; + throw new Error(`Rule ${ruleId} not found or not owned by account ${emailAccountId}`); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx(1 hunks)apps/web/utils/actions/settings.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
`apps/web/**/*.{ts,tsx}`: Use TypeScript with strict null checks Path aliases: U...
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/utils/actions/settings.tsapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx
`**/*.{ts,tsx}`: Define validation schemas using Zod Apply the same validation in both client and server Use descriptive error messages
**/*.{ts,tsx}: Define validation schemas using Zod
Apply the same validation in both client and server
Use descriptive error messages
📄 Source: CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
List of files the instruction was applied to:
apps/web/utils/actions/settings.tsapps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx
`apps/web/app/**/*`: NextJS app router structure with (app) directory
apps/web/app/**/*: NextJS app router structure with (app) directory
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx
`apps/web/**/*.tsx`: Follow tailwindcss patterns with prettier-plugin-tailwindcs...
apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss
Prefer functional components with hooks
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Follow consistent naming conventions (PascalCase for components)
Use LoadingContent component for async data
📄 Source: CodeRabbit Inference Engine (apps/web/CLAUDE.md)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx
`**/*.tsx`: For API GET requests to server, use the `swr` package Use `result?.serverError` with `toastError` and `toastSuccess` for error handling; success toast is optional
**/*.tsx: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorandtoastSuccessfor error handling; success toast is optional
📄 Source: CodeRabbit Inference Engine (.cursor/rules/data-fetching.mdc)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx
`**/*.tsx`: Use React Hook Form with Zod for validation Validate form inputs before submission Show validation errors inline next to form fields
**/*.tsx: Use React Hook Form with Zod for validation
Validate form inputs before submission
Show validation errors inline next to form fields
📄 Source: CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
List of files the instruction was applied to:
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx
🧠 Learnings (2)
apps/web/utils/actions/settings.ts (4)
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/data-fetching.mdc:0-0
Timestamp: 2025-07-03T12:02:38.024Z
Learning: For mutating data, use Next.js server actions
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: In Next.js projects, server actions should be implemented using the next-safe-action library to ensure type safety, input validation, context management, and centralized error handling.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: Input validation schemas for server actions should be defined using Zod in dedicated .validation.ts files, which can be reused on the client for form validation.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: The next-safe-action library provides different clients for context management: actionClientUser for authenticated user context, actionClient for user plus resource-specific context (like emailAccountId), and adminActionClient for admin-restricted actions.
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (10)
Learnt from: elie222
PR: elie222/inbox-zero#537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Use React Hook Form with Zod for validation
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Validate form inputs before submission
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.{ts,tsx} : Define validation schemas using Zod
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-06-23T12:27:23.938Z
Learning: Input validation schemas for server actions should be defined using Zod in dedicated .validation.ts files, which can be reused on the client for form validation.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-06-23T12:27:33.499Z
Learning: For form text inputs in React using Shadcn UI, use the `Input` component with `registerProps` from a form library (such as react-hook-form) and pass any validation errors to the `error` prop.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-06-23T12:26:36.694Z
Learning: Always use Zod schemas for validating LLM responses, and make schemas as specific as possible to guide the LLM's output.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/form-handling.mdc:0-0
Timestamp: 2025-07-03T12:02:53.001Z
Learning: Applies to **/*.tsx : Show validation errors inline next to form fields
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-06-23T12:27:17.978Z
Learning: All input parameters (route, query, body) must be validated for type, format, and length before use; use libraries like Zod for request body validation.
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-06-23T12:26:36.694Z
Learning: When implementing LLM-related functions in TypeScript, follow a standard structure: create a scoped logger, define a Zod schema for output validation, perform early input validation and returns, separate system and user prompts, log inputs and outputs, call the LLM with proper configuration, and return the validated result.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/web/app/(app)/[emailAccountId]/settings/DigestScheduleForm.tsx (6)
87-95: LGTM: Zod validation properly integratedThe form now correctly uses
zodResolverwith the Zod schema, addressing the previous review comment about missing validation.
360-362: Time conversion logic is correctDespite the past review comment flagging this as a bug, the 24-hour to 12-hour conversion logic is actually correct. The
% 12 || 12pattern properly handles both midnight (0 → 12) and noon (12 → 12) cases.
153-153: Bitmask calculation for daysOfWeek is correctThe front end encoding in DigestScheduleForm.tsx
daysOfWeek: 1 << (6 - Number.parseInt(dayOfWeek, 10))matches exactly the backend’s maskFor logic in utils/schedule.ts
const maskFor = (jsDay: number) => 1 << (6 - jsDay);This yields Sunday→64 … Saturday→1 as expected. No changes required.
22-30: Zod validation implementation looks good.The form now properly uses Zod validation with
zodResolver, which addresses the past review comment about missing validation schema.Also applies to: 93-93
153-153: daysOfWeek bitmask calculation is correctThe formula
1 << (6 - Number.parseInt(dayOfWeek, 10))intentionally maps day indices (0 = Sunday … 6 = Saturday) to bit positions 6…0, matching theDAYSconstant inapps/web/utils/schedule.ts(e.g.SUNDAY: 0b1000000, …,SATURDAY: 0b0000001). The existing tests inutils/schedule.test.tsconfirm this behavior, and the backend simply persists the resulting integer. No changes are needed.
140-142: Fix the 12-hour to 24-hour conversion logic.The conversion from 12-hour to 24-hour format has a bug. The current logic
Number.parseInt(hour, 10) % 12will convert 12 PM to 0, which when added to 12 becomes 12 (correct), but 12 AM becomes 0, which should stay 0. However, there's an issue with the AM check.-let hour24 = Number.parseInt(hour, 10) % 12; -if (ampm === "PM") hour24 += 12; -if (ampm === "AM" && hour24 === 12) hour24 = 0; +let hour24 = Number.parseInt(hour, 10); +if (ampm === "AM") { + hour24 = hour24 === 12 ? 0 : hour24; +} else { + hour24 = hour24 === 12 ? 12 : hour24 + 12; +}⛔ Skipped due to learnings
Learnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/form-handling.mdc:0-0 Timestamp: 2025-07-03T12:02:53.001Z Learning: Applies to **/*.tsx : Use React Hook Form with Zod for validationLearnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/form-handling.mdc:0-0 Timestamp: 2025-07-03T12:02:53.001Z Learning: Applies to **/*.tsx : Validate form inputs before submissionLearnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/form-handling.mdc:0-0 Timestamp: 2025-07-03T12:02:53.001Z Learning: Applies to **/*.{ts,tsx} : Define validation schemas using Zodapps/web/utils/actions/settings.ts (2)
53-79: LGTM: Clean and efficient schedule update actionThe simplified upsert approach is much cleaner than the previous implementation. The explicit separation of create and update objects is well-structured.
56-79: Simplified schedule upsert logic looks good.The refactored
updateDigestScheduleActionproperly handles the upsert operation and correctly excludesemailAccountIdfrom the update object.
|
bugbot run |
|
Claude finished @elie222's task —— View job PR Review: Add digest settingsI'll provide a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage. Review Tasks:
SummaryThis PR adds comprehensive digest settings functionality, allowing users to configure both what gets included in digest emails and when they are sent. The implementation follows the established patterns in the codebase and includes proper form validation, error handling, and data management. Code Quality & Architecture✅ Strengths
|
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Removals
API
Tests
Utilities