-
-
Notifications
You must be signed in to change notification settings - Fork 292
feat: v1/campaign public api endpoint #335
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
base: main
Are you sure you want to change the base?
Conversation
|
@magicspon is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
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 a GET /v1/campaigns public API endpoint with OpenAPI schema, an MDX API reference, and a server implementation. The endpoint accepts query parameters Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/docs/api-reference/openapi.json">
<violation number="1" location="apps/docs/api-reference/openapi.json:1421">
P2: The `status` field in the campaign response schema should include the same enum values as the query parameter filter for better API documentation and type generation. API consumers will receive status values but won't know what values to expect.</violation>
</file>
<file name="apps/web/src/server/public-api/api/campaigns/get-campaigns.ts">
<violation number="1" location="apps/web/src/server/public-api/api/campaigns/get-campaigns.ts:73">
P1: Page parameter is not validated. If a non-numeric string is passed (e.g., "abc"), `Number()` returns `NaN`, causing Prisma to fail. If "0" or negative values are passed, the offset becomes negative, which is invalid for Prisma's `skip`. Consider adding validation or using `Math.max(1, ...)` to ensure a valid positive page number.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const statusParam = c.req.query("status"); | ||
| const searchParam = c.req.query("search"); | ||
|
|
||
| const page = pageParam ? Number(pageParam) : 1; |
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.
P1: Page parameter is not validated. If a non-numeric string is passed (e.g., "abc"), Number() returns NaN, causing Prisma to fail. If "0" or negative values are passed, the offset becomes negative, which is invalid for Prisma's skip. Consider adding validation or using Math.max(1, ...) to ensure a valid positive page number.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/public-api/api/campaigns/get-campaigns.ts, line 73:
<comment>Page parameter is not validated. If a non-numeric string is passed (e.g., "abc"), `Number()` returns `NaN`, causing Prisma to fail. If "0" or negative values are passed, the offset becomes negative, which is invalid for Prisma's `skip`. Consider adding validation or using `Math.max(1, ...)` to ensure a valid positive page number.</comment>
<file context>
@@ -0,0 +1,133 @@
+ const statusParam = c.req.query("status");
+ const searchParam = c.req.query("search");
+
+ const page = pageParam ? Number(pageParam) : 1;
+ const limit = 30;
+ const offset = (page - 1) * limit;
</file context>
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: 2
🧹 Nitpick comments (3)
apps/docs/api-reference/openapi.json (1)
1421-1421: Consider using enum for status field in response schema.The
statusfield in the campaign response object is defined as a plain string, but the query parameterstatus(lines 1371-1381) uses an enum with specific values (DRAFT, SCHEDULED, SENDING, PAUSED, SENT, CANCELLED). For consistency and better API contract documentation, consider defining the responsestatusfield as an enum as well.🔎 Suggested improvement
- "status": { "type": "string" }, + "status": { + "type": "string", + "enum": [ + "DRAFT", + "SCHEDULED", + "SENDING", + "PAUSED", + "SENT", + "CANCELLED" + ] + },apps/web/src/server/public-api/api/campaigns/get-campaigns.ts (2)
50-50: Consider using enum for status field in response schema.The
statusfield in the response schema is defined asz.string(), but you've already defined astatusesenum for the query parameter. Usingz.enum(statuses)here would provide better type safety and consistency with the query parameter definition.🔎 Suggested improvement
- status: z.string(), + status: z.enum(statuses),
73-75: Add validation for page parameter.The page parameter is parsed with
Number(pageParam)without validation. This could result in invalid values:
Number("abc")returnsNaN- Negative numbers would cause incorrect offset calculations
- Zero would also be problematic
Consider adding validation to ensure page is a positive integer.
🔎 Suggested improvement
- const page = pageParam ? Number(pageParam) : 1; + const page = pageParam ? Math.max(1, parseInt(pageParam, 10) || 1) : 1; const limit = 30; const offset = (page - 1) * limit;This ensures the page is always at least 1, and falls back to 1 if parsing fails.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/docs/api-reference/campaigns/get-campaigns.mdxapps/docs/api-reference/openapi.jsonapps/web/src/server/public-api/api/campaigns/get-campaigns.tsapps/web/src/server/public-api/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports and ensure proper naming of key components in React/NextJS code
Files:
apps/web/src/server/public-api/api/campaigns/get-campaigns.tsapps/web/src/server/public-api/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use TypeScript-first approach with 2-space indent and semicolons enabled by Prettier in apps/web (Next.js), apps/marketing, apps/smtp-server, and all packages
Never use dynamic imports; always import on the top level
Run ESLint via @usesend/eslint-config and ensure no warnings remain before submitting PRs
Files:
apps/web/src/server/public-api/api/campaigns/get-campaigns.tsapps/web/src/server/public-api/index.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use alias
~/for src imports in apps/web (e.g.,import { x } from "~/utils/x")
Files:
apps/web/src/server/public-api/api/campaigns/get-campaigns.tsapps/web/src/server/public-api/index.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Prefer to use TRPC for client-server communication unless explicitly asked otherwise in apps/web
Use Prisma for database access in apps/web
Files:
apps/web/src/server/public-api/api/campaigns/get-campaigns.tsapps/web/src/server/public-api/index.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Run Prettier 3 for code formatting on TypeScript, TSX, and Markdown files
Files:
apps/web/src/server/public-api/api/campaigns/get-campaigns.tsapps/web/src/server/public-api/index.ts
🧬 Code graph analysis (1)
apps/web/src/server/public-api/api/campaigns/get-campaigns.ts (3)
apps/web/src/server/public-api/index.ts (1)
app(26-26)apps/web/src/server/public-api/hono.ts (1)
PublicAPIApp(136-136)apps/web/src/server/db.ts (1)
db(20-20)
⏰ 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: cubic · AI code reviewer
🔇 Additional comments (3)
apps/docs/api-reference/campaigns/get-campaigns.mdx (1)
1-3: LGTM!The front matter correctly references the new GET /v1/campaigns endpoint.
apps/web/src/server/public-api/index.ts (1)
21-21: LGTM!The import and registration of
getCampaignsfollow the established pattern and are properly grouped with other campaign-related APIs.Also applies to: 54-54
apps/web/src/server/public-api/api/campaigns/get-campaigns.ts (1)
102-129: LGTM!The database query logic is well-structured:
- Concurrent execution with
Promise.alloptimizes performance- Proper pagination with skip/take
- Case-insensitive search with OR conditions
- Results ordered by creation date (newest first)
| import { PublicAPIApp } from "~/server/public-api/hono"; | ||
| import { db } from "~/server/db"; | ||
|
|
||
| const statuses = Object.values(CampaignStatus) as [CampaignStatus]; |
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.
Fix the type assertion for enum values.
The type assertion as [CampaignStatus] creates a tuple with a single element, but z.enum() requires a tuple type of [string, ...string[]] (at least one element with rest). This could cause TypeScript errors.
🔎 Proposed fix
-const statuses = Object.values(CampaignStatus) as [CampaignStatus];
+const statuses = Object.values(CampaignStatus) as [CampaignStatus, ...CampaignStatus[]];📝 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.
| const statuses = Object.values(CampaignStatus) as [CampaignStatus]; | |
| const statuses = Object.values(CampaignStatus) as [CampaignStatus, ...CampaignStatus[]]; |
🤖 Prompt for AI Agents
In apps/web/src/server/public-api/api/campaigns/get-campaigns.ts around line 6,
the current type assertion "as [CampaignStatus]" creates a single-element tuple
which is incompatible with z.enum; change the assertion to a rest tuple
signature (e.g., "as [CampaignStatus, ...CampaignStatus[]]") so
Object.values(CampaignStatus) is typed as a proper non-empty tuple for z.enum,
or alternatively replace z.enum(...) usage with z.nativeEnum(CampaignStatus) if
that better suits the codebase.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
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.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/server/public-api/api/campaigns/get-campaigns.ts">
<violation number="1" location="apps/web/src/server/public-api/api/campaigns/get-campaigns.ts:66">
P1: Missing validation for the `page` query parameter. Invalid values like non-numeric strings, zero, or negative numbers will cause database errors. Consider adding numeric validation to the Zod schema or validating before use.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| | undefined; | ||
| const searchParam = c.req.query("search"); | ||
|
|
||
| const page = pageParam ? Number(pageParam) : 1; |
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.
P1: Missing validation for the page query parameter. Invalid values like non-numeric strings, zero, or negative numbers will cause database errors. Consider adding numeric validation to the Zod schema or validating before use.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/public-api/api/campaigns/get-campaigns.ts, line 66:
<comment>Missing validation for the `page` query parameter. Invalid values like non-numeric strings, zero, or negative numbers will cause database errors. Consider adding numeric validation to the Zod schema or validating before use.</comment>
<file context>
@@ -1,133 +1,126 @@
- const page = pageParam ? Number(pageParam) : 1;
- const limit = 30;
- const offset = (page - 1) * limit;
+ const page = pageParam ? Number(pageParam) : 1;
+ const limit = 30;
+ const offset = (page - 1) * limit;
</file context>
| const page = pageParam ? Number(pageParam) : 1; | |
| const parsedPage = pageParam ? Number(pageParam) : 1; | |
| const page = Number.isNaN(parsedPage) || parsedPage < 1 ? 1 : Math.floor(parsedPage); |
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
🧹 Nitpick comments (1)
apps/docs/docs.json (1)
92-92: LGTM! The new page entry is well-placed.The addition of "api-reference/campaigns/get-campaigns" is correct and follows a logical ordering (create → list → get single). It's consistent with the existing "get-campaign" naming pattern in the Campaigns group.
Optional improvement for future consideration: There's a cross-section naming inconsistency in the docs—Emails and Domains use "list-" for plural endpoints, while Contacts and Campaigns use "get-". Standardizing this across all sections would improve discoverability, but it's not critical for this PR.
See: #333
adds a new
/v1/campaignsendpoint to the public api. Returning an array of campaignsSummary by cubic
Add a public GET /v1/campaigns endpoint to list campaigns. Supports pagination, status filtering, and text search.
Written for commit 032c7ef. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.