feat: implement createTask function and associated tests#1655
feat: implement createTask function and associated tests#1655pradipthaadhi wants to merge 1 commit intotestfrom
Conversation
- Added createTask function to handle task creation via the Recoup API, including error handling for various response scenarios. - Created unit tests for createTask to verify correct API interaction, including optional parameters and error cases.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/tasks/createTask.ts (1)
20-23: Consider validatingaccessTokento fail fast.The function accepts any string for
accessToken, including empty strings. While the API will reject invalid tokens, failing fast with a descriptive error improves debugging experience.🛡️ Optional: Add input validation
export async function createTask( accessToken: string, params: CreateTaskParams, ): Promise<ScheduledAction> { + if (!accessToken) { + throw new Error("accessToken is required"); + } + const response = await fetch(`${getClientApiBaseUrl()}/api/tasks`, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/createTask.ts` around lines 20 - 23, The createTask function currently accepts any string for accessToken which can be empty; add a fast-fail input validation at the top of createTask to check that accessToken is a non-empty string (and optionally well-formed if you have a pattern), and throw a clear, descriptive error (e.g., Error('createTask: accessToken is required')) before proceeding to use CreateTaskParams or returning a ScheduledAction; update callers/tests if necessary to handle the thrown error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/tasks/createTask.ts`:
- Around line 35-36: The spread construction in createTask.ts is inconsistent:
it uses a truthy check for params.account_id but an explicit !== undefined check
for params.model, which treats empty string and null differently; update both
spreads to use the same explicit presence check (e.g., params.account_id !==
undefined and params.model !== undefined) or, if you intend to exclude empty
strings for account_id, add a clarifying comment above the account_id spread
explaining that empty strings must be excluded—refer to the params object fields
account_id and model and make the checks consistent across the two spreads.
---
Nitpick comments:
In `@lib/tasks/createTask.ts`:
- Around line 20-23: The createTask function currently accepts any string for
accessToken which can be empty; add a fast-fail input validation at the top of
createTask to check that accessToken is a non-empty string (and optionally
well-formed if you have a pattern), and throw a clear, descriptive error (e.g.,
Error('createTask: accessToken is required')) before proceeding to use
CreateTaskParams or returning a ScheduledAction; update callers/tests if
necessary to handle the thrown error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02121c33-755b-4889-bc74-6bc63c9cae1e
⛔ Files ignored due to path filters (1)
lib/tasks/__tests__/createTask.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (1)
lib/tasks/createTask.ts
| ...(params.account_id ? { account_id: params.account_id } : {}), | ||
| ...(params.model !== undefined ? { model: params.model } : {}), |
There was a problem hiding this comment.
Inconsistent handling of optional parameters.
Line 35 uses a truthiness check for account_id, which excludes both undefined and empty string "". Line 36 uses an explicit !== undefined check for model, which would include empty strings and null values.
If account_id should never be an empty string, this is fine. However, for consistency and to make the intent explicit, consider aligning both patterns:
♻️ Suggested alignment for consistency
- ...(params.account_id ? { account_id: params.account_id } : {}),
+ ...(params.account_id !== undefined ? { account_id: params.account_id } : {}),
...(params.model !== undefined ? { model: params.model } : {}),Or if empty strings should truly be excluded for account_id, add a comment explaining why:
+ // Exclude empty strings - account_id must be a valid non-empty ID
...(params.account_id ? { account_id: params.account_id } : {}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/tasks/createTask.ts` around lines 35 - 36, The spread construction in
createTask.ts is inconsistent: it uses a truthy check for params.account_id but
an explicit !== undefined check for params.model, which treats empty string and
null differently; update both spreads to use the same explicit presence check
(e.g., params.account_id !== undefined and params.model !== undefined) or, if
you intend to exclude empty strings for account_id, add a clarifying comment
above the account_id spread explaining that empty strings must be excluded—refer
to the params object fields account_id and model and make the checks consistent
across the two spreads.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 2 files
Confidence score: 4/5
- This PR looks safe to merge overall; the main issues are minor consistency concerns rather than clear runtime breakage.
- Most notable:
lib/tasks/createTask.tsreturnsScheduledActionbut parsesTask-typed data, which could confuse consumers or type checks. - Inconsistent optional param handling in
lib/tasks/createTask.ts(account_idtruthy check vsmodel !== undefined) may lead to subtle input differences but is not likely to be merge-blocking. - Pay close attention to
lib/tasks/createTask.ts- return type consistency and optional parameter checks.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/tasks/__tests__/createTask.test.ts">
<violation number="1" location="lib/tasks/__tests__/createTask.test.ts:1">
P1: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
This test file is 136 lines, exceeding the 100-line limit. Split the error-case tests (HTTP error, API error status, empty tasks) into a separate file like `createTask.errors.test.ts` to keep each file under 100 lines.</violation>
</file>
<file name="lib/tasks/createTask.ts">
<violation number="1" location="lib/tasks/createTask.ts:23">
P2: Return type should be `Task` instead of `ScheduledAction` to match the parsed response type (`GetTasksResponse.tasks` is `Task[]`) and stay consistent with `getTasks`, which returns `Task[]`. The current `ScheduledAction` return type discards the optional `recent_runs` and `upcoming` fields, which callers can't access without casting.</violation>
<violation number="2" location="lib/tasks/createTask.ts:35">
P3: Inconsistent conditional patterns for optional parameters: `account_id` uses a truthiness check (excluding `undefined` and `""`) while `model` uses `!== undefined` (allowing `null` and `""`). If both should follow the same exclusion logic, align them. If the difference is intentional (e.g., `model` deliberately allows `null` to clear the field), add a brief comment to make that intent explicit.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| @@ -0,0 +1,136 @@ | |||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | |||
There was a problem hiding this comment.
P1: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
This test file is 136 lines, exceeding the 100-line limit. Split the error-case tests (HTTP error, API error status, empty tasks) into a separate file like createTask.errors.test.ts to keep each file under 100 lines.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/__tests__/createTask.test.ts, line 1:
<comment>This test file is 136 lines, exceeding the 100-line limit. Split the error-case tests (HTTP error, API error status, empty tasks) into a separate file like `createTask.errors.test.ts` to keep each file under 100 lines.</comment>
<file context>
@@ -0,0 +1,136 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { createTask } from "@/lib/tasks/createTask";
+import { getClientApiBaseUrl } from "@/lib/api/getClientApiBaseUrl";
</file context>
| export async function createTask( | ||
| accessToken: string, | ||
| params: CreateTaskParams, | ||
| ): Promise<ScheduledAction> { |
There was a problem hiding this comment.
P2: Return type should be Task instead of ScheduledAction to match the parsed response type (GetTasksResponse.tasks is Task[]) and stay consistent with getTasks, which returns Task[]. The current ScheduledAction return type discards the optional recent_runs and upcoming fields, which callers can't access without casting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/createTask.ts, line 23:
<comment>Return type should be `Task` instead of `ScheduledAction` to match the parsed response type (`GetTasksResponse.tasks` is `Task[]`) and stay consistent with `getTasks`, which returns `Task[]`. The current `ScheduledAction` return type discards the optional `recent_runs` and `upcoming` fields, which callers can't access without casting.</comment>
<file context>
@@ -0,0 +1,55 @@
+export async function createTask(
+ accessToken: string,
+ params: CreateTaskParams,
+): Promise<ScheduledAction> {
+ const response = await fetch(`${getClientApiBaseUrl()}/api/tasks`, {
+ method: "POST",
</file context>
| prompt: params.prompt, | ||
| schedule: params.schedule, | ||
| artist_account_id: params.artist_account_id, | ||
| ...(params.account_id ? { account_id: params.account_id } : {}), |
There was a problem hiding this comment.
P3: Inconsistent conditional patterns for optional parameters: account_id uses a truthiness check (excluding undefined and "") while model uses !== undefined (allowing null and ""). If both should follow the same exclusion logic, align them. If the difference is intentional (e.g., model deliberately allows null to clear the field), add a brief comment to make that intent explicit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/createTask.ts, line 35:
<comment>Inconsistent conditional patterns for optional parameters: `account_id` uses a truthiness check (excluding `undefined` and `""`) while `model` uses `!== undefined` (allowing `null` and `""`). If both should follow the same exclusion logic, align them. If the difference is intentional (e.g., `model` deliberately allows `null` to clear the field), add a brief comment to make that intent explicit.</comment>
<file context>
@@ -0,0 +1,55 @@
+ prompt: params.prompt,
+ schedule: params.schedule,
+ artist_account_id: params.artist_account_id,
+ ...(params.account_id ? { account_id: params.account_id } : {}),
+ ...(params.model !== undefined ? { model: params.model } : {}),
+ }),
</file context>
There was a problem hiding this comment.
YAGNI principle - Where is this lib being used?
2 participants
Summary by CodeRabbit