Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughRemoves multiple onboarding pages/components and the categorization-preferences API, deletes ColdEmailDialog, updates Rules.tsx to always open RuleDialog for edits, and introduces STEP_KEYS/getStepNumber for dynamic onboarding step navigation used across setup and automation pages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RulesUI as Rules.tsx
participant ColdDialog as ColdEmailDialog (removed)
participant RuleDialog
participant Onboarding as OnboardingContent
rect `#f8f9fa`
Note over User,ColdDialog: Old flow (prior to this PR)
User->>RulesUI: Click "Edit Rule"
RulesUI->>ColdDialog: Open ColdEmailDialog
ColdDialog->>User: Show rule editor
end
rect `#eefaf1`
Note over User,RuleDialog: New flow (this PR)
User->>RulesUI: Click "Edit Rule"
RulesUI->>RuleDialog: Open RuleDialog in edit mode
RuleDialog->>User: Show rule editor
end
rect `#fff7e6`
Note over User,Onboarding: Onboarding step navigation
User->>RulesUI: Click "No rules" setup
RulesUI->>Onboarding: getStepNumber(STEP_KEYS.LABELS)
Onboarding-->>RulesUI: Return step number
RulesUI->>User: Redirect to /onboarding?step=X
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/app/(app)/[emailAccountId]/cold-email-blocker/page.tsx (1)
11-11: Consider makingdescriptionoptional instead of passing an empty stringPassing
description=""works, but it’s slightly unclear at the call‑site. If we expect headers without descriptions, you might letPageHeadertreat an omitted/undefined description as “no subheading” and skip rendering it.apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (1)
26-51: Centralized step metadata looks good; consider tightening typing & structureThe
STEP_KEYS/STEP_ORDER+getStepNumbersetup and thestepMap→stepsderivation give you a nice single source of truth for onboarding steps; the clamping andonNextlogic line up with it.Two optional improvements:
- Give
stepMapan explicit type likeRecord<(typeof STEP_KEYS)[keyof typeof STEP_KEYS], () => JSX.Element>so TS enforces that every key has a renderer.- Since every entry in
STEP_ORDERhas a correspondingstepMapentry,filter(isDefined)is redundant and could be dropped once the type is tightened.Overall, the refactor makes the onboarding flow more maintainable.
Also applies to: 63-94
apps/web/app/(app)/[emailAccountId]/setup/SetupContent.tsx (1)
23-26: Setup checklist integration with onboarding steps looks consistent
- Using
prefixPath(emailAccountId, \/onboarding?step=${getStepNumber(STEP_KEYS.LABELS)}`)` for the “Set up your Personal Assistant” step aligns this checklist with the unified onboarding step model.- Wrapping the completed check icon in a
Linkmakes it easy to revisit a finished step without changing the incomplete-state behavior.- Calendar icon color tweak is purely visual and consistent with other blue accents.
Once you extract
STEP_KEYS/getStepNumberinto a shared non‑client module (see AutomationPage comment), this import can be pointed there, but otherwise the setup flow changes look solid.Also applies to: 175-185, 260-263, 287-288
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/app/(app)/[emailAccountId]/assistant/Rules.tsx(3 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/ExampleDialog.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/page.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/automation/page.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailContent.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailDialog.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/cold-email-blocker/page.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/setup/SetupContent.tsx(4 hunks)apps/web/app/api/user/categorization-preferences/route.ts(0 hunks)apps/web/app/api/user/setup-progress/route.ts(0 hunks)
💤 Files with no reviewable changes (8)
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/page.tsx
- apps/web/app/api/user/setup-progress/route.ts
- apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailDialog.tsx
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/draft-replies/page.tsx
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/ExampleDialog.tsx
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
- apps/web/app/api/user/categorization-preferences/route.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 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.
Applied to files:
apps/web/app/(app)/[emailAccountId]/automation/page.tsxapps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsxapps/web/app/(app)/[emailAccountId]/cold-email-blocker/page.tsxapps/web/app/(app)/[emailAccountId]/setup/SetupContent.tsx
🧬 Code graph analysis (6)
apps/web/app/(app)/[emailAccountId]/automation/page.tsx (1)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (2)
getStepNumber(46-51)STEP_KEYS(26-34)
apps/web/app/(app)/[emailAccountId]/assistant/Rules.tsx (1)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (2)
getStepNumber(46-51)STEP_KEYS(26-34)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (8)
apps/web/app/(app)/[emailAccountId]/onboarding/StepIntro.tsx (1)
StepIntro(11-58)apps/web/app/(app)/[emailAccountId]/onboarding/StepFeatures.tsx (1)
StepFeatures(56-134)apps/web/app/(app)/[emailAccountId]/onboarding/StepWho.tsx (1)
StepWho(24-201)apps/web/app/(app)/[emailAccountId]/onboarding/StepCompanySize.tsx (1)
StepCompanySize(47-94)apps/web/app/(app)/[emailAccountId]/onboarding/StepLabels.tsx (1)
StepLabels(10-56)apps/web/app/(app)/[emailAccountId]/onboarding/StepDraft.tsx (1)
StepDraft(13-85)apps/web/app/(app)/[emailAccountId]/onboarding/StepCustomRules.tsx (1)
StepCustomRules(10-56)apps/web/utils/types.ts (1)
isDefined(12-14)
apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailContent.tsx (2)
apps/web/providers/EmailAccountProvider.tsx (1)
useAccount(79-89)apps/web/components/Typography.tsx (1)
MessageText(128-128)
apps/web/app/(app)/[emailAccountId]/cold-email-blocker/page.tsx (1)
apps/web/components/PageHeader.tsx (1)
PageHeader(14-36)
apps/web/app/(app)/[emailAccountId]/setup/SetupContent.tsx (1)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (2)
getStepNumber(46-51)STEP_KEYS(26-34)
⏰ 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). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailContent.tsx (1)
12-48: Cold email settings UX now correctly points to RulesThe new default tab, explanatory
MessageText, and “Go to Assistant Rules” button all look consistent with the unified rules flow;prefixPath(emailAccountId, "/automation?tab=rules")matches the Automation page routing.apps/web/app/(app)/[emailAccountId]/assistant/Rules.tsx (1)
65-68: Rules editing and onboarding CTA are now coherent with the new flowAlways using
RuleDialogfor “Edit manually” (witheditMode: true) and routing the “Set up default rules” CTA to/onboarding?step=${getStepNumber(STEP_KEYS.LABELS)}keeps the rules experience consistent and leverages the new shared onboarding step metadata correctly.Also applies to: 300-305, 473-477
apps/web/app/(app)/[emailAccountId]/automation/page.tsx (1)
21-24: Fix server → client import of onboarding step metadataThe review comment is accurate. Verification confirms that
automation/page.tsxis a server component (usescookies(),await prisma.rule.findFirst(), andredirect()—all server-only APIs—with no"use client"directive), whileOnboardingContent.tsxbegins with"use client".The import of
STEP_KEYSandgetStepNumberfrom this client module on lines 21–24 violates Next.js server/client boundaries, and these imports are then used in the redirect call on lines 65–71.The suggested refactor to extract these constants into a non-client shared module (e.g.,
onboarding/steps.ts) is the correct approach. No changes to the review are needed.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.