Load data from server when revisiting assistant set up #384
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates several onboarding and rule-handling components. The onboarding components now accept customizable default values and integrate user authentication with preference retrieval before rendering. Helper functions are added to process user settings, and the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant OP as OnboardingPage
participant Auth as auth()
participant DB as getUserPreferences()
participant CS as CategoriesSetup
U->>OP: Request OnboardingPage
OP->>Auth: Verify authentication
Auth-->>OP: Return authentication status
OP->>DB: Retrieve user preferences
DB-->>OP: Return defaultValues
OP->>CS: Render component with defaultValues
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/api/ai/summarise/route.tsOops! Something went wrong! :( ESLint: 9.23.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/actions/reply-tracking.tsOops! Something went wrong! :( ESLint: 9.23.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/app/(app)/automation/onboarding/CategoriesSetup.tsxOops! Something went wrong! :( ESLint: 9.23.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
✨ Finishing Touches
🪧 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://cdn.statically.io/img/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/web/utils/actions/rule.ts (1)
607-607: Avoid potentially unnecessary newline concatenation.
Ifuser.rulesPromptis not empty, you might prepend a newline; otherwise, you risk introducing an extra blank line up front. A minor refinement is suggested:- `${user.rulesPrompt || ""}\n${rules.map((r) => `* ${r}`).join("\n")}`.trim() + user.rulesPrompt + ? `${user.rulesPrompt}\n${rules.map((r) => `* ${r}`).join("\n")}`.trim() + : rules.map((r) => `* ${r}`).join("\n").trim()apps/web/app/(app)/automation/onboarding/page.tsx (1)
37-66: User preference fetching logic.
getUserPreferencescleanly compiles rule and cold email settings. One small note: you returnundefinedif no user is found, so ensure all consumers handle that case gracefully.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/(app)/automation/onboarding/CategoriesSetup.tsx(1 hunks)apps/web/app/(app)/automation/onboarding/page.tsx(1 hunks)apps/web/utils/actions/rule.ts(8 hunks)apps/web/utils/actions/rule.validation.ts(1 hunks)apps/web/utils/reply-tracker/enable.ts(2 hunks)apps/web/utils/rule/consts.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/web/app/(app)/automation/onboarding/CategoriesSetup.tsx (1)
apps/web/utils/actions/rule.validation.ts (2)
CreateRulesOnboardingBody(162-164)createRulesOnboardingBody(153-161)
apps/web/utils/actions/rule.ts (1)
apps/web/utils/rule/consts.ts (1)
RuleName(2-9)
🔇 Additional comments (21)
apps/web/utils/rule/consts.ts (1)
1-9: Good implementation of a centralized constants file.The introduction of the
RuleNameconstant is a good practice for maintaining consistent rule names across the application. This enhances maintainability by eliminating hardcoded strings and providing a single source of truth for rule names.apps/web/utils/actions/rule.validation.ts (1)
152-152: Nice addition for improved type safety.Exporting the
CategoryActiontype derived from the Zod enum improves type safety and makes the code more maintainable when working with category actions throughout the codebase.apps/web/utils/reply-tracker/enable.ts (2)
10-10: LGTM: Good import of the new constant.Adding this import is a good step toward centralizing rule names.
86-87: Good replacement of hardcoded string with constant.Replacing the hardcoded "To Reply" string with
RuleName.ToReplyimproves maintainability and consistency across the codebase.apps/web/app/(app)/automation/onboarding/CategoriesSetup.tsx (2)
38-42: Good component signature update to support persistence.The updated component signature now accepts optional default values, which supports the PR objective of loading data from the server when revisiting the assistant setup. This is a good practice for making components more reusable.
48-55: Well-implemented fallback pattern for default values.The implementation correctly uses the optional chaining operator (
?.) with fallback values, ensuring the form will work correctly whether or not default values are provided. This is a clean and standard approach.apps/web/utils/actions/rule.ts (7)
41-41: Good move using a central constant for rule names.
This change helps keep rule name references consistent throughout the codebase.
496-496: Confirm scoping ofdeleteMany: {}.
Specifying an emptydeleteManyobject in the nested Prisma call may delete all related actions under this rule, or potentially more if not scoped correctly. Please ensure that this is intentional and that no additional filtering is required, so you don’t accidentally remove unrelated actions.Would you like me to generate a quick script to confirm whether
deleteMany: {}here is properly scoped to remove only the actions for the current rule?
549-549: Consistent usage ofRuleName.Newsletters.
Switching to a shared constant clarifies your intent and reduces typos.
560-560: Consistent usage ofRuleName.Marketing.
Using enumerated rule names instead of raw strings makes refactoring simpler and avoids mismatches.
571-571: Consistent usage ofRuleName.Calendar.
Good to see these category names come from a single source of truth.
582-582: Consistent usage ofRuleName.Receipts.
Centralizing all rule names underRuleNameprevents confusion and improves maintainability.
593-593: Consistent usage ofRuleName.Notifications.
Again, leveraging theRuleNameobject for uniformity is beneficial.apps/web/app/(app)/automation/onboarding/page.tsx (8)
3-7: Solid import additions.
Importing theauth,prisma, andRuleNameutilities, along with the new types, aligns well with the updated onboarding features.
9-11: Appropriate switch to an async function.
MarkingOnboardingPageasasynccorrectly handles Prisma calls for user authentication and data loading.
13-13: Nice integration of user preferences retrieval.
StoringdefaultValuesfromgetUserPreferencespaves the way for a personalized onboarding experience.
17-17: Customizable defaults passed toCategoriesSetup.
PassingdefaultValuesas props ensures the UI reflects the user's stored preferences.
22-35: Type-safety withUserPreferences.
Defining a clearUserPreferencestype promotes stricter type checking and code clarity.
68-75: Clarity ingetToReplySetting.
Straightforward logic to determine the “toReply” preference.
77-89: Reusable approach ingetRuleSetting.
Effectively checks for a matching rule’s actions and translates them into the correct category action string.
91-105: Logical switch ingetColdEmailSetting.
MappingColdEmailSettingto your label or archive action is neat and future-proof.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/utils/actions/rule.ts (2)
499-500: Inconsistent label in action creation.The label name "Newsletter" is hardcoded, but the rule is using
RuleName.Newsletters(plural). Consider using the RuleName constant here too for consistency.- { type: ActionType.LABEL, label: "Newsletter" }, + { type: ActionType.LABEL, label: RuleName.Newsletters },
458-473: Consider adding error handling for background process.The code initiates a background process for loading previous emails but doesn't handle potential fetch errors or set appropriate timeouts.
fetch( `${env.NEXT_PUBLIC_BASE_URL}/api/reply-tracker/process-previous`, { method: "POST", headers: { "Content-Type": "application/json", [INTERNAL_API_KEY_HEADER]: env.INTERNAL_API_KEY, }, body: JSON.stringify({ userId: session.user.id, } satisfies ProcessPreviousBody), + signal: AbortSignal.timeout(30000), // 30 second timeout }, - ); + ).catch(error => { + logger.warn("Failed to process previous emails in background", { error }); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/components/SideNav.tsx(2 hunks)apps/web/utils/actions/rule.ts(9 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/web/components/SideNav.tsx (1)
apps/web/components/SideNavMenu.tsx (1)
SideNavMenu(21-47)
apps/web/utils/actions/rule.ts (1)
apps/web/utils/rule/consts.ts (1)
RuleName(2-9)
🔇 Additional comments (7)
apps/web/components/SideNav.tsx (2)
52-52: Good addition of ClientOnly component importThis import aligns with the PR objective to handle server/client rendering correctly when loading user data, which helps prevent hydration mismatches when user preferences are involved.
279-284: Good use of ClientOnly to prevent hydration mismatchesWrapping the
SideNavMenufor clean items in aClientOnlycomponent ensures that these navigation items are only rendered on the client side. This is a good approach when the component depends on data that might differ between server and client rendering, such as user preferences or feature flags.This change aligns with the PR objective to properly load data from the server when revisiting the assistant setup, as it prevents potential hydration errors that could occur if the server and client render different versions of the navigation menu.
apps/web/utils/actions/rule.ts (5)
41-41: Good addition of RuleName constant import.Using a centralized constants file for rule names improves type safety and maintainability throughout the codebase.
491-512: Improved rule update logic with action cleanup.The addition of
deleteMany: {}in the actions object ensures all existing actions are removed before creating new ones, preventing stale or duplicate actions. The error handling with proper logging is also a good improvement.
539-539: Good addition of error logging.Adding error logging for rule creation failures enhances debugging capabilities and keeps error handling consistent with the update flow.
555-555: Consistent use of RuleName constants.Replacing string literals with the RuleName constants improves code consistency and reduces the risk of typos or inconsistent naming across the codebase.
Also applies to: 566-566, 577-577, 588-588, 599-599
613-613: Changed newline formatting in rulesPrompt.The change from
\n\nto\nmakes the rule entries more compact in the prompt. This is a minor formatting improvement.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/utils/actions/rule.ts (1)
555-564: Good addition ofdeleteRulefunctionThis helper function properly encapsulates the rule deletion logic, but there's a potential issue with error handling.
Consider adding specific error handling within the async function:
async function deleteRule(name: string) { const promise = async () => { const rule = await prisma.rule.findUnique({ where: { name_userId: { name, userId: session?.user.id! } }, }); if (!rule) return; - await prisma.rule.delete({ where: { id: rule.id } }); + try { + await prisma.rule.delete({ where: { id: rule.id } }); + } catch (error) { + logger.error("Error deleting rule", { error, name }); + // Consider whether to rethrow or handle silently + } }; promises.push(promise()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(app)/automation/onboarding/page.tsx(1 hunks)apps/web/utils/actions/rule.ts(5 hunks)apps/web/utils/rule/consts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/rule/consts.ts
🧰 Additional context used
🧬 Code Definitions (2)
apps/web/utils/actions/rule.ts (1)
apps/web/utils/rule/consts.ts (1)
RuleName(2-9)
apps/web/app/(app)/automation/onboarding/page.tsx (3)
apps/web/app/(app)/automation/onboarding/CategoriesSetup.tsx (1)
CategoriesSetup(38-145)apps/web/utils/rule/consts.ts (1)
RuleName(2-9)apps/web/utils/actions/rule.validation.ts (1)
CategoryAction(152-152)
🔇 Additional comments (16)
apps/web/utils/actions/rule.ts (9)
41-41: Good addition ofRuleNameimportUsing the
RuleNameconstants instead of string literals improves maintainability and prevents typos.
491-514: Enhanced rule updates with improved cleanupThe addition of
deleteMany: {}ensures a clean slate before creating new actions, which prevents potential duplicates or inconsistencies. The error handling is also appropriately improved.
539-543: Improved error logging for rule creationGreat addition of error logging that will help with debugging when rule creation fails.
569-577: Improved rule management with constantsUsing
RuleName.Newsletterinstead of string literals and adding cleanup logic for when newsletters are not selected is a good improvement for maintenance and consistency.
582-590: Consistent use of RuleName constantsThe same pattern is correctly applied to the Marketing rule, making the code more maintainable.
595-603: Consistent use of RuleName constantsThe same pattern is correctly applied to the Calendar rule, making the code more maintainable.
608-616: Consistent use of RuleName constantsThe same pattern is correctly applied to the Receipt rule, making the code more maintainable.
621-629: Consistent use of RuleName constantsThe same pattern is correctly applied to the Notification rule, making the code more maintainable.
637-637: Updated string formattingChanged from
\n\nto\nfor the rules prompt, which makes the formatting more consistent.apps/web/app/(app)/automation/onboarding/page.tsx (7)
3-7: Good imports for new functionalityThe imports are appropriately added to support the new authentication and user preferences functionality.
9-14: Enhanced page with user authentication and preferencesMaking the component async and fetching user preferences is a good improvement for personalization.
17-17: Passing user preferences to componentThe component now correctly passes the fetched user preferences as default values.
22-35: Well-structured type definitionThe
UserPreferencestype is clearly defined with appropriate Prisma selections.
68-75: Well-implemented helper function for ToReply settingThe function correctly determines if there's a rule tracking replies and returns the appropriate setting.
77-89: Comprehensive rule setting helperThe function handles all the necessary cases to determine the appropriate category action based on rule actions.
91-105: Complete cold email setting helperThe function correctly maps the database cold email settings to the appropriate category actions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/utils/actions/rule.ts (2)
539-543: Consider explaining the necessity of empty then() clause.The comment "doesn't update without this line" suggests a workaround for an issue with Prisma updates. It would be helpful to provide more context about why this empty
.then(() => {})is necessary.- .then(() => {}) + // This empty .then() is necessary because of a Prisma issue where + // updates may not be applied properly without resolving the promise + .then(() => {})
605-606: Fixed capitalization inconsistency in rule naming.The comments now use "Receipt" and "Notification" (singular) to match the RuleName constants, while the prompt text still uses "Receipts" and "Notifications" (plural). Consider standardizing the naming convention.
- "Label all receipts as 'Receipts'", + "Label all receipts as 'Receipt'", - "Label all notifications as 'Notifications'", + "Label all notifications as 'Notification'",Also applies to: 618-619
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/utils/actions/rule.ts(6 hunks)apps/web/utils/actions/rule.validation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/actions/rule.validation.ts
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/utils/actions/rule.ts (1)
apps/web/utils/rule/consts.ts (1)
RuleName(2-9)
🔇 Additional comments (6)
apps/web/utils/actions/rule.ts (6)
41-41: Excellent use of constants instead of string literals.Importing
RuleNamefrom a constants file is a good practice that centralizes rule name definitions, improving maintainability and reducing the risk of typos.
491-514: Good improvement to rule update logic.Adding
deleteMany: {}ensures that existing actions are properly cleaned up before creating new ones, preventing orphaned or duplicate actions. The additional error handling with logger.error is a nice touch for better debugging.
555-564: Good addition of helper function for rule deletion.The new
deleteRulefunction nicely encapsulates the logic for finding and deleting a rule by name, making the code more maintainable and easier to read.
567-577: Improved rule management with conditional creation/deletion.Adding the ability to delete rules when their corresponding setting is set to "none" makes the system more consistent and prevents orphaned rules.
569-569: Great use of RuleName constants.Replacing string literals with constants from the RuleName object improves code maintainability and reduces the chance of errors from typos.
Also applies to: 582-582, 595-595, 608-608, 621-621
637-637:❓ Verification inconclusive
Changed newline handling in rulesPrompt concatenation.
Changed from
\n\nto\nwhich affects the formatting of the rules prompt. This reduces excessive whitespace but make sure this doesn't negatively impact how the rules are displayed or processed.
🏁 Script executed:
#!/bin/bash # Check how rulesPrompt is used elsewhere in the codebase to ensure this change doesn't break anything echo "Searching for rulesPrompt usage in the codebase..." rg -A 3 "rulesPrompt" | grep -v "apps/web/utils/actions/rule.ts"Length of output: 12556
Action Required: Verify Formatting Consistency of rulesPrompt
The updated concatenation in
apps/web/utils/actions/rule.tsnow uses a single newline between the existingrulesPromptand the appended rule list instead of a double newline. I reviewed other usages ofrulesPromptthroughout the codebase (for example, inapps/web/utils/rule/prompt-file.tsandapps/web/app/(app)/onboarding/OnboardingEmailAssistant.tsx) where the newline handling varies—some instances use a double newline while others use a single newline.Please ensure that the change in this file:
- Intentionally reduces the whitespace without breaking the display or processing of rules.
- Is consistent with the overall formatting guidelines for prompts across the application.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/utils/actions/rule.ts (2)
485-507: Improved rule action managementAdding
deleteMany: {}to the actions object ensures that existing actions are properly cleared before creating new ones, preventing potential duplicates or orphaned actions.
630-630: Adjusted prompt formattingChanged from double newlines (
\n\n) to single newlines (\n) between rules in the prompt string. This is a minor formatting change that affects how rules appear in the prompt.Consider documenting the expected format for rule prompts to ensure consistent formatting across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(app)/automation/onboarding/CategoriesSetup.tsx(3 hunks)apps/web/app/api/ai/summarise/route.ts(2 hunks)apps/web/utils/actions/ai-rule.ts(4 hunks)apps/web/utils/actions/reply-tracking.ts(2 hunks)apps/web/utils/actions/rule.ts(7 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/web/app/(app)/automation/onboarding/CategoriesSetup.tsx (1)
apps/web/utils/actions/rule.validation.ts (2)
CreateRulesOnboardingBody(162-164)createRulesOnboardingBody(153-161)
apps/web/utils/actions/rule.ts (1)
apps/web/utils/rule/consts.ts (1)
RuleName(2-9)
🔇 Additional comments (16)
apps/web/app/(app)/automation/onboarding/CategoriesSetup.tsx (3)
38-42: Enhanced component flexibility with props destructuringThe function signature now properly accepts an optional
defaultValuesobject, allowing the component to be initialized with existing user preferences rather than always using hardcoded defaults. This is key for enabling the "Load data from server when revisiting assistant setup" functionality specified in the PR.
48-55: Good implementation of fallback defaultsThe form initialization properly uses the optional chaining operator (
?.) with fallbacks to ensure the form always has valid default values, even when partial or no default values are provided.
90-91: Field naming consistency improvedChanged from plural to singular field names (
newsletter,receipt,notification,coldEmail) to match the schema defined inrule.validation.ts. This ensures consistency between the UI components and the data validation schema, making the codebase more maintainable.Also applies to: 111-112, 118-119, 125-126
apps/web/app/api/ai/summarise/route.ts (2)
8-8: Good addition of getAiUser importThis import aligns with the broader refactoring effort to centralize user data retrieval across the application.
33-33: Improved user retrieval patternReplacing the direct Prisma query with the
getAiUserutility function enhances code maintainability and consistency across the codebase.apps/web/utils/actions/ai-rule.ts (4)
44-44: Good addition of getAiUser importAdding the import for the centralized user retrieval utility is consistent with the refactoring pattern seen across the PR.
195-195: Improved user retrieval in createAutomationActionReplacing the direct Prisma query with
getAiUsercentralizes the user retrieval logic, making the code more maintainable.
576-576: Consistent user retrieval in generateRulesPromptActionUsing
getAiUserhere maintains consistency with the pattern applied throughout the codebase.
679-679: Standardized user retrieval in reportAiMistakeActionThe change to use
getAiUserin this function completes the consistent application of the pattern across all AI rule-related actions.apps/web/utils/actions/reply-tracking.ts (2)
16-16: Good addition of getAiUser importThis import supports the consistent refactoring pattern seen across the application.
42-42: Improved user retrieval in processPreviousSentEmailsActionReplacing the direct Prisma query with the
getAiUserutility maintains consistency and improves code maintainability.apps/web/utils/actions/rule.ts (5)
41-42: Good imports for improved code qualityAdding imports for
RuleNameconstants andgetAiUserutility enhances code maintainability by centralizing rule name definitions and user retrieval logic.
395-395: Consistent user retrieval pattern appliedUsing
getAiUserhere maintains consistency with the pattern applied throughout the codebase.
548-557: Well-implemented rule deletion functionalityThe new
deleteRulefunction provides a cleaner way to remove rules when categories are deselected, improving user experience by ensuring settings accurately reflect preferences.
562-567: Improved code maintainability with RuleName constantsUsing the
RuleNameenumeration instead of hardcoded strings reduces the risk of typos and makes future changes easier to manage.Also applies to: 569-570
430-430:✅ Verification successful
Field name change from 'coldEmails' to 'coldEmail'
The singular form 'coldEmail' is now used instead of 'coldEmails'. This appears to be part of a broader naming standardization to singular forms for category actions.
Also applies to: 435-437
🏁 Script executed:
#!/bin/bash # Check if this naming change is consistent across the codebase # Find references to both singular and plural forms echo "Looking for 'coldEmail' references:" rg -g '*.ts' -g '*.tsx' 'coldEmail' echo "Looking for any remaining 'coldEmails' references that might need updating:" rg -g '*.ts' -g '*.tsx' 'coldEmails'Length of output: 16476
Approval: Naming Standardization Verified in apps/web/utils/actions/rule.ts
I've confirmed that the changes in this file correctly update the field from the plural "coldEmails" to the singular "coldEmail." In particular:
- Line 430: The condition now correctly checks
if (isSet(data.coldEmail)) {.- Lines 435–437: The corresponding logic (e.g. handling of
coldEmailBlocker) aligns with the updated naming.Note: While this file now adheres to the singular naming convention, our repository search shows that several other components (for example, in API routes and UI components like ColdEmailList and ColdEmailRejected) still reference "coldEmails" (or related plural forms such as "coldEmailers"). If a global update is intended, please verify those areas accordingly.
Summary by CodeRabbit
New Features
Bug Fixes