Email domains & Marketing campaigns #2964
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
WalkthroughThis PR adds email domain management with API endpoints for CRUD operations, domain verification via scheduled cron job, and UI components for domain administration. It also extends campaign functionality with sender address support, scheduling capabilities for marketing campaigns, and new status states (scheduled, sending, cancelled). Database schema introduces an EmailDomain model with status tracking and updates Campaign to track sender and queue IDs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dashboard as Campaign Dashboard
participant API as Campaign API
participant DB as Database
participant QStash as QStash Queue
participant Resend as Resend Service
User->>Dashboard: Create Marketing Campaign
Dashboard->>API: POST /api/campaigns (type: marketing)
API->>DB: Save campaign (draft)
API-->>Dashboard: 201 Created
User->>Dashboard: Set sender & schedule time
Dashboard->>API: PATCH /api/campaigns/{id} (from, scheduledAt, status: scheduled)
API->>API: validateCampaign (check status transition, from address)
API->>DB: Load verified email domains
API->>DB: Update campaign (scheduled)
API->>QStash: Queue scheduled broadcast job
API->>DB: Save qstashMessageId
API-->>Dashboard: Updated campaign
Note over QStash,Resend: At scheduled time...
QStash->>API: POST /api/cron/campaigns/broadcast
API->>DB: Load campaign & groups
API->>DB: Query verified email domains
API->>Resend: Send batch emails
Resend-->>API: Email sent
API->>DB: Create CampaignEmail records
API->>DB: Update campaign (sent)
sequenceDiagram
actor User
participant Dashboard as Domain Dashboard
participant API as Email Domain API
participant Resend as Resend Service
participant DB as Database
participant Cron as Hourly Cron
User->>Dashboard: Add email domain
Dashboard->>API: POST /api/email-domains (slug)
API->>Resend: Create resend domain
Resend-->>API: resendDomainId
API->>DB: Save EmailDomain (pending)
API-->>Dashboard: Domain created
Note over Cron,DB: Every hour...
Cron->>API: GET /api/cron/email-domains/verify
API->>DB: Find domains with resendDomainId
API->>Resend: Get domain status
Resend-->>API: Status (verified/failed/etc)
API->>DB: Update lastChecked & status
API-->>Cron: Done
User->>Dashboard: View domain status
Dashboard->>API: GET /api/email-domains/{domain}/verify
API->>Resend: Fetch latest status
Resend-->>API: Current status
API->>DB: Update if changed
API-->>Dashboard: Display status badge
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
…improve verification handling
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-dns-records.tsx (1)
174-183: Critical: Fix unreachable loading branch and add error handling (duplicate issue).This issue was previously flagged but remains unresolved. The loading skeleton is unreachable because
recordsdefaults to[]on Line 186, so!recordsis never true. Additionally, there's no error handling branch.Apply the diff from the previous review to fix the conditional rendering:
- const { data, isValidating } = useSWRImmutable<GetDomainResponseSuccess>( + const { data, error, isValidating } = useSWRImmutable<GetDomainResponseSuccess>( workspaceId && `/api/email-domains/${domain.slug}/verify?workspaceId=${workspaceId}`, fetcher, { onError: (error) => { console.error("Failed to fetch email domain verification", error); }, }, ); const isVerified = data?.status === "verified"; - const records = data?.records || []; + const hasRecords = (data?.records?.length ?? 0) > 0; + const records = data?.records ?? []; return ( <div className="mt-4 space-y-4"> - {!records && !isValidating ? ( + {isValidating && !data ? ( <div className="h-20 animate-pulse rounded-lg bg-neutral-200" /> ) : isVerified ? ( <div className="flex items-center gap-2 text-pretty rounded-lg bg-green-100/80 p-3 text-sm text-green-600"> <CircleCheck className="h-5 w-5 shrink-0" /> <div> Good news! All the DNS records are verified. You are ready to start sending emails with this domain. </div> </div> - ) : records && records.length > 0 ? ( + ) : hasRecords ? ( <div className="flex flex-col gap-8"> ... </div> + ) : error ? ( + <div className="rounded-lg bg-red-100/80 p-4 text-center text-sm text-red-700"> + Failed to load verification records. Please try again. + </div> ) : ( <div className="rounded-lg bg-neutral-100/80 p-4 text-center text-sm text-neutral-600"> Loading verification records... </div> )} </div> );Also applies to: 185-187, 199-210
apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts (2)
116-125: Mark campaign as "sending" only on the first page.This status update runs unconditionally on every paginated batch. On subsequent pages, the campaign status is already "sending", so the
whereclause matches zero rows. While harmless, this is an unnecessary database call.Apply this diff to update the status only on the first page:
- // Mark the campaign as sending - await prisma.campaign.update({ - where: { - id: campaignId, - status: "scheduled", - }, - data: { - status: "sending", - }, - }); + // Mark the campaign as sending (first page only) + if (!startingAfter) { + await prisma.campaign.update({ + where: { + id: campaignId, + status: "scheduled", + }, + data: { + status: "sending", + }, + }); + }
192-198: Remove PII from console logs.Logging partner emails and names violates privacy best practices and may breach compliance requirements (GDPR, CCPA). Console logs can be persisted to external logging systems where PII retention is not controlled.
Apply this diff to log only non-PII metrics:
- console.table( - partnerUsers.map((partnerUser) => ({ - id: partnerUser.partner.id, - name: partnerUser.partner.name, - email: partnerUser.email, - })), - ); + console.log( + `Broadcasting to ${partnerUsers.length} partner users for campaign ${campaignId}`, + );apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (1)
122-127: Inconsistent conditional spread operators prevent field clearing.Lines 122-126 use truthy checks (
name &&,subject &&,from &&, etc.) that prevent clearing fields with empty strings ornull. Line 127 correctly uses!== undefinedforscheduledAt. This inconsistency can block legitimate updates where users want to clear optional fields likefrom.As noted in the previous review, the fix requires:
- Update
apps/web/lib/zod/schemas/campaigns.tsto add.nullable()to clearable fields- Change conditional checks to use
!== undefinedfor consistencyRefer to the detailed analysis and suggested fix in the existing review comment.
apps/web/app/(ee)/api/email-domains/route.ts (1)
51-63: Non-atomic existence check; rely on DB uniqueness or wrap in a transaction.Between
findFirstandcreate, a race can pass the check and rely on a later P2002. Either:
- enforce a unique index at the DB level (preferred), or
- wrap the check+create in a
$transaction(usingtx).The previous review flagged this; validation is now added, but atomicity remains.
Would you confirm a DB-level uniqueness on
emailDomain.programIdexists? Run:#!/bin/bash set -euo pipefail echo "Searching Prisma schema for EmailDomain uniqueness on programId…" fd -up --hidden --exclude node_modules schema.prisma | while read -r f; do echo "==> $f" rg -nP -C3 'model\s+EmailDomain\b(?s).*?\n}' "$f" || true rg -nP '@@unique\((?:[^)]*programId[^)]*)\)' "$f" || true doneapps/web/app/(ee)/api/email-domains/[domain]/route.ts (3)
79-90: PATCH returns stale entity and may write undefined fields.You update the DB but return the pre-update
emailDomain. Also avoid passing possibly-undefined fields to Prisma; include only defined keys.Apply:
- try { - await prisma.emailDomain.update({ + try { + const updated = await prisma.emailDomain.update({ where: { id: emailDomain.id, }, data: { - slug, - fromAddress, + ...(slug !== undefined ? { slug } : {}), + ...(fromAddress !== undefined ? { fromAddress } : {}), }, }); - return NextResponse.json(EmailDomainSchema.parse(emailDomain)); + return NextResponse.json(EmailDomainSchema.parse(updated));
94-101: Guard P2002 conflict message when slug isn’t provided.On PATCH,
slugcan be undefined; user would see “undefined domain…”. Prefer a generic message.- message: `This ${slug} domain has been registered already by another program.`, + message: "This domain has been registered already by another program.",
149-151: DELETE should not fail after DB delete due to external Resend issues.After deleting the DB record, awaiting
resend.domains.removecan 500 the request even though deletion succeeded. Make it best‑effort and non‑blocking.- if (emailDomain.resendDomainId && resend) { - await resend.domains.remove(emailDomain.resendDomainId); - } + if (emailDomain.resendDomainId) { + waitUntil( + (async () => { + try { + if (resend) { + await resend.domains.remove(emailDomain.resendDomainId!); + } else { + console.warn("Resend not configured; skipped domain removal"); + } + } catch (e) { + console.error("Failed to remove Resend domain", e); + } + })(), + ); + }
🧹 Nitpick comments (8)
apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-dns-records.tsx (3)
43-145: Consider memoizing the columns array.The
columnsarray is recreated and mutated on every render. For better performance and React best practices, wrap it inuseMemowith[showPriority, showStatus]dependencies.Example refactor:
const columns = useMemo(() => { const baseColumns = [ /* ... */ ]; if (showPriority) baseColumns.push({ /* ... */ }); if (showStatus) baseColumns.push({ /* ... */ }); return baseColumns; }, [showPriority, showStatus]);
214-214: Remove redundant conditional checks.
- Line 214: The check
records.length > 0is redundant because the parent condition on Line 210 already ensures this.- Line 234: The check
dmarcRecords.length > 0is always true sincedmarcRecordsis a static array with one element.Apply this diff:
<div className="flex flex-col gap-6"> - {records.length > 0 && ( - <DnsRecordsTable + <DnsRecordsTable title="DKIM and SPF (Required)" ... - /> - )} + /> </div> </div> <div className="flex flex-col rounded-lg border border-neutral-200 bg-neutral-100 p-4"> - {dmarcRecords.length > 0 && ( - <DnsRecordsTable + <DnsRecordsTable title="DMARC (Recommended)" ... - /> - )} + /> </div>Also applies to: 234-234
188-196: Move static dmarcRecords to module level.The
dmarcRecordsarray is static and recreated on every render. Define it as a module-level constant outside the component for better performance.Apply this diff:
+const DMARC_RECORDS = [ + { + record: "dmarc", + type: "TXT", + name: "_dmarc", + value: "v=DMARC1; p=none;", + ttl: "Auto", + }, +] as const; + export function EmailDomainDnsRecords({ domain }: EmailDomainDnsRecordsProps) { const { id: workspaceId } = useWorkspace(); ... - const dmarcRecords = [ - { - record: "dmarc", - type: "TXT", - name: "_dmarc", - value: "v=DMARC1; p=none;", - ttl: "Auto", - }, - ]; - return ( <div className="mt-4 space-y-4"> ... - records={dmarcRecords} + records={DMARC_RECORDS}apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts (2)
74-74: Fix typo in comment.- // Idealy this should not happen but just in case + // Ideally this should not happen but just in case
228-228: Consider cleaner idempotency key formatting.The idempotency key uses
${campaign.id}-${startingAfter}, which becomes"campaign-id-undefined"(string literal) on the first page. While functionally correct and unique, a cleaner format would avoid the literal "undefined" string.Consider one of these alternatives:
headers: { - "Idempotency-Key": `${campaign.id}-${startingAfter}`, + "Idempotency-Key": `${campaign.id}-${startingAfter || "0"}`, },Or:
headers: { - "Idempotency-Key": `${campaign.id}-${startingAfter}`, + "Idempotency-Key": `${campaign.id}${startingAfter ? `-${startingAfter}` : ""}`, },apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (1)
207-217: Add error handling for QStash cleanup operations.The QStash cleanup calls (lines 208, 216) lack error handling. While cleanup failures on delete are less critical than on create, they should still be caught to prevent unhandled rejections in
waitUntil.Apply this diff:
waitUntil( (async () => { if (campaign.type === "marketing" && campaign.qstashMessageId) { - await qstash.messages.delete(campaign.qstashMessageId); + try { + await qstash.messages.delete(campaign.qstashMessageId); + } catch (error) { + console.warn( + `Failed to delete QStash message ${campaign.qstashMessageId}:`, + error, + ); + } } else if (campaign.type === "transactional" && campaign.workflow) { const { condition } = parseWorkflowConfig(campaign.workflow); if (condition.attribute === "partnerJoined") { return; } - await qstash.schedules.delete(campaign.workflow.id); + try { + await qstash.schedules.delete(campaign.workflow.id); + } catch (error) { + console.warn( + `Failed to delete QStash schedule for workflow ${campaign.workflow.id}:`, + error, + ); + } } })(), );apps/web/app/(ee)/api/email-domains/[domain]/route.ts (2)
47-49: Make previous Resend-domain removal best‑effort and non‑blocking.An await here can fail the PATCH due to a transient external error even though the core update could proceed. Defer and swallow errors.
- if (emailDomain.resendDomainId) { - await resend.domains.remove(emailDomain.resendDomainId); - } + if (emailDomain.resendDomainId) { + waitUntil( + (async () => { + try { + await resend.domains.remove(emailDomain.resendDomainId!); + } catch (e) { + console.error("Failed to remove previous Resend domain", e); + } + })(), + ); + }
37-38: Type clarity: avoid truthy string in boolean condition.Make the intent explicit; prevents accidental truthiness bugs.
- const domainChanged = slug && slug !== emailDomain.slug; + const domainChanged = slug !== undefined && slug !== emailDomain.slug;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts(5 hunks)apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts(1 hunks)apps/web/app/(ee)/api/email-domains/[domain]/route.ts(1 hunks)apps/web/app/(ee)/api/email-domains/route.ts(1 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-dns-records.tsx(1 hunks)apps/web/lib/api/campaigns/schedule-campaigns.ts(1 hunks)packages/prisma/schema/campaign.prisma(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/web/app/(ee)/api/email-domains/route.ts (9)
apps/web/app/(ee)/api/cron/email-domains/verify/route.ts (1)
GET(14-59)apps/web/app/(ee)/api/email-domains/[domain]/verify/route.ts (1)
GET(10-67)apps/web/lib/auth/workspace.ts (1)
withWorkspace(42-436)apps/web/lib/zod/schemas/email-domains.ts (2)
EmailDomainSchema(9-18)createEmailDomainBodySchema(20-36)apps/web/lib/api/utils.ts (1)
parseRequestBody(9-20)apps/web/lib/api/domains/validate-email-domain.ts (1)
validateEmailDomain(3-20)apps/web/lib/api/errors.ts (1)
DubApiError(75-92)packages/email/src/resend/client.ts (1)
resend(3-5)apps/web/lib/api/create-id.ts (1)
createId(66-71)
apps/web/lib/api/campaigns/schedule-campaigns.ts (2)
apps/web/lib/api/workflows/utils.ts (1)
isScheduledWorkflow(9-22)apps/web/lib/zod/schemas/workflows.ts (1)
WORKFLOW_SCHEDULES(39-42)
apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts (6)
packages/email/src/index.ts (1)
sendBatchEmail(31-67)packages/email/src/templates/campaign-email.tsx (1)
CampaignEmail(18-129)apps/web/lib/api/workflows/render-campaign-email-html.ts (1)
renderCampaignEmailHTML(10-69)apps/web/lib/types.ts (1)
TiptapNode(648-654)apps/web/lib/api/create-id.ts (1)
createId(66-71)apps/web/lib/api/errors.ts (1)
handleAndReturnErrorResponse(175-178)
apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-dns-records.tsx (4)
apps/web/lib/types.ts (1)
EmailDomainProps(664-664)apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/constants.ts (1)
EMAIL_DOMAIN_STATUS_TO_VARIANT(3-10)packages/ui/src/table/table.tsx (2)
useTable(49-241)Table(339-662)packages/email/src/resend/types.ts (1)
GetDomainResponseSuccess(12-12)
apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (4)
apps/web/lib/api/campaigns/validate-campaign.ts (1)
validateCampaign(15-56)apps/web/lib/zod/schemas/campaigns.ts (1)
updateCampaignSchema(84-104)apps/web/lib/api/campaigns/schedule-campaigns.ts (2)
scheduleMarketingCampaign(9-79)scheduleTransactionalCampaign(82-125)apps/web/lib/api/workflows/parse-workflow-config.ts (1)
parseWorkflowConfig(8-30)
apps/web/app/(ee)/api/email-domains/[domain]/route.ts (7)
apps/web/lib/auth/workspace.ts (1)
withWorkspace(42-436)apps/web/lib/zod/schemas/email-domains.ts (2)
updateEmailDomainBodySchema(38-39)EmailDomainSchema(9-18)apps/web/lib/api/utils.ts (1)
parseRequestBody(9-20)apps/web/lib/api/domains/get-email-domain-or-throw.ts (1)
getEmailDomainOrThrow(4-32)apps/web/lib/api/domains/validate-email-domain.ts (1)
validateEmailDomain(3-20)packages/email/src/resend/client.ts (1)
resend(3-5)apps/web/lib/api/errors.ts (1)
DubApiError(75-92)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (6)
apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts (1)
249-276: LGTM: Pagination and status transitions are well-implemented.The pagination logic correctly handles batching by checking if the full page was returned, and the final status update includes a proper
whereclause to ensure atomic state transitions from "sending" to "sent".packages/prisma/schema/campaign.prisma (2)
6-18: LGTM!The new campaign statuses (
sending,sent,cancelled) properly extend the lifecycle for marketing campaigns. The comment annotations clearly distinguish between transactional and marketing campaign states.
30-30: Email validation forfromfield is already enforced.The application layer validates the
fromfield as a properly formatted email address using Zod schema validation inapps/web/lib/zod/schemas/campaigns.ts(line 91:z.string().email().trim().toLowerCase()) andapps/web/lib/actions/campaigns/send-campaign-preview-email.ts(line 19:z.string().email().optional()). No action required.apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (1)
147-161: LGTM!The scheduling logic correctly routes to the appropriate handler based on campaign type, and
waitUntilensures async operations don't block the response. Clean separation of concerns.apps/web/app/(ee)/api/email-domains/route.ts (2)
23-30: LGTM: GET handler shape/validation.Validates with
z.array(EmailDomainSchema)and returns JSON; looks good.
46-50: LGTM: Cross-field server-side validation added.
validateEmailDomainenforces fromAddress↔slug consistency; good catch-all.
apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-dns-records.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/web/app/(ee)/api/email-domains/route.ts (2)
65-71: Good: Resend config check occurs before external/DB side effects.Prevents partial success and orphaned records. This addresses earlier feedback.
51-64: Eliminate TOCTOU on “one domain per program” — rely on DB uniqueness (or wrap in a transaction).The pre-check (
findFirst) followed bycreateis racy under concurrency and can allow two creates if the schema doesn’t enforce a unique constraint onprogramId. Prefer:
- Enforce
@@unique([programId])in Prisma and remove the pre-check, mapping P2002 to a friendly message; or- Wrap check+create in an interactive transaction (consider isolation level), noting serverless constraints.
Also, the P2002 handler assumes the conflict is due to slug being registered “by another program,” which can be misleading if the constraint hit is
programId. Map byerror.meta.target.Run this to confirm uniqueness constraints in your Prisma schema:
#!/bin/bash # Locate schema files fd -t f -E node_modules 'schema.prisma' # Print EmailDomain model blocks for f in $(fd -t f -E node_modules 'schema.prisma'); do echo "== $f ==" awk '/model[[:space:]]+EmailDomain\b/{flag=1} flag{print} /\}/{if(flag){exit}}' "$f" done # Check for uniqueness on programId and slug rg -nP '@@unique\(\s*\[(?:[^\]]*programId[^\]]*)\]\)|\bprogramId\s*@unique' -S rg -nP '@@unique\(\s*\[(?:[^\]]*slug[^\]]*)\]\)|\bslug\s*@unique' -SIf you choose the DB-uniqueness path, apply this focused refactor:
- const existingEmailDomain = await prisma.emailDomain.findFirst({ - where: { - programId, - }, - }); - - if (existingEmailDomain) { - throw new DubApiError({ - code: "bad_request", - message: - "An email domain has already been configured for this program. Each program can only have one email domain.", - }); - } @@ - } catch (error) { + } catch (err) { // Cleanup to avoid orphaned Resend domains - waitUntil(resend.domains.remove(resendDomain.id)); + waitUntil( + (async () => { + try { + await resend.domains.remove(resendDomain.id); + } catch (e) { + console.error("Failed to rollback Resend domain after DB error", e); + } + })(), + ); - if (error instanceof Prisma.PrismaClientKnownRequestError) { - if (error.code === "P2002") { - throw new DubApiError({ - code: "conflict", - message: `This ${slug} domain has been registered already by another program.`, - }); - } - } + if (err instanceof Prisma.PrismaClientKnownRequestError && err.code === "P2002") { + const target = + Array.isArray((err as any).meta?.target) + ? ((err as any).meta!.target as string[]).join(",") + : ((err as any).meta?.target as string | undefined); + if (target?.includes("programId")) { + throw new DubApiError({ + code: "conflict", + message: + "An email domain has already been configured for this program. Each program can only have one email domain.", + }); + } + if (target?.includes("slug")) { + throw new DubApiError({ + code: "conflict", + message: `This ${slug} domain has been registered already by another program.`, + }); + } + } - throw new DubApiError({ - code: "internal_server_error", - message: error.message, - }); + throw new DubApiError({ + code: "internal_server_error", + message: err instanceof Error ? err.message : "Unexpected error", + }); }Also applies to: 83-94
🧹 Nitpick comments (5)
apps/web/app/(ee)/api/email-domains/route.ts (5)
58-63: Use conflict (409) instead of bad_request for existing program domain.Semantically this is a 409 conflict.
- throw new DubApiError({ - code: "bad_request", + throw new DubApiError({ + code: "conflict", message: "An email domain has already been configured for this program. Each program can only have one email domain.", });
72-82: Avoid variable shadowing; clarify Resend errors.Rename the destructured
errortoresendErrorto avoid shadowing thecatch (error)variable and improve readability.- const { data: resendDomain, error } = await resend.domains.create({ + const { data: resendDomain, error: resendError } = await resend.domains.create({ name: slug, }); - if (error) { + if (resendError) { throw new DubApiError({ code: "unprocessable_entity", - message: error.message, + message: resendError.message, }); }
95-96: Harden background verify with guarded waitUntil.Wrap
verifyin a try/catch insidewaitUntilto avoid unhandled rejections and add a minimal log.- waitUntil(resend.domains.verify(resendDomain.id)); + waitUntil( + (async () => { + try { + await resend.domains.verify(resendDomain.id); + } catch (e) { + console.error("Resend domain verify failed", e); + } + })(), + );
97-97: Return 201 Created for resource creation.Aligns status code with creation semantics.
- return NextResponse.json(EmailDomainSchema.parse(emailDomain)); + return NextResponse.json(EmailDomainSchema.parse(emailDomain), { status: 201 });
102-109: Address type-safety and improve P2002 error messaging with defensive checks and meta.target inspection.The review comment identifies two valid improvements:
Type-safety gap (line 115): Accessing
error.messagewithout guarding the error type. SinceuseUnknownInCatchVariablesis not enabled in tsconfig, TypeScript defaults to typing catch variables asany, but defensive checks prevent runtime issues if error is not an Error object.Error messaging (lines 105–107): The P2002 handler uses a generic message instead of checking
meta.target(a standard Prisma field that contains conflicting field names as an array). This allows differentiating between programId and slug conflicts to provide context-appropriate messages.The suggested diff is appropriate: wrapping
error.messagein a type guard and inspectingmeta.targetto differentiate error contexts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/(ee)/api/email-domains/route.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/app/(ee)/api/email-domains/route.ts (7)
apps/web/lib/auth/workspace.ts (1)
withWorkspace(42-436)apps/web/lib/zod/schemas/email-domains.ts (2)
EmailDomainSchema(9-18)createEmailDomainBodySchema(20-36)apps/web/lib/api/utils.ts (1)
parseRequestBody(9-20)apps/web/lib/api/domains/validate-email-domain.ts (1)
validateEmailDomain(3-20)apps/web/lib/api/errors.ts (1)
DubApiError(75-92)packages/email/src/resend/client.ts (1)
resend(3-5)apps/web/lib/api/create-id.ts (1)
createId(66-71)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (3)
apps/web/app/(ee)/api/email-domains/route.ts (3)
23-31: LGTM: Program-scoped fetch with response validation.Fetching by programId and validating with
z.array(EmailDomainSchema)is clean and consistent.
42-50: Good: Normalized input and cross-field validation.Parsing via zod +
validateEmailDomainensures lowercase normalization and fromAddress↔slug alignment.
99-101: Nice: Best-effort rollback to avoid orphaned Resend domains.This cleanup closes the external-call gap on DB failures.
…into program-email-domains
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/lib/api/campaigns/validate-campaign.ts (1)
34-49: Fix editability gate: check field presence, not truthiness.This issue was previously flagged. Falsy updates (e.g., empty string, null) bypass the guard, allowing edits to non-editable campaigns.
Apply:
- if ( - input.name || - input.subject || - input.bodyJson || - input.groupIds || - input.triggerCondition || - input.from || - input.scheduledAt - ) { - if (!CAMPAIGN_EDITABLE_STATUSES.includes(campaign.status)) { - throw new DubApiError({ - code: "bad_request", - message: `You can't make changes to a "${campaign.status}" campaign.`, - }); - } - } + const editableKeys = [ + "name", + "subject", + "bodyJson", + "groupIds", + "triggerCondition", + "from", + "scheduledAt", + ] as const; + const isEditing = editableKeys.some((k) => + Object.prototype.hasOwnProperty.call(input, k), + ); + if (isEditing && !CAMPAIGN_EDITABLE_STATUSES.includes(campaign.status)) { + throw new DubApiError({ + code: "bad_request", + message: `You can't make changes to a "${campaign.status}" campaign.`, + }); + }apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-card.tsx (1)
59-64: Auto-expand only once to avoid fighting user intent.This issue was previously flagged. The effect will re-open the panel after a user collapses it if status remains non-verified.
Apply:
+ const autoOpenedRef = useRef(false); // Automatically open DNS records section if status is not verified useEffect(() => { - if (data?.status && data.status !== "verified") { + if (!autoOpenedRef.current && data?.status && data.status !== "verified") { setShowDetails(true); + autoOpenedRef.current = true; } }, [data?.status]);apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (1)
122-127: Align conditional checks: use!== undefinedconsistently.This issue was previously flagged. The truthy checks prevent setting falsy values (empty strings or null) which may be valid clears. Line 127 correctly uses
!== undefinedforscheduledAt.However, before applying the fix, verify that the schema supports nullable values for these fields. Based on the schema in apps/web/lib/zod/schemas/campaigns.ts (lines 83-103), the fields do not have
.nullable(), so they only accept defined non-null values or can be omitted entirely.Recommended approach:
- If clearing fields (setting to null or empty string) is intended behavior, update the schema first to add
.nullable()where appropriate- Then align all conditionals to use
!== undefinedApply this pattern after schema verification:
...(name !== undefined && { name }), ...(subject !== undefined && { subject }), ...(from !== undefined && { from }), ...(status !== undefined && { status }), ...(bodyJson !== undefined && { bodyJson }), ...(scheduledAt !== undefined && { scheduledAt }),
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (1)
205-219: Consider error handling in cleanup logic.The cleanup logic is well-structured, but failures in
waitUntilare silent. Consider adding error logging or monitoring for cleanup failures.waitUntil( (async () => { + try { if (campaign.type === "marketing" && campaign.qstashMessageId) { await qstash.messages.delete(campaign.qstashMessageId); } else if (campaign.type === "transactional" && campaign.workflow) { const { condition } = parseWorkflowConfig(campaign.workflow); if (condition.attribute === "partnerJoined") { return; } await qstash.schedules.delete(campaign.workflow.id); } + } catch (error) { + console.error(`Failed to cleanup campaign ${campaignId}:`, error); + } })(), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts(5 hunks)apps/web/app/(ee)/api/campaigns/route.ts(1 hunks)apps/web/app/(ee)/api/email-domains/[domain]/route.ts(1 hunks)apps/web/app/(ee)/api/email-domains/route.ts(1 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-card.tsx(1 hunks)apps/web/lib/api/campaigns/validate-campaign.ts(1 hunks)apps/web/lib/zod/schemas/email-domains.ts(1 hunks)apps/web/tests/campaigns/index.test.ts(1 hunks)apps/web/ui/modals/add-edit-email-domain-modal.tsx(1 hunks)apps/web/ui/modals/delete-email-domain-modal.tsx(1 hunks)packages/prisma/schema/domain.prisma(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/app/(ee)/api/email-domains/route.ts
- apps/web/ui/modals/delete-email-domain-modal.tsx
- apps/web/app/(ee)/api/email-domains/[domain]/route.ts
- apps/web/ui/modals/add-edit-email-domain-modal.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (5)
apps/web/lib/api/campaigns/validate-campaign.ts (1)
validateCampaign(16-85)apps/web/lib/zod/schemas/campaigns.ts (1)
updateCampaignSchema(84-104)apps/web/lib/api/utils.ts (1)
parseRequestBody(9-20)apps/web/lib/api/campaigns/schedule-campaigns.ts (2)
scheduleMarketingCampaign(9-79)scheduleTransactionalCampaign(82-125)apps/web/lib/api/workflows/parse-workflow-config.ts (1)
parseWorkflowConfig(8-30)
apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-card.tsx (7)
apps/web/lib/types.ts (1)
EmailDomainProps(664-664)apps/web/lib/swr/use-workspace.ts (1)
useWorkspace(6-46)packages/ui/src/hooks/use-in-viewport.tsx (1)
useInViewport(5-55)apps/web/ui/modals/add-edit-email-domain-modal.tsx (1)
useAddEditEmailDomainModal(171-186)apps/web/ui/modals/delete-email-domain-modal.tsx (1)
useDeleteEmailDomainModal(121-151)apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/constants.ts (1)
EMAIL_DOMAIN_STATUS_TO_VARIANT(3-10)apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-dns-records.tsx (1)
EmailDomainDnsRecords(171-255)
apps/web/lib/api/campaigns/validate-campaign.ts (4)
apps/web/lib/zod/schemas/campaigns.ts (1)
updateCampaignSchema(84-104)apps/web/lib/api/campaigns/constants.ts (2)
CAMPAIGN_STATUS_TRANSITIONS(8-24)CAMPAIGN_EDITABLE_STATUSES(26-30)apps/web/lib/api/errors.ts (1)
DubApiError(75-92)packages/prisma/index.ts (1)
prisma(3-9)
apps/web/lib/zod/schemas/email-domains.ts (2)
packages/prisma/client.ts (1)
EmailDomainStatus(12-12)apps/web/lib/api/domains/is-valid-domain.ts (2)
isValidDomainFormat(11-13)isValidDomain(3-9)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (11)
apps/web/tests/campaigns/index.test.ts (1)
54-54: LGTM: Correct HTTP status for resource creation.The change to expect 201 (Created) aligns with RESTful semantics for POST operations that create new resources.
apps/web/app/(ee)/api/campaigns/route.ts (1)
94-99: LGTM: Correct HTTP status for POST.Returning 201 (Created) is the correct HTTP status for successful resource creation.
packages/prisma/schema/domain.prisma (2)
63-69: LGTM: Well-defined status enum.The status values cover all necessary states for domain verification lifecycle.
71-86: Disregard the index suggestion –workspaceIdis not used in database queries.The review incorrectly assumes
workspaceIdis used in query filters. However, the actual implementation inapps/web/app/(ee)/api/email-domains/route.ts(line 22-26) filters EmailDomain records only byprogramId, notworkspaceId:const emailDomains = await prisma.emailDomain.findMany({ where: { programId, }, });Although
workspaceIdis stored on the EmailDomain model and passed as a query parameter by the frontend, it is never used in WHERE clauses. Indexes only improve performance when used in filtering, sorting, or join conditions—sinceworkspaceIdserves neither role here, adding an index would not provide any benefit.Likely an incorrect or invalid review comment.
apps/web/lib/zod/schemas/email-domains.ts (3)
8-16: LGTM: Well-structured domain schema.The schema correctly captures all domain properties with appropriate types.
18-28: LGTM: Comprehensive domain validation.The validation chain properly enforces format, length constraints, and domain restrictions while normalizing to lowercase.
30-31: LGTM: Appropriate use of partial schema.Using
.partial()correctly enables optional field updates.apps/web/app/app.dub.co/(dashboard)/[slug]/settings/domains/email/email-domain-card.tsx (2)
37-43: LGTM: Efficient lazy loading pattern.Using viewport visibility to gate the SWR fetch is a good optimization for performance when many domain cards are rendered.
101-115: LGTM: Well-implemented refresh control.The refresh button correctly uses the loading state and provides good visual feedback to users.
apps/web/app/(ee)/api/campaigns/[campaignId]/route.ts (2)
64-76: LGTM: Proper validation integration.The destructuring pattern correctly extracts validated fields for downstream use.
147-161: LGTM: Clean type-based scheduling dispatch.The separation of marketing and transactional campaign scheduling logic is well-structured and appropriately uses
waitUntilfor non-blocking execution.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/lib/api/campaigns/validate-campaign.ts (1)
37-52: Fix editability gate: check field presence, not truthiness.Falsy updates (e.g., empty string, null) bypass the guard. This issue was previously flagged and remains unresolved. Check for key presence using
Object.prototype.hasOwnProperty.call(input, key)or theinoperator instead of truthiness.Apply the previously suggested fix:
- if ( - input.name || - input.subject || - input.bodyJson || - input.groupIds || - input.triggerCondition || - input.from || - input.scheduledAt - ) { - if (!CAMPAIGN_EDITABLE_STATUSES.includes(campaign.status)) { - throw new DubApiError({ - code: "bad_request", - message: `You can't make changes to a "${campaign.status}" campaign.`, - }); - } - } + const editableKeys = [ + "name", + "subject", + "bodyJson", + "groupIds", + "triggerCondition", + "from", + "scheduledAt", + ] as const; + const isEditing = editableKeys.some((k) => + Object.prototype.hasOwnProperty.call(input, k), + ); + if (isEditing && !CAMPAIGN_EDITABLE_STATUSES.includes(campaign.status)) { + throw new DubApiError({ + code: "bad_request", + message: `You can't make changes to a "${campaign.status}" campaign.`, + }); + }
🧹 Nitpick comments (2)
apps/web/lib/api/campaigns/validate-campaign.ts (2)
16-17: Offer to implement required fields validation.The TODO highlights an important validation gap: ensuring campaigns have all required fields before transitioning to certain statuses (e.g., from/subject/body before moving to scheduled/active).
Would you like me to generate the implementation for this validation logic, or open an issue to track it?
100-107: Email format already validated by schema.The
split("@")check is redundant sinceupdateCampaignSchemaalready validates email format withz.string().email(). However, keeping this as a defensive check for domain extraction is acceptable.If you prefer removing the redundancy, you can simplify to:
- const parts = campaign.from.split("@"); - - if (parts.length !== 2) { - throw new DubApiError({ - code: "bad_request", - message: `Campaign (${campaign.id}) has an invalid email address format for 'from' field.`, - }); - } - - const domainPart = parts[1]; + const domainPart = campaign.from.split("@")[1];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts(1 hunks)apps/web/lib/api/campaigns/validate-campaign.ts(1 hunks)apps/web/lib/api/workflows/execute-send-campaign-workflow.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(ee)/api/cron/campaigns/broadcast/route.ts
- apps/web/lib/api/workflows/execute-send-campaign-workflow.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/lib/api/campaigns/validate-campaign.ts (4)
apps/web/lib/zod/schemas/campaigns.ts (1)
updateCampaignSchema(84-104)apps/web/lib/api/campaigns/constants.ts (2)
CAMPAIGN_STATUS_TRANSITIONS(8-24)CAMPAIGN_EDITABLE_STATUSES(26-30)apps/web/lib/api/errors.ts (1)
DubApiError(75-92)packages/prisma/index.ts (1)
prisma(3-9)
⏰ 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). (1)
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/lib/api/campaigns/validate-campaign.ts (1)
34-49: Critical: Editability guard still checks truthiness, not key presence.This issue was flagged in a previous review but remains unaddressed. Falsy updates (empty string,
null,0,false) will bypass the editability check, allowing modifications to non-editable campaigns.Apply the fix from the previous review:
- if ( - input.name || - input.subject || - input.bodyJson || - input.groupIds || - input.triggerCondition || - input.from || - input.scheduledAt - ) { - if (!CAMPAIGN_EDITABLE_STATUSES.includes(campaign.status)) { - throw new DubApiError({ - code: "bad_request", - message: `You can't make changes to a "${campaign.status}" campaign.`, - }); - } - } + const editableKeys = [ + "name", + "subject", + "bodyJson", + "groupIds", + "triggerCondition", + "from", + "scheduledAt", + ] as const; + const isEditing = editableKeys.some((k) => + Object.prototype.hasOwnProperty.call(input, k), + ); + if (isEditing && !CAMPAIGN_EDITABLE_STATUSES.includes(campaign.status)) { + throw new DubApiError({ + code: "bad_request", + message: `You can't make changes to a "${campaign.status}" campaign.`, + }); + }
🧹 Nitpick comments (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/campaigns/[campaignId]/campaign-editor.tsx (2)
82-83: Consider allowing domain selection when multiple domains exist.The code always uses the first email domain for the From field. If users have multiple verified domains, they cannot choose which one to use. Consider adding a domain selector in the From field UI.
291-341: Provide actionable guidance when no email domains exist.When
!firstEmailDomain, the From field is disabled with the tooltip "No email domain configured," but users have no clear path to add a domain. Consider providing a link to the email domain settings or more actionable guidance.Example approach:
tooltip={ isReadOnly ? statusMessages[campaign.status] : !firstEmailDomain ? "No email domain configured. Add one in Settings." : "" }Or add a helper link in the UI when domains are missing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/campaigns/[campaignId]/campaign-editor.tsx(11 hunks)apps/web/lib/api/campaigns/validate-campaign.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/campaigns/[campaignId]/campaign-editor.tsx (4)
apps/web/lib/swr/use-program.ts (1)
useProgram(6-40)apps/web/lib/swr/use-workspace.ts (1)
useWorkspace(6-46)apps/web/lib/swr/use-email-domains.ts (1)
useEmailDomains(6-25)apps/web/lib/api/campaigns/constants.ts (1)
CAMPAIGN_READONLY_STATUSES(32-36)
apps/web/lib/api/campaigns/validate-campaign.ts (4)
apps/web/lib/zod/schemas/campaigns.ts (1)
updateCampaignSchema(84-104)apps/web/lib/api/campaigns/constants.ts (2)
CAMPAIGN_STATUS_TRANSITIONS(8-24)CAMPAIGN_EDITABLE_STATUSES(26-30)apps/web/lib/api/errors.ts (1)
DubApiError(75-92)packages/prisma/index.ts (1)
prisma(3-9)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (4)
apps/web/lib/api/campaigns/validate-campaign.ts (1)
60-74: From address validation correctly updated.The previous review concern has been properly addressed. The code now validates the new
input.fromaddress by spreading the campaign object and overwriting thefromfield before passing it tovalidateCampaignFromAddress.apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/campaigns/[campaignId]/campaign-editor.tsx (3)
73-77: Status messages are clear and well-defined.The
statusMessagesobject provides helpful tooltip text for each read-only campaign status.
348-362: Recipients field correctly handles both active and read-only states.The tooltip logic appropriately differentiates between active campaigns (which are paused to edit) and read-only campaigns (which cannot be edited at all).
434-436: Body editability logic correctly handles campaign types.The asymmetric logic (read-only for marketing, not-active for transactional) aligns with the campaign lifecycle: marketing campaigns become immutable once sending begins, while transactional campaigns can be paused and edited.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes