-
-
Couldn't load subscription status.
- Fork 221
add campaign api #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add campaign api #274
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds campaign management surfaces: OpenAPI/Zod schemas and public API routes for creating, retrieving, scheduling, pausing, and resuming campaigns (/v1/campaigns and subpaths). Implements server-side input validation, date parsing, HTML rendering and per-contact rendering, unsubscribe-placeholder detection, and domain/API-key access checks. Introduces createCampaignFromApi and getCampaignForTeam services, exposes html in campaign read/update/duplicate flows, adds a Campaign.isApi DB column, UI gating to prevent edits of API-created campaigns, registers routes in the public API, and expands SDK types/clients and docs. Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
Deploying usesend with
|
| Latest commit: |
a41c280
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9b3de5b1.usesend.pages.dev |
| Branch Preview URL: | https://km-2025-10-12-campaign-api.usesend.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (13)
apps/web/src/server/public-api/api/campaigns/resume-campaign.ts (1)
53-56: Avoid redundant DB read
getCampaignForTeamresult is not used. Drop it to save a query.- await getCampaignForTeam({ - campaignId, - teamId: team.id, - }); + // No-op fetch removedapps/docs/api-reference/openapi.json (3)
1914-2014: Document “content OR html” and natural‑language scheduledAtAlign schema with server validation.
"schema": { "type": "object", "properties": { @@ "scheduledAt": { "type": "string", - "format": "date-time" + "format": "date-time", + "description": "ISO 8601 or natural language (e.g., 'tomorrow 9am', 'next monday 10:30')" }, @@ }, "required": [ "name", "from", "subject", "contactBookId" ], + "oneOf": [ + { "required": ["content"] }, + { "required": ["html"] } + ] }
2310-2341: Clarify scheduledAt format for scheduling endpointServer accepts natural language; reflect in docs.
"scheduledAt": { "type": "string", - "format": "date-time" + "format": "date-time", + "description": "ISO 8601 or natural language (e.g., 'tomorrow 9am', 'next monday 10:30')" },
2157-2437: Consider adding standard error responses (400/403/404) for new campaign endpointsOpenAPI currently lists only 200. Add error shapes to help clients.
apps/web/src/server/public-api/api/campaigns/schedule-campaign.ts (2)
65-68: Drop unused fetch to save a query
getCampaignForTeamis called but unused.- await getCampaignForTeam({ - campaignId, - teamId: team.id, - }); + // No-op fetch removed
29-36: Optional: allow omitting body for “send now”You can set
required: falseto permit no body;{}still validates today.- body: { - required: true, + body: { + required: false, content: {apps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsx (2)
182-184: Prefer readOnly over disabled to allow text selection/copyDisabled inputs are unfocusable and can hurt accessibility. Use readOnly (with aria-readonly) and keep visual style.
- disabled={isApiCampaign} - readOnly={isApiCampaign} + readOnly={isApiCampaign} + aria-readonly={isApiCampaign}Apply similarly to Subject, From, Reply To, and Preview fields.
Also applies to: 260-262, 299-301, 335-337, 374-376
431-453: Nice gating; consider linking to API docsAdd a link to the API docs to guide users on how to update API-created campaigns.
apps/web/src/server/public-api/schemas/campaign-schema.ts (1)
44-51: Cap scheduledAt length to avoid heavy parsingLimit input length in schemas (and optionally validate in parser).
Based on learnings
- scheduledAt: z - .string() + scheduledAt: z + .string() + .max(256) .optional() @@ export const campaignScheduleSchema = z.object({ - scheduledAt: z - .string() + scheduledAt: z + .string() + .max(256) .optional()Optionally add a defensive check in
parseScheduledAt:export const parseScheduledAt = (scheduledAt?: string): Date | undefined => { if (!scheduledAt) return undefined; + if (scheduledAt.length > 256) { + throw new UnsendApiError({ + code: "BAD_REQUEST", + message: "scheduledAt is too long", + }); + }Also applies to: 57-65
apps/web/src/server/service/campaign-service.ts (4)
32-36: Precompile regexes and avoid re-creating them on replaceCurrent pattern rebuilds RegExp objects during replacement. Precompile with "gi" and reuse to reduce overhead. Static analysis warning is benign here (constants), but this also silences it.
-const CAMPAIGN_UNSUB_PLACEHOLDER_REGEXES = - CAMPAIGN_UNSUB_PLACEHOLDER_TOKENS.map((placeholder) => { - const inner = placeholder.replace(/[{}]/g, "").trim(); - return new RegExp(`\\{\\{\\s*${inner}\\s*\\}}`, "i"); - }); +const CAMPAIGN_UNSUB_PLACEHOLDER_REGEXES = + CAMPAIGN_UNSUB_PLACEHOLDER_TOKENS.map((placeholder) => { + const inner = placeholder.replace(/[{}]/g, "").trim(); + return new RegExp(`\\{\\{\\s*${inner}\\s*\\}}`, "gi"); + }); -function replaceUnsubscribePlaceholders(html: string, url: string) { - return CAMPAIGN_UNSUB_PLACEHOLDER_REGEXES.reduce((acc, regex) => { - return acc.replace(new RegExp(regex.source, "gi"), url); - }, html); -} +function replaceUnsubscribePlaceholders(html: string, url: string) { + const safeUrl = escapeHtml(url); + return CAMPAIGN_UNSUB_PLACEHOLDER_REGEXES.reduce( + (acc, regex) => acc.replace(regex, safeUrl), + html + ); +}Note: escapeHtml is introduced below.
Also applies to: 49-53
246-256: Message clarity for API create-time validationThe error says “before sending” but this is enforced at creation. Consider: “Campaign must include an unsubscribe link.”
- message: "Campaign must include an unsubscribe link before sending", + message: "Campaign must include an unsubscribe link",
258-276: Validate batchSize boundsEnforce positive integer and reasonable upper bound to avoid overload.
- ...(typeof batchSize === "number" ? { batchSize } : {}), + ...(typeof batchSize === "number" ? { batchSize } : {}),Add pre-validation above create:
if (typeof batchSize === "number") { if (!Number.isInteger(batchSize) || batchSize <= 0 || batchSize > 10000) { throw new UnsendApiError({ code: "BAD_REQUEST", message: "batchSize must be a positive integer <= 10000", }); } }
119-151: Avoid per-contact JSON.parse/renderer instantiation (hot path)This runs for every contact. Consider parsing content once per campaign batch and reusing the JSON object (or a renderer instance) for render calls.
E.g., parse in CampaignBatchService before the loop and pass parsedContent into processContactEmail, or cache by campaign.id within the batch execution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/docs/api-reference/openapi.json(1 hunks)apps/web/prisma/migrations/20251013114734_add_api_to_campaign/migration.sql(1 hunks)apps/web/prisma/schema.prisma(1 hunks)apps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsx(13 hunks)apps/web/src/server/api/routers/campaign.ts(3 hunks)apps/web/src/server/public-api/api/campaigns/create-campaign.ts(1 hunks)apps/web/src/server/public-api/api/campaigns/get-campaign.ts(1 hunks)apps/web/src/server/public-api/api/campaigns/pause-campaign.ts(1 hunks)apps/web/src/server/public-api/api/campaigns/resume-campaign.ts(1 hunks)apps/web/src/server/public-api/api/campaigns/schedule-campaign.ts(1 hunks)apps/web/src/server/public-api/index.ts(2 hunks)apps/web/src/server/public-api/schemas/campaign-schema.ts(1 hunks)apps/web/src/server/service/campaign-service.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/server/public-api/api/campaigns/create-campaign.tsapps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsxapps/web/src/server/public-api/api/campaigns/schedule-campaign.tsapps/web/src/server/public-api/schemas/campaign-schema.tsapps/web/src/server/public-api/api/campaigns/resume-campaign.tsapps/web/src/server/public-api/api/campaigns/get-campaign.tsapps/web/src/server/api/routers/campaign.tsapps/web/src/server/service/campaign-service.tsapps/web/src/server/public-api/index.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript-first: use .ts/.tsx for source code (avoid JavaScript source files)
Use 2-space indentation and semicolons (Prettier 3 enforces these)
Adhere to @usesend/eslint-config; fix all ESLint warnings (CI fails on warnings)
Do not use dynamic imports; always place imports at the top of the module
Files:
apps/web/src/server/public-api/api/campaigns/create-campaign.tsapps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsxapps/web/src/server/public-api/api/campaigns/schedule-campaign.tsapps/web/src/server/public-api/schemas/campaign-schema.tsapps/web/src/server/public-api/api/campaigns/resume-campaign.tsapps/web/src/server/public-api/api/campaigns/get-campaign.tsapps/web/src/server/api/routers/campaign.tsapps/web/src/server/service/campaign-service.tsapps/web/src/server/public-api/index.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 (run pnpm format)
Files:
apps/web/src/server/public-api/api/campaigns/create-campaign.tsapps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsxapps/web/src/server/public-api/api/campaigns/schedule-campaign.tsapps/web/src/server/public-api/schemas/campaign-schema.tsapps/web/src/server/public-api/api/campaigns/resume-campaign.tsapps/web/src/server/public-api/api/campaigns/get-campaign.tsapps/web/src/server/api/routers/campaign.tsapps/web/src/server/service/campaign-service.tsapps/web/src/server/public-api/index.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: In apps/web, use the/ alias for src imports (e.g., import { x } from "/utils/x")
Prefer using tRPC in apps/web unless explicitly asked otherwise
Files:
apps/web/src/server/public-api/api/campaigns/create-campaign.tsapps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsxapps/web/src/server/public-api/api/campaigns/schedule-campaign.tsapps/web/src/server/public-api/schemas/campaign-schema.tsapps/web/src/server/public-api/api/campaigns/resume-campaign.tsapps/web/src/server/public-api/api/campaigns/get-campaign.tsapps/web/src/server/api/routers/campaign.tsapps/web/src/server/service/campaign-service.tsapps/web/src/server/public-api/index.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsx
🧬 Code graph analysis (10)
apps/web/src/server/public-api/api/campaigns/create-campaign.ts (3)
apps/web/src/server/public-api/schemas/campaign-schema.ts (4)
campaignCreateSchema(31-55)campaignResponseSchema(70-97)CampaignCreateInput(67-67)parseScheduledAt(10-29)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)apps/web/src/server/service/campaign-service.ts (3)
createCampaignFromApi(163-280)scheduleCampaign(384-475)getCampaignForTeam(282-329)
apps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsx (1)
packages/email-editor/src/editor.tsx (1)
Editor(74-116)
apps/web/src/server/public-api/api/campaigns/schedule-campaign.ts (3)
apps/web/src/server/public-api/schemas/campaign-schema.ts (3)
campaignScheduleSchema(57-65)CampaignScheduleInput(68-68)parseScheduledAt(10-29)apps/web/src/server/service/campaign-service.ts (2)
scheduleCampaign(384-475)getCampaignForTeam(282-329)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)
apps/web/src/server/public-api/schemas/campaign-schema.ts (1)
apps/web/src/server/public-api/api-error.ts (1)
UnsendApiError(62-75)
apps/web/src/server/public-api/api/campaigns/resume-campaign.ts (2)
apps/web/src/server/service/campaign-service.ts (2)
resumeCampaign(503-534)getCampaignForTeam(282-329)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)
apps/web/src/server/public-api/api/campaigns/get-campaign.ts (3)
apps/web/src/server/public-api/schemas/campaign-schema.ts (1)
campaignResponseSchema(70-97)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)apps/web/src/server/service/campaign-service.ts (1)
getCampaignForTeam(282-329)
apps/web/src/server/api/routers/campaign.ts (1)
packages/email-editor/src/renderer.tsx (1)
EmailRenderer(172-774)
apps/web/src/server/service/campaign-service.ts (2)
packages/email-editor/src/renderer.tsx (1)
EmailRenderer(172-774)apps/web/src/server/service/domain-service.ts (2)
validateApiKeyDomainAccess(130-152)validateDomainFromEmail(85-128)
apps/web/src/server/public-api/index.ts (1)
apps/web/src/server/service/campaign-service.ts (3)
scheduleCampaign(384-475)pauseCampaign(477-501)resumeCampaign(503-534)
apps/web/src/server/public-api/api/campaigns/pause-campaign.ts (2)
apps/web/src/server/service/campaign-service.ts (2)
pauseCampaign(477-501)getCampaignForTeam(282-329)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)
🪛 ast-grep (0.39.6)
apps/web/src/server/service/campaign-service.ts
[warning] 34-34: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\{\\{\\s*${inner}\\s*\\}}, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 50-50: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(regex.source, "gi")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (6)
apps/web/src/server/public-api/api/campaigns/create-campaign.ts (1)
39-79: LGTM: create-and-optional-schedule flow is soundValidation, scheduling precedence (sendNow over scheduledAt), and response shaping look good.
apps/web/src/server/api/routers/campaign.ts (1)
243-252: LGTM: duplicate now copies html tooCopying
htmlensures visual fidelity of duplicated campaigns.apps/web/src/server/service/campaign-service.ts (4)
340-356: LGTM: robust unsubscribe placeholder validation before sendingUsing prepared HTML + content ensures both editor JSON and raw HTML paths are covered.
405-414: LGTM: error mapping for invalid content in schedulingGood propagation of parse/render errors into BAD_REQUEST.
743-747: LGTM: per-contact rendering integrates unsubscribe + variablesCorrectly leverages the new renderer utility for contact-specific HTML.
211-214: Prisma composite unique verification
Ensure the Prisma schema declares a composite unique key or compound primary key onContactBook(id, teamId). If it exists,findUnique({ where: { id: contactBookId, teamId } })is valid; otherwise usefindFirstor add the composite constraint.
| if (data.content) { | ||
| const jsonContent = data.content ? JSON.parse(data.content) : null; | ||
|
|
||
| const renderer = new EmailRenderer(jsonContent); | ||
| html = await renderer.render(); | ||
| htmlToSave = await renderer.render(); | ||
| } else if (typeof htmlInput === "string") { | ||
| htmlToSave = htmlInput; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Guard JSON parsing and drop unnecessary await
Prevent crashes on invalid JSON; render() is synchronous.
- if (data.content) {
- const jsonContent = data.content ? JSON.parse(data.content) : null;
-
- const renderer = new EmailRenderer(jsonContent);
- htmlToSave = await renderer.render();
- } else if (typeof htmlInput === "string") {
+ if (data.content) {
+ let jsonContent: unknown = null;
+ try {
+ jsonContent = JSON.parse(data.content);
+ } catch {
+ throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid content JSON" });
+ }
+ const renderer = new EmailRenderer(jsonContent as any);
+ htmlToSave = renderer.render();
+ } else if (typeof htmlInput === "string") {
htmlToSave = htmlInput;
}📝 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.
| if (data.content) { | |
| const jsonContent = data.content ? JSON.parse(data.content) : null; | |
| const renderer = new EmailRenderer(jsonContent); | |
| html = await renderer.render(); | |
| htmlToSave = await renderer.render(); | |
| } else if (typeof htmlInput === "string") { | |
| htmlToSave = htmlInput; | |
| } | |
| if (data.content) { | |
| let jsonContent: unknown = null; | |
| try { | |
| jsonContent = JSON.parse(data.content); | |
| } catch { | |
| throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid content JSON" }); | |
| } | |
| const renderer = new EmailRenderer(jsonContent as any); | |
| htmlToSave = renderer.render(); | |
| } else if (typeof htmlInput === "string") { | |
| htmlToSave = htmlInput; | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/server/api/routers/campaign.ts around lines 149 to 156, the code
currently unconditionally JSON.parse(data.content) which can throw on invalid
JSON and calls await on renderer.render() though render() is synchronous; update
it to first check data.content is a non-empty string, then try to JSON.parse
inside a try/catch and handle parse errors (e.g., set jsonContent = null or
return a validation error), instantiate EmailRenderer with the parsed or null
value, and call renderer.render() without await (assign directly) so no
unnecessary await is used.
| import { createRoute, z } from "@hono/zod-openapi"; | ||
| import { PublicAPIApp } from "~/server/public-api/hono"; | ||
| import { | ||
| getCampaignForTeam, | ||
| pauseCampaign as pauseCampaignService, | ||
| } from "~/server/service/campaign-service"; | ||
| import { campaignResponseSchema } from "~/server/public-api/schemas/campaign-schema"; | ||
|
|
||
| const route = createRoute({ | ||
| method: "post", | ||
| path: "/v1/campaigns/{campaignId}/pause", | ||
| request: { | ||
| params: z.object({ | ||
| campaignId: z | ||
| .string() | ||
| .min(1) | ||
| .openapi({ | ||
| param: { | ||
| name: "campaignId", | ||
| in: "path", | ||
| }, | ||
| example: "cmp_123", | ||
| }), | ||
| }), | ||
| }, | ||
| responses: { | ||
| 200: { | ||
| description: "Pause a campaign", | ||
| content: { | ||
| "application/json": { | ||
| schema: z.object({ | ||
| success: z.boolean(), | ||
| }), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| function pauseCampaign(app: PublicAPIApp) { | ||
| app.openapi(route, async (c) => { | ||
| const team = c.var.team; | ||
| const campaignId = c.req.param("campaignId"); | ||
|
|
||
| await pauseCampaignService({ | ||
| campaignId, | ||
| teamId: team.id, | ||
| }); | ||
|
|
||
| await getCampaignForTeam({ | ||
| campaignId, | ||
| teamId: team.id, | ||
| }); | ||
|
|
||
| return c.json({ success: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the unused import and redundant query
campaignResponseSchema is imported but never used, which ESLint treats as an error in this repo. Additionally, the extra getCampaignForTeam call only repeats the database lookup and its result is discarded. Please drop both to keep the handler lint-clean and avoid the needless query.
-import {
- getCampaignForTeam,
- pauseCampaign as pauseCampaignService,
-} from "~/server/service/campaign-service";
-import { campaignResponseSchema } from "~/server/public-api/schemas/campaign-schema";
+import { pauseCampaign as pauseCampaignService } from "~/server/service/campaign-service";
@@
- await pauseCampaignService({
- campaignId,
- teamId: team.id,
- });
-
- await getCampaignForTeam({
- campaignId,
- teamId: team.id,
- });
+ await pauseCampaignService({
+ campaignId,
+ teamId: team.id,
+ });🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/campaigns/pause-campaign.ts around lines
1-55, remove the unused import campaignResponseSchema from the top of the file
and delete the redundant await getCampaignForTeam(...) call inside the route
handler (the call’s result is never used); keep the pauseCampaignService call
and the final return c.json({ success: true }) so the handler behavior remains
the same and ESLint no longer flags the unused import or unnecessary DB query.
| campaignResponseSchema, | ||
| parseScheduledAt, | ||
| } from "~/server/public-api/schemas/campaign-schema"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove unused imports to satisfy ESLint and reduce bundle
campaignResponseSchema and parseScheduledAt are not used.
As per coding guidelines
-import {
- campaignResponseSchema,
- parseScheduledAt,
-} from "~/server/public-api/schemas/campaign-schema";
+// (no imports needed from campaign-schema here)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/campaigns/resume-campaign.ts around lines
8 to 10, the imports campaignResponseSchema and parseScheduledAt are unused;
remove them from the import list (or from the file entirely if only referenced
in that import) to satisfy ESLint and reduce bundle size, then run the
linter/build to confirm no remaining references or type errors.
| campaignScheduleSchema, | ||
| CampaignScheduleInput, | ||
| campaignResponseSchema, | ||
| parseScheduledAt, | ||
| } from "~/server/public-api/schemas/campaign-schema"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove unused import
campaignResponseSchema isn’t used.
As per coding guidelines
-import {
- campaignScheduleSchema,
- CampaignScheduleInput,
- campaignResponseSchema,
- parseScheduledAt,
-} from "~/server/public-api/schemas/campaign-schema";
+import {
+ campaignScheduleSchema,
+ CampaignScheduleInput,
+ parseScheduledAt,
+} from "~/server/public-api/schemas/campaign-schema";📝 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.
| campaignScheduleSchema, | |
| CampaignScheduleInput, | |
| campaignResponseSchema, | |
| parseScheduledAt, | |
| } from "~/server/public-api/schemas/campaign-schema"; | |
| import { | |
| campaignScheduleSchema, | |
| CampaignScheduleInput, | |
| parseScheduledAt, | |
| } from "~/server/public-api/schemas/campaign-schema"; |
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/campaigns/schedule-campaign.ts around
lines 4 to 8, the import list includes campaignResponseSchema which is not used;
remove campaignResponseSchema from the import statement (leaving
campaignScheduleSchema, CampaignScheduleInput, parseScheduledAt) so the file no
longer imports an unused symbol and adheres to coding guidelines.
| function replaceContactVariables(html: string, contact: Contact) { | ||
| return html.replace( | ||
| CONTACT_VARIABLE_REGEX, | ||
| (_, key: string, fallback?: string) => { | ||
| const valueMap: Record<string, string | null | undefined> = { | ||
| email: contact.email, | ||
| firstname: contact.firstName, | ||
| lastname: contact.lastName, | ||
| }; | ||
|
|
||
| const normalizedKey = key.toLowerCase(); | ||
| const contactValue = valueMap[normalizedKey]; | ||
|
|
||
| if (contactValue && contactValue.length > 0) { | ||
| return contactValue; | ||
| } | ||
|
|
||
| return fallback ?? ""; | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escape contact variables to prevent HTML injection in raw-HTML path
Raw replacements can inject arbitrary HTML if contact data contains angle brackets/quotes. Escape both contact values and fallbacks.
-function replaceContactVariables(html: string, contact: Contact) {
- return html.replace(
- CONTACT_VARIABLE_REGEX,
- (_, key: string, fallback?: string) => {
- const valueMap: Record<string, string | null | undefined> = {
- email: contact.email,
- firstname: contact.firstName,
- lastname: contact.lastName,
- };
-
- const normalizedKey = key.toLowerCase();
- const contactValue = valueMap[normalizedKey];
-
- if (contactValue && contactValue.length > 0) {
- return contactValue;
- }
-
- return fallback ?? "";
- }
- );
-}
+function replaceContactVariables(html: string, contact: Contact) {
+ return html.replace(
+ CONTACT_VARIABLE_REGEX,
+ (_: unknown, key: string, fallback?: string) => {
+ const valueMap: Record<string, string | null | undefined> = {
+ email: contact.email,
+ firstname: contact.firstName,
+ lastname: contact.lastName,
+ };
+ const normalizedKey = key.toLowerCase();
+ const contactValue = valueMap[normalizedKey];
+ const valueToUse =
+ contactValue && contactValue.length > 0 ? contactValue : fallback ?? "";
+ return escapeHtml(valueToUse);
+ }
+ );
+}Add this helper (outside the above range):
function escapeHtml(s: string) {
return s
.replace(/&/g, "&")
.replace(/</g, "<")
.replace(/>/g, ">")
.replace(/"/g, """)
.replace(/'/g, "'");
}🤖 Prompt for AI Agents
In apps/web/src/server/service/campaign-service.ts around lines 55 to 75,
contact variable replacements are inserted as raw HTML which allows HTML
injection if contact values or fallbacks contain special characters; add the
provided escapeHtml helper (placed outside the shown range) and call it on both
the resolved contactValue and the fallback before returning them from the
replace callback so that all returned strings are HTML-escaped (use
escapeHtml(contactValue) and escapeHtml(fallback) or empty string).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 13 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="apps/docs/api-reference/openapi.json">
<violation number="1" location="apps/docs/api-reference/openapi.json:1918">
Document that either content or html must be supplied so the OpenAPI spec matches backend validation.</violation>
<violation number="2" location="apps/docs/api-reference/openapi.json:2000">
Adjust the schedule endpoint schema to describe the broader accepted scheduledAt values instead of the restrictive date-time format.</violation>
</file>
<file name="apps/web/src/server/service/campaign-service.ts">
<violation number="1" location="apps/web/src/server/service/campaign-service.ts:72">
HTML-escape contact values and fallbacks before inserting into raw HTML to prevent injection when data contains special characters</violation>
</file>
<file name="apps/web/src/server/public-api/api/campaigns/pause-campaign.ts">
<violation number="1" location="apps/web/src/server/public-api/api/campaigns/pause-campaign.ts:50">
Remove this redundant database call; the result is unused and only adds overhead</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
apps/docs/api-reference/openapi.json
Outdated
| }, | ||
| "scheduledAt": { | ||
| "type": "string", | ||
| "format": "date-time" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjust the schedule endpoint schema to describe the broader accepted scheduledAt values instead of the restrictive date-time format.
Prompt for AI agents
Address the following comment on apps/docs/api-reference/openapi.json at line 2000:
<comment>Adjust the schedule endpoint schema to describe the broader accepted scheduledAt values instead of the restrictive date-time format.</comment>
<file context>
@@ -1910,6 +1910,531 @@
+ },
+ "scheduledAt": {
+ "type": "string",
+ "format": "date-time"
+ },
+ "batchSize": {
</file context>
| "format": "date-time" | |
| "description": "Timestamp in ISO 8601 format or natural language (e.g., 'tomorrow 9am')" |
✅ Addressed in 4b32764
| "post": { | ||
| "requestBody": { | ||
| "required": true, | ||
| "content": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document that either content or html must be supplied so the OpenAPI spec matches backend validation.
Prompt for AI agents
Address the following comment on apps/docs/api-reference/openapi.json at line 1918:
<comment>Document that either content or html must be supplied so the OpenAPI spec matches backend validation.</comment>
<file context>
@@ -1910,6 +1910,531 @@
+ "post": {
+ "requestBody": {
+ "required": true,
+ "content": {
+ "application/json": {
+ "schema": {
</file context>
| return contactValue; | ||
| } | ||
|
|
||
| return fallback ?? ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTML-escape contact values and fallbacks before inserting into raw HTML to prevent injection when data contains special characters
Prompt for AI agents
Address the following comment on apps/web/src/server/service/campaign-service.ts at line 72:
<comment>HTML-escape contact values and fallbacks before inserting into raw HTML to prevent injection when data contains special characters</comment>
<file context>
@@ -19,52 +19,339 @@ import { logger } from "../logger/log";
+ return contactValue;
+ }
+
+ return fallback ?? "";
+ }
+ );
</file context>
| teamId: team.id, | ||
| }); | ||
|
|
||
| await getCampaignForTeam({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this redundant database call; the result is unused and only adds overhead
Prompt for AI agents
Address the following comment on apps/web/src/server/public-api/api/campaigns/pause-campaign.ts at line 50:
<comment>Remove this redundant database call; the result is unused and only adds overhead</comment>
<file context>
@@ -0,0 +1,59 @@
+ teamId: team.id,
+ });
+
+ await getCampaignForTeam({
+ campaignId,
+ teamId: team.id,
</file context>
✅ Addressed in 22d1673
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/python-sdk/usesend/usesend.py (1)
84-102: Add request timeout and handle 204 responses.Avoid indefinite hangs and false errors on empty responses.
Apply:
@@ - def __init__( + def __init__( self, key: Optional[str] = None, url: Optional[str] = None, *, raise_on_error: bool = True, session: Optional[requests.Session] = None, + timeout: Optional[float] = 10.0, ) -> None: @@ - self._session = session or requests.Session() + self._session = session or requests.Session() + self.timeout = timeout @@ - resp = self._session.request( - method, f"{self.url}{path}", headers=self.headers, json=json - ) + resp = self._session.request( + method, + f"{self.url}{path}", + headers=self.headers, + json=json, + timeout=self.timeout, + ) @@ - if not resp.ok: + if not resp.ok: try: payload = resp.json() error = payload.get("error", default_error) except Exception: error = default_error if self.raise_on_error: raise UseSendHTTPError(resp.status_code, error, method, path) return None, error - - try: - return resp.json(), None - except Exception: - return None, default_error + # 204 No Content or empty body + if resp.status_code == 204 or not resp.content: + return {}, None + try: + return resp.json(), None + except Exception: + return None, default_error
♻️ Duplicate comments (1)
apps/docs/api-reference/openapi.json (1)
1914-2156: Clarify the either/or constraint for content and html in the request schema.The POST /v1/campaigns endpoint should document that either
contentorhtmlmust be supplied. Currently, both are optional in the request schema (neither appears in therequiredarray on lines 2008-2013), but the backend likely validates this constraint. Add a description or useoneOfto express this requirement.Consider adding a description near the properties or restructuring the schema to clarify the mutual requirement:
"properties": { + "description": "Either 'content' or 'html' must be provided", "name": {Alternatively, document this constraint more explicitly in the API documentation or response error messages.
🧹 Nitpick comments (13)
packages/sdk/README.md (3)
57-71: Wrap async usage and show basic error handling in examplesTop-level await may confuse users; wrap in an async function and check
errorto mirror SDK responses.-import { UseSend } from "usesend"; - -const usesend = new UseSend("us_12345"); - -// Create a campaign -const campaign = await usesend.campaigns.create({ +import { UseSend } from "usesend"; + +const usesend = new UseSend("us_12345"); + +async function main() { + // Create a campaign + const campaign = await usesend.campaigns.create({ name: "Welcome Series", from: "hello@company.com", subject: "Welcome to our platform!", contactBookId: "cb_12345", html: "<h1>Welcome!</h1><p>Thanks for joining us.</p>", sendNow: false, -}); + }); + if (campaign.error) throw new Error(campaign.error.message); + // … +} + +main().catch(console.error);
63-70: Document string or string[] for replyTo/cc/bcc with an exampleShow the accepted array shape to avoid confusion when sending multiple addresses.
const campaign = await usesend.campaigns.create({ name: "Welcome Series", from: "hello@company.com", subject: "Welcome to our platform!", contactBookId: "cb_12345", html: "<h1>Welcome!</h1><p>Thanks for joining us.</p>", + replyTo: ["support@company.com", "noreply@company.com"], + cc: ["manager@company.com"], + bcc: "auditor@company.com", sendNow: false, });
72-76: Use a future timestamp in the example and note natural‑language supportThe current date example is in the past. Also call out natural‑language scheduling here.
-// Schedule a campaign +// Schedule a campaign (ISO 8601 or natural language e.g. "next monday 9am") await usesend.campaigns.schedule(campaign.data.id, { - scheduledAt: "2024-12-01T09:00:00Z", + scheduledAt: "2025-12-01T09:00:00Z", batchSize: 1000, });packages/sdk/src/campaign.ts (3)
42-45: Remove redundant constructor assignmentParameter properties already assign
usesend; the extrathis.usesend = usesend;is unnecessary and may violate lint rules.export class Campaigns { - constructor(private readonly usesend: UseSend) { - this.usesend = usesend; - } + constructor(private readonly usesend: UseSend) {}
35-40: Derive action response type from OpenAPI for consistencyAvoid hand‑rolled
{ success: boolean }; derive from schema so changes propagate automatically.-type CampaignActionResponseSuccess = { success: boolean }; +type CampaignActionResponseSuccess = + paths["/v1/campaigns/{campaignId}/pause"]["post"]["responses"]["200"]["content"]["application/json"];Optional: add a second alias for
resumeand union them if the shapes diverge later.
77-93: Avoid sending empty bodies to no‑body endpoints (optional)Server likely ignores
{}, but if strict content negotiation is enabled this can 415. Consider allowingpost(path)without a body.- async pause(campaignId: string): Promise<CampaignActionResponse> { - const data = await this.usesend.post<CampaignActionResponseSuccess>( - `/campaigns/${campaignId}/pause`, - {}, - ); + async pause(campaignId: string): Promise<CampaignActionResponse> { + const data = await this.usesend.post<CampaignActionResponseSuccess>( + `/campaigns/${campaignId}/pause`, + undefined as unknown as void, + ); return data; } - async resume(campaignId: string): Promise<CampaignActionResponse> { - const data = await this.usesend.post<CampaignActionResponseSuccess>( - `/campaigns/${campaignId}/resume`, - {}, - ); + async resume(campaignId: string): Promise<CampaignActionResponse> { + const data = await this.usesend.post<CampaignActionResponseSuccess>( + `/campaigns/${campaignId}/resume`, + undefined as unknown as void, + ); return data; }Or, change
UseSend.postsignature to makebodyoptional and omitbodywhen undefined.packages/sdk/types/schema.d.ts (1)
1131-1153: Natural‑language scheduling documented in the schemaGood to see the note (“tomorrow 9am”, “next monday 10:30”). Consider echoing this in README (you partly do via example tweak above).
Also applies to: 1180-1192, 1209-1231
packages/python-sdk/usesend/__init__.py (2)
8-8: Sort__all__to satisfy Ruff (RUF022).Keeps exports tidy and fixes the lint warning.
Apply:
-__all__ = ["UseSend", "UseSendHTTPError", "types", "Domains", "Campaigns"] +__all__ = ["Campaigns", "Domains", "UseSend", "UseSendHTTPError", "types"]
5-5: Confirm need for# type: ignoreon import.If
.campaignstype-checks cleanly, drop the pragma to avoid masking real issues. If it’s for circulars, consider forward refs or re-export patterns instead.packages/python-sdk/usesend/usesend.py (1)
129-129: Remove unused# noqa: E402(RUF100).Not enabled; drop to reduce noise. Consider aligning pragma style with the other imports.
-from .campaigns import Campaigns # type: ignore # noqa: E402 +from .campaigns import Campaigns # type: ignorepackages/python-sdk/README.md (1)
40-62: Run Prettier on README (per repo guidelines).Ensures consistent formatting for code fences and lists.
As per coding guidelines
Also applies to: 81-91
packages/python-sdk/usesend/campaigns.py (2)
68-68: Remove unnecessary bottom import andnoqa.Forward refs make this redundant; also flagged by Ruff.
-from .usesend import UseSend # noqa: E402 pylint: disable=wrong-import-position +# (removed; forward reference via quotes is sufficient)
23-31: Optional: avoidtype: ignore[return-value]with casts.Improves type hygiene without changing runtime behavior.
Example for one method:
- return (data, err) # type: ignore[return-value] + from typing import cast, Tuple + return cast(Tuple[Optional[CampaignCreateResponse], Optional[APIError]], (data, err))(Similarly for other methods.)
Also applies to: 32-39, 40-48, 49-57, 58-66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/docs/api-reference/openapi.json(1 hunks)packages/python-sdk/README.md(2 hunks)packages/python-sdk/usesend/__init__.py(1 hunks)packages/python-sdk/usesend/campaigns.py(1 hunks)packages/python-sdk/usesend/types.py(1 hunks)packages/python-sdk/usesend/usesend.py(2 hunks)packages/sdk/README.md(1 hunks)packages/sdk/index.ts(1 hunks)packages/sdk/src/campaign.ts(1 hunks)packages/sdk/src/usesend.ts(2 hunks)packages/sdk/types/schema.d.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
packages/sdk/index.tspackages/sdk/src/usesend.tspackages/sdk/src/campaign.tspackages/sdk/types/schema.d.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript-first: use .ts/.tsx for source code (avoid JavaScript source files)
Use 2-space indentation and semicolons (Prettier 3 enforces these)
Adhere to @usesend/eslint-config; fix all ESLint warnings (CI fails on warnings)
Do not use dynamic imports; always place imports at the top of the module
Files:
packages/sdk/index.tspackages/sdk/src/usesend.tspackages/sdk/src/campaign.tspackages/sdk/types/schema.d.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 (run pnpm format)
Files:
packages/sdk/index.tspackages/sdk/README.mdpackages/python-sdk/README.mdpackages/sdk/src/usesend.tspackages/sdk/src/campaign.tspackages/sdk/types/schema.d.ts
🧬 Code graph analysis (5)
packages/python-sdk/usesend/__init__.py (1)
packages/python-sdk/usesend/campaigns.py (1)
Campaigns(17-65)
packages/python-sdk/usesend/usesend.py (2)
packages/python-sdk/usesend/campaigns.py (1)
Campaigns(17-65)packages/sdk/src/campaign.ts (1)
Campaigns(42-94)
packages/python-sdk/usesend/campaigns.py (2)
packages/python-sdk/usesend/types.py (4)
APIError(445-447)CampaignSchedule(428-430)CampaignScheduleResponse(433-434)CampaignActionResponse(437-438)packages/python-sdk/usesend/usesend.py (3)
post(107-108)get(110-111)UseSend(34-122)
packages/sdk/src/usesend.ts (2)
packages/sdk/index.ts (1)
Campaigns(3-3)packages/sdk/src/campaign.ts (1)
Campaigns(42-94)
packages/sdk/src/campaign.ts (2)
packages/sdk/types/index.ts (1)
ErrorResponse(1-4)packages/sdk/src/usesend.ts (1)
UseSend(15-133)
🪛 Ruff (0.14.0)
packages/python-sdk/usesend/__init__.py
8-8: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
packages/python-sdk/usesend/usesend.py
129-129: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
packages/python-sdk/usesend/campaigns.py
68-68: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
⏰ 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: Cloudflare Pages
🔇 Additional comments (6)
packages/sdk/src/usesend.ts (2)
5-5: New Campaigns client import is correctMatches the new resource client and keeps imports top‑level as required.
22-22: SDK surface addition is well-implemented and follows existing patternsThe
readonly campaigns = new Campaigns(this);property is properly implemented: Campaigns is imported at the top (line 5), the property follows the same pattern as emails/domains/contacts (lines 19-22), uses 2-space indentation and semicolons per Prettier 3 standards, and the Campaigns class is correctly exported from./campaign.ts. The implementation is ready to merge.packages/sdk/index.ts (1)
3-3: Export added correctly
Campaignsis now available from the package root.packages/sdk/types/schema.d.ts (1)
962-1045: Schema additions are correct with no duplicatesVerification confirms
/v1/campaignspath exists only once (line 962) with no duplicate definitions. Campaign-related endpoints are properly scoped as distinct paths (/{campaignId},/{campaignId}/schedule, etc.). The POST request/response shapes align with SDK usage.packages/python-sdk/usesend/usesend.py (1)
75-75: LGTM: campaigns client wired.Attribute initialization matches other resources and aligns with the new endpoints.
packages/python-sdk/usesend/types.py (1)
342-372: Review comment is incorrect—remove all suggested changes.The OpenAPI schema confirms that
isApidoes not exist as a campaign field, andbatchSizeandbatchWindowMinutesare required (always present) in all campaign responses. The Python SDK types are already correct:
isApifield does not exist in the API schema—do not add it.batchSizeandbatchWindowMinutesare required fields (present in all responses without optional modifiers)—keep them asint, notNotRequired[int].The Python SDK
CampaignandCampaignCreateResponsetypes already match the actual API response schema.Likely an incorrect or invalid review comment.
| "responses": { | ||
| "200": { | ||
| "description": "Create a campaign", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": { | ||
| "id": { | ||
| "type": "string" | ||
| }, | ||
| "name": { | ||
| "type": "string" | ||
| }, | ||
| "from": { | ||
| "type": "string" | ||
| }, | ||
| "subject": { | ||
| "type": "string" | ||
| }, | ||
| "previewText": { | ||
| "type": "string", | ||
| "nullable": true | ||
| }, | ||
| "contactBookId": { | ||
| "type": "string", | ||
| "nullable": true | ||
| }, | ||
| "html": { | ||
| "type": "string", | ||
| "nullable": true | ||
| }, | ||
| "content": { | ||
| "type": "string", | ||
| "nullable": true | ||
| }, | ||
| "status": { | ||
| "type": "string" | ||
| }, | ||
| "scheduledAt": { | ||
| "type": "string", | ||
| "nullable": true, | ||
| "format": "date-time" | ||
| }, | ||
| "batchSize": { | ||
| "type": "integer" | ||
| }, | ||
| "batchWindowMinutes": { | ||
| "type": "integer" | ||
| }, | ||
| "total": { | ||
| "type": "integer" | ||
| }, | ||
| "sent": { | ||
| "type": "integer" | ||
| }, | ||
| "delivered": { | ||
| "type": "integer" | ||
| }, | ||
| "opened": { | ||
| "type": "integer" | ||
| }, | ||
| "clicked": { | ||
| "type": "integer" | ||
| }, | ||
| "unsubscribed": { | ||
| "type": "integer" | ||
| }, | ||
| "bounced": { | ||
| "type": "integer" | ||
| }, | ||
| "hardBounced": { | ||
| "type": "integer" | ||
| }, | ||
| "complained": { | ||
| "type": "integer" | ||
| }, | ||
| "replyTo": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "cc": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "bcc": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "createdAt": { | ||
| "type": "string", | ||
| "format": "date-time" | ||
| }, | ||
| "updatedAt": { | ||
| "type": "string", | ||
| "format": "date-time" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "id", | ||
| "name", | ||
| "from", | ||
| "subject", | ||
| "previewText", | ||
| "contactBookId", | ||
| "html", | ||
| "content", | ||
| "status", | ||
| "scheduledAt", | ||
| "batchSize", | ||
| "batchWindowMinutes", | ||
| "total", | ||
| "sent", | ||
| "delivered", | ||
| "opened", | ||
| "clicked", | ||
| "unsubscribed", | ||
| "bounced", | ||
| "hardBounced", | ||
| "complained", | ||
| "replyTo", | ||
| "cc", | ||
| "bcc", | ||
| "createdAt", | ||
| "updatedAt" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve contradiction between required and nullable in the campaign response schema.
The response schema marks both html (lines 2129-2130) and content (lines 2129-2130) as required, but lines 2046-2053 and 2199-2206 allow them to be nullable. If these fields can be null, they should not be in the required array; if they must always be present, remove "nullable": true.
Clarify whether both fields must always be present in the response:
- "html": {
+ // If nullable and can be omitted:
+ "html": {
"type": "string",
"nullable": true
},
- "content": {
+ // If nullable and can be omitted:
+ "content": {
"type": "string",
"nullable": true
},
- "required": [
+ "required": [
"id",
"name",
"from",
"subject",
"previewText",
"contactBookId",
- "html",
- "content",
+ // Remove html and content from required if nullable
...Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/docs/api-reference/openapi.json lines 2018-2153, the campaign response
schema lists "html" and "content" as both "nullable": true and as required
properties, causing a contradiction; decide which behavior is correct and make
the schema consistent by either (A) if those fields may be null, remove "html"
and "content" from the "required" array, or (B) if they must always be present,
remove "nullable": true for those properties; update the JSON accordingly and
run any OpenAPI validation/generation step to ensure no other refs rely on the
previous contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/server/api/routers/campaign.ts (1)
130-141: Authorize contactBook by team; prevent cross‑team linking.The lookup doesn’t restrict by team; a guessed ID could be attached across teams.
- if (data.contactBookId) { - const contactBook = await db.contactBook.findUnique({ - where: { id: data.contactBookId }, - }); + if (data.contactBookId) { + const contactBook = await db.contactBook.findFirst({ + where: { id: data.contactBookId, teamId: team.id }, + }); if (!contactBook) { throw new TRPCError({ code: "BAD_REQUEST", message: "Contact book not found", }); } }
♻️ Duplicate comments (3)
apps/web/src/server/public-api/api/campaigns/pause-campaign.ts (1)
1-7: Remove unused imports to fix ESLint violations.Both
getCampaignForTeam(line 4) andcampaignResponseSchema(line 7) are imported but never used in this file. According to the coding guidelines, ESLint warnings cause CI failures in this repository.Apply this diff to remove the unused imports:
-import { - getCampaignForTeam, - pauseCampaign as pauseCampaignService, -} from "~/server/service/campaign-service"; -import { campaignResponseSchema } from "~/server/public-api/schemas/campaign-schema"; +import { pauseCampaign as pauseCampaignService } from "~/server/service/campaign-service";apps/web/src/server/api/routers/campaign.ts (2)
128-129: Good fix: use authorized campaign id from context.Switching to campaignOld.id closes the id‑forgery gap noted earlier.
150-157: Guard JSON parsing and drop unnecessary await on render().Invalid JSON will crash the request; EmailRenderer.render() is synchronous (returns string). Based on packages/email-editor/src/renderer.tsx.
- if (data.content) { - const jsonContent = data.content ? JSON.parse(data.content) : null; - - const renderer = new EmailRenderer(jsonContent); - htmlToSave = await renderer.render(); - } else if (typeof htmlInput === "string") { - htmlToSave = htmlInput; - } + if (data.content) { + let jsonContent: unknown = null; + try { + jsonContent = JSON.parse(data.content); + } catch { + throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid content JSON" }); + } + const renderer = new EmailRenderer(jsonContent as any); + htmlToSave = renderer.render(); + } else if (typeof htmlInput === "string") { + htmlToSave = htmlInput; + }
🧹 Nitpick comments (1)
apps/web/src/server/api/routers/campaign.ts (1)
114-126: Define content vs html contract (mutually exclusive) in the schema.Prevent ambiguous updates when both are sent; enforce at Zod layer.
- updateCampaign: campaignProcedure - .input( - z.object({ + updateCampaign: campaignProcedure + .input( + z + .object({ name: z.string().optional(), from: z.string().optional(), subject: z.string().optional(), previewText: z.string().optional(), content: z.string().optional(), html: z.string().optional(), contactBookId: z.string().optional(), replyTo: z.string().array().optional(), - }) - ) + }) + .refine((d) => !(d.content && d.html), { + message: "Provide either content or html, not both", + path: ["html"], + }) + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsx(13 hunks)apps/web/src/server/api/routers/campaign.ts(4 hunks)apps/web/src/server/public-api/api/campaigns/pause-campaign.ts(1 hunks)apps/web/src/server/public-api/api/campaigns/schedule-campaign.ts(1 hunks)packages/python-sdk/pyproject.toml(1 hunks)packages/sdk/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/python-sdk/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsx
- apps/web/src/server/public-api/api/campaigns/schedule-campaign.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/server/api/routers/campaign.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript-first: use .ts/.tsx for source code (avoid JavaScript source files)
Use 2-space indentation and semicolons (Prettier 3 enforces these)
Adhere to @usesend/eslint-config; fix all ESLint warnings (CI fails on warnings)
Do not use dynamic imports; always place imports at the top of the module
Files:
apps/web/src/server/api/routers/campaign.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 (run pnpm format)
Files:
apps/web/src/server/api/routers/campaign.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: In apps/web, use the/ alias for src imports (e.g., import { x } from "/utils/x")
Prefer using tRPC in apps/web unless explicitly asked otherwise
Files:
apps/web/src/server/api/routers/campaign.tsapps/web/src/server/public-api/api/campaigns/pause-campaign.ts
🧬 Code graph analysis (2)
apps/web/src/server/api/routers/campaign.ts (2)
packages/email-editor/src/renderer.tsx (1)
EmailRenderer(172-774)apps/web/src/server/db.ts (1)
db(20-20)
apps/web/src/server/public-api/api/campaigns/pause-campaign.ts (2)
apps/web/src/server/service/campaign-service.ts (1)
pauseCampaign(477-501)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)
⏰ 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: Cloudflare Pages
🔇 Additional comments (4)
packages/sdk/package.json (1)
3-3: Verify semantic versioning alignment with SDK changes.The version is bumped from 1.5.5 to 1.5.6 (PATCH increment). However, the PR summary indicates new campaign client functionality is being added to the SDK. Per semantic versioning, new features should use a MINOR bump (1.5.5 → 1.6.0), not a PATCH bump.
Please confirm:
- Are new public SDK methods/classes being added (e.g., a new Campaigns client)?
- If yes, should this be a 1.6.0 (MINOR) bump instead?
- Are there any breaking changes that would require a 2.0.0 (MAJOR) bump?
You can verify the scope of SDK changes by examining the index.ts exports and any new client classes added in the SDK package.
apps/web/src/server/api/routers/campaign.ts (3)
159-171: LGTM: staged update object and conditional html inclusion.Creating campaignUpdateData then setting html only when provided is clean and avoids unintended overwrites.
213-214: LGTM: deterministic ordering by updatedAt then createdAt.
242-254: No action required; review comment is incorrect.The middleware powering
campaignProcedurecallsdb.campaign.findUnique()without aselectoromitclause (line 232-234 in trpc.ts). In Prisma, this returns all fields from the campaign table, includinghtml. The campaign object is therefore complete when passed to the mutation context, andcampaign.htmlis safely available at line 254.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/python-sdk/usesend/types.py (1)
377-394: Document mutual requirement forcontentorhtml.Both
contentandhtmlareNotRequired, but per the PR objectives, at least one must be provided. TypedDict cannot express this constraint, so runtime validation at the API level is necessary. Consider adding a docstring toCampaignCreatenoting that eithercontentorhtmlmust be supplied.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/python-sdk/README.md(2 hunks)packages/python-sdk/usesend/types.py(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/python-sdk/README.md
🔇 Additional comments (3)
packages/python-sdk/usesend/types.py (3)
17-251: Quote style unification looks good.The conversion from single to double quotes for Literals and TypedDict keys improves consistency. No functional impact.
430-434: Good—sendNowhas been added.This addresses the previous review comment requesting
sendNowinCampaignSchedule. The field is correctly typed asOptional[bool].
436-451: LGTM!
CampaignScheduleResponse,CampaignActionResponse, andAPIErrorare simple, well-defined types that follow standard API patterns.
| Campaign = TypedDict( | ||
| "Campaign", | ||
| { | ||
| "id": str, | ||
| "name": str, | ||
| "from": str, | ||
| "subject": str, | ||
| "previewText": Optional[str], | ||
| "contactBookId": Optional[str], | ||
| "html": Optional[str], | ||
| "content": Optional[str], | ||
| "status": str, | ||
| "scheduledAt": Optional[str], | ||
| "batchSize": int, | ||
| "batchWindowMinutes": int, | ||
| "total": int, | ||
| "sent": int, | ||
| "delivered": int, | ||
| "opened": int, | ||
| "clicked": int, | ||
| "unsubscribed": int, | ||
| "bounced": int, | ||
| "hardBounced": int, | ||
| "complained": int, | ||
| "replyTo": List[str], | ||
| "cc": List[str], | ||
| "bcc": List[str], | ||
| "createdAt": str, | ||
| "updatedAt": str, | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
| CampaignCreate = TypedDict( | ||
| "CampaignCreate", | ||
| { | ||
| "name": Required[str], | ||
| "from": Required[str], | ||
| "subject": Required[str], | ||
| "previewText": NotRequired[str], | ||
| "contactBookId": Required[str], | ||
| "content": NotRequired[str], | ||
| "html": NotRequired[str], | ||
| "replyTo": NotRequired[Union[str, List[str]]], | ||
| "cc": NotRequired[Union[str, List[str]]], | ||
| "bcc": NotRequired[Union[str, List[str]]], | ||
| "sendNow": NotRequired[bool], | ||
| "scheduledAt": NotRequired[str], | ||
| "batchSize": NotRequired[int], | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
| CampaignCreateResponse = TypedDict( | ||
| "CampaignCreateResponse", | ||
| { | ||
| "id": str, | ||
| "name": str, | ||
| "from": str, | ||
| "subject": str, | ||
| "previewText": Optional[str], | ||
| "contactBookId": Optional[str], | ||
| "html": Optional[str], | ||
| "content": Optional[str], | ||
| "status": str, | ||
| "scheduledAt": Optional[str], | ||
| "batchSize": int, | ||
| "batchWindowMinutes": int, | ||
| "total": int, | ||
| "sent": int, | ||
| "delivered": int, | ||
| "opened": int, | ||
| "clicked": int, | ||
| "unsubscribed": int, | ||
| "bounced": int, | ||
| "hardBounced": int, | ||
| "complained": int, | ||
| "replyTo": List[str], | ||
| "cc": List[str], | ||
| "bcc": List[str], | ||
| "createdAt": str, | ||
| "updatedAt": str, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify contactBookId optionality.
In CampaignCreate (line 384), contactBookId is Required[str], but in Campaign (line 352) and CampaignCreateResponse (line 405), it's Optional[str]. If it's required at creation, why would it be nullable in responses?
Please confirm whether contactBookId can legitimately be None in API responses, or if the response types should declare it as a required (non-Optional) string.
🏁 Script executed:
#!/bin/bash
# Check how contactBookId is handled in the API response serialization
rg -n -A5 -B5 'contactBookId.*Campaign' --type=tsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for contactBookId usage in Python files
rg -n -A3 -B3 'contactBookId' --type=py packages/python-sdk/Length of output: 2418
🏁 Script executed:
#!/bin/bash
# Look for API documentation, examples, or tests related to Campaign
fd -e py -e md -e json -e yaml --search-path packages/python-sdk | xargs rg -l 'Campaign' | head -20Length of output: 186
🏁 Script executed:
#!/bin/bash
# Check if there are any test files or fixtures showing expected API responses
fd -e py --search-path packages/python-sdk -path '*/test*' -o -path '*/example*'Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check campaigns.py for API response handling
rg -n -A10 'Campaign' packages/python-sdk/usesend/campaigns.py | head -60Length of output: 1903
🏁 Script executed:
#!/bin/bash
# Look for test files with different search pattern
find packages/python-sdk -type f -name '*test*.py' -o -name '*spec*.py'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check README for examples of campaign creation/response
rg -n -A20 -B5 'campaign\|Campaign' packages/python-sdk/README.mdLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check the Contact type definition for comparison
rg -n -B5 -A15 'class Contact' packages/python-sdk/usesend/types.pyLength of output: 2225
Fix contactBookId type inconsistency in Campaign types.
The inconsistency is confirmed. CampaignCreate (line 384) marks contactBookId as Required[str], but both CampaignCreateResponse (line 405) and Campaign (line 352) declare it as Optional[str]. This contradicts the pattern in Contact and ContactListItem types (lines 293, 319), where contactBookId is consistently str (required, non-optional).
If contactBookId is required when creating a campaign, it should not be nullable in the response. Update lines 352 and 405 to declare "contactBookId": str instead of Optional[str], or verify with the API that it can legitimately return None and document this exceptional case.
🤖 Prompt for AI Agents
packages/python-sdk/usesend/types.py around lines 344 to 427: CampaignCreate
marks contactBookId as Required[str] but Campaign (line ~352) and
CampaignCreateResponse (line ~405) declare it Optional[str]; update those two
entries to "contactBookId": str (non-optional) so the create/response types are
consistent, or if the API can return null confirm and instead make
CampaignCreate use Optional[str]—apply the former change unless API validation
indicates otherwise.
Summary by cubic
Adds a public Campaign API with create/get/schedule/pause/resume endpoints and makes API-created campaigns read-only in the dashboard. Improves HTML rendering, scheduling, and unsubscribe safety.
New Features
Migration
Summary by CodeRabbit
New Features
SDKs
UI
Database
Bug Fixes
Documentation