feat: include task owner email in tasks response#418
feat: include task owner email in tasks response#418arpitgupta1214 wants to merge 1 commit intotestfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request consolidates task enrichment logic by removing the per-task Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
arpitgupta1214
force-pushed
the
codex/task-list-owner-email-api
branch
from
f48c06f to
650512b
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Confidence score: 4/5
- Test maintainability concern in
lib/tasks/__tests__/getTasksHandler.test.ts: duplicated inline task fixtures and a 124-line file could make future changes harder, but it’s unlikely to affect runtime behavior. - Overall risk is low since the only issue is a style/maintainability note in tests, so this looks safe to merge.
- Pay close attention to
lib/tasks/__tests__/getTasksHandler.test.ts- consider extracting shared fixtures to reduce duplication and length.
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__/getTasksHandler.test.ts">
<violation number="1" location="lib/tasks/__tests__/getTasksHandler.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
This test file is 124 lines, exceeding the 100-line limit. The task fixture objects are defined inline three times (raw tasks, mock return, assertion). Extract shared test fixtures into a helper (e.g., `makeTask()` factory) to eliminate the repetition and bring the file under 100 lines.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant API as getTasksHandler
participant DB as Supabase (PostgreSQL)
participant Logic as NEW: enrichTasks
participant Trigger as Trigger.dev API
Client->>API: GET /api/tasks
API->>API: validateGetTasksQuery()
alt Validation Failed
API-->>Client: Return Error Response
else Validation Success
API->>DB: selectScheduledActions(query)
DB-->>API: tasks[]
Note over API,Trigger: Start Bulk Enrichment Process
API->>Logic: NEW: enrichTasks(tasks)
par NEW: Fetch Account Emails
Logic->>DB: NEW: selectAccountEmails(uniqueAccountIds)
DB-->>Logic: account_id mapping (emails)
and CHANGED: Batch Trigger Metadata
loop For each task with trigger_schedule_id
Logic->>Trigger: fetchTriggerRuns(scheduleId)
Trigger-->>Logic: recentRuns[]
opt if runs exist
Logic->>Trigger: retrieveTaskRun(latestRunId)
Trigger-->>Logic: payload.upcoming[]
end
end
end
Note over Logic: Map emails and trigger data to task objects
Logic-->>API: enrichedTasks[] (including owner_email)
API-->>Client: 200 OK (tasks with owner_email, runs, and upcoming)
end
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,124 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
This test file is 124 lines, exceeding the 100-line limit. The task fixture objects are defined inline three times (raw tasks, mock return, assertion). Extract shared test fixtures into a helper (e.g., makeTask() factory) to eliminate the repetition and bring the 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__/getTasksHandler.test.ts, line 1:
<comment>This test file is 124 lines, exceeding the 100-line limit. The task fixture objects are defined inline three times (raw tasks, mock return, assertion). Extract shared test fixtures into a helper (e.g., `makeTask()` factory) to eliminate the repetition and bring the file under 100 lines.</comment>
<file context>
@@ -0,0 +1,124 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+import { getTasksHandler } from "@/lib/tasks/getTasksHandler";
</file context>
![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 (2)
lib/tasks/getTasksHandler.ts (1)
14-47: RefactorgetTasksHandlerinto smaller units.This handler is over the 20-line guideline. Consider extracting success/error response builders (private helpers) to keep orchestration concise.
As per coding guidelines,
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: “Flag functions longer than 20 lines” and “Keep functions small and focused”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/getTasksHandler.ts` around lines 14 - 47, The getTasksHandler function is too long; extract two private helpers—buildTasksSuccessResponse(enrichedTasks) and buildErrorResponse(error)—that each return the corresponding NextResponse.json payload and headers (reuse getCorsHeaders()), then refactor getTasksHandler to simply validate the query via validateGetTasksQuery, call selectScheduledActions and enrichTasks, and return buildTasksSuccessResponse(enrichedTasks) inside the try block and buildErrorResponse(error) in the catch; keep references to validateGetTasksQuery, selectScheduledActions, enrichTasks, and getCorsHeaders to locate where to wire the helpers.lib/tasks/enrichTasks.ts (1)
27-84: SplitenrichTasksinto smaller private helpers.
enrichTasksis doing multiple jobs and is substantially over the function-size limits. Extracting internal helpers (e.g., trigger enrichment + email lookup) will improve readability and testability.As per coding guidelines,
lib/**/*.ts: “Single responsibility per function” and “Keep functions under 50 lines”; and**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: “Flag functions longer than 20 lines” and “Keep functions small and focused”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/enrichTasks.ts` around lines 27 - 84, enrichTasks is doing too many things; split it into smaller private helpers to follow single-responsibility and size guidelines: extract the trigger enrichment logic into a helper (e.g., fetchTriggerInfoEntries or enrichTriggerForTasks) that takes ScheduledAction[] and returns TriggerInfoEntry[] by moving the Promise.all(tasks.map(...)) block and its inner logic (fetchTriggerRuns, retrieveTaskRun, payload parsing, and error handling) out of enrichTasks; extract the account/email logic into a helper (e.g., getAccountEmailMap or fetchAccountEmails) that calls selectAccountEmails and builds the emailByAccountId Map; then have enrichTasks call these helpers and assemble the final array (or use a small buildEnrichedTasks helper) so each function (fetchTriggerInfoEntries, getAccountEmailMap, buildEnrichedTasks) is focused and under the size limits while reusing the existing symbols (enrichTasks, fetchTriggerRuns, retrieveTaskRun, selectAccountEmails).
🤖 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/enrichTasks.ts`:
- Around line 49-57: The two empty catch blocks in lib/tasks/enrichTasks.ts
silently drop Trigger enrichment (the payload retrieval catch and the outer
Trigger.dev API catch) — update both to emit telemetry and contextual logs
instead of failing silently: capture and report the thrown error (message/stack)
and include task identifiers (e.g., task.id, any trigger id) plus which path was
taken (payload vs API fallback), and increment a telemetry/metric counter for
Trigger enrichment fallbacks so outages are visible; update the error handling
around the recentRuns/upcoming return points (referencing recentRuns, upcoming
and task.id) to call the existing telemetry/logger helper (or
Sentry/Telemetry.track) with this context before returning the fallback values.
---
Nitpick comments:
In `@lib/tasks/enrichTasks.ts`:
- Around line 27-84: enrichTasks is doing too many things; split it into smaller
private helpers to follow single-responsibility and size guidelines: extract the
trigger enrichment logic into a helper (e.g., fetchTriggerInfoEntries or
enrichTriggerForTasks) that takes ScheduledAction[] and returns
TriggerInfoEntry[] by moving the Promise.all(tasks.map(...)) block and its inner
logic (fetchTriggerRuns, retrieveTaskRun, payload parsing, and error handling)
out of enrichTasks; extract the account/email logic into a helper (e.g.,
getAccountEmailMap or fetchAccountEmails) that calls selectAccountEmails and
builds the emailByAccountId Map; then have enrichTasks call these helpers and
assemble the final array (or use a small buildEnrichedTasks helper) so each
function (fetchTriggerInfoEntries, getAccountEmailMap, buildEnrichedTasks) is
focused and under the size limits while reusing the existing symbols
(enrichTasks, fetchTriggerRuns, retrieveTaskRun, selectAccountEmails).
In `@lib/tasks/getTasksHandler.ts`:
- Around line 14-47: The getTasksHandler function is too long; extract two
private helpers—buildTasksSuccessResponse(enrichedTasks) and
buildErrorResponse(error)—that each return the corresponding NextResponse.json
payload and headers (reuse getCorsHeaders()), then refactor getTasksHandler to
simply validate the query via validateGetTasksQuery, call selectScheduledActions
and enrichTasks, and return buildTasksSuccessResponse(enrichedTasks) inside the
try block and buildErrorResponse(error) in the catch; keep references to
validateGetTasksQuery, selectScheduledActions, enrichTasks, and getCorsHeaders
to locate where to wire the helpers.
🪄 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: 11e67991-2b9e-44fa-9927-4330ffb8b96a
⛔ Files ignored due to path filters (3)
lib/tasks/__tests__/enrichTaskWithTriggerInfo.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/tasks/__tests__/enrichTasks.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/tasks/__tests__/getTasksHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (3)
lib/tasks/enrichTaskWithTriggerInfo.tslib/tasks/enrichTasks.tslib/tasks/getTasksHandler.ts
💤 Files with no reviewable changes (1)
- lib/tasks/enrichTaskWithTriggerInfo.ts
| } catch { | ||
| // payload retrieval failed — skip upcoming | ||
| } | ||
| } | ||
|
|
||
| return [task.id, { recent_runs: recentRuns, upcoming }] as const; | ||
| } catch { | ||
| // Trigger.dev API failed — return task without trigger enrichment | ||
| return [task.id, { recent_runs: [], upcoming: [] }] as const; |
There was a problem hiding this comment.
Add telemetry for fallback paths in Trigger enrichment.
Both catch blocks silently downgrade data (recent_runs/upcoming) without logging. That makes Trigger outages invisible and hard to debug in production.
Suggested fix
- } catch {
- // payload retrieval failed — skip upcoming
+ } catch (error) {
+ console.warn("Failed to retrieve Trigger run payload", {
+ taskId: task.id,
+ runId: latestRun.id,
+ error: error instanceof Error ? error.message : String(error),
+ });
}
@@
- } catch {
- // Trigger.dev API failed — return task without trigger enrichment
+ } catch (error) {
+ console.warn("Failed to fetch Trigger runs", {
+ taskId: task.id,
+ scheduleId,
+ error: error instanceof Error ? error.message : String(error),
+ });
return [task.id, { recent_runs: [], upcoming: [] }] as const;
}As per coding guidelines, **/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: “Handle errors gracefully”.
📝 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.
| } catch { | |
| // payload retrieval failed — skip upcoming | |
| } | |
| } | |
| return [task.id, { recent_runs: recentRuns, upcoming }] as const; | |
| } catch { | |
| // Trigger.dev API failed — return task without trigger enrichment | |
| return [task.id, { recent_runs: [], upcoming: [] }] as const; | |
| } catch (error) { | |
| console.warn("Failed to retrieve Trigger run payload", { | |
| taskId: task.id, | |
| runId: latestRun.id, | |
| error: error instanceof Error ? error.message : String(error), | |
| }); | |
| } | |
| } | |
| return [task.id, { recent_runs: recentRuns, upcoming }] as const; | |
| } catch (error) { | |
| console.warn("Failed to fetch Trigger runs", { | |
| taskId: task.id, | |
| scheduleId, | |
| error: error instanceof Error ? error.message : String(error), | |
| }); | |
| return [task.id, { recent_runs: [], upcoming: [] }] as const; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/tasks/enrichTasks.ts` around lines 49 - 57, The two empty catch blocks in
lib/tasks/enrichTasks.ts silently drop Trigger enrichment (the payload retrieval
catch and the outer Trigger.dev API catch) — update both to emit telemetry and
contextual logs instead of failing silently: capture and report the thrown error
(message/stack) and include task identifiers (e.g., task.id, any trigger id)
plus which path was taken (payload vs API fallback), and increment a
telemetry/metric counter for Trigger enrichment fallbacks so outages are
visible; update the error handling around the recentRuns/upcoming return points
(referencing recentRuns, upcoming and task.id) to call the existing
telemetry/logger helper (or Sentry/Telemetry.track) with this context before
returning the fallback values.
|
Smoke-tested the latest branch preview:
Verified results:
Example from the live response: {
"status": "success",
"tasks": [
{
"id": "df8a6224-0eb1-49a7-86da-805d77f96b96",
"account_id": "24994fe8-e63c-4c4a-867e-f38976e3fd31",
"owner_email": "arpitgupta1214@gmail.com"
}
]
} |
1 participant
Summary
owner_emailin the defaultGET /api/tasksresponseTesting
Summary by cubic
Add
owner_emailto the tasks API response and consolidate enrichment intoenrichTasksto batch Trigger metadata and email lookups. UpdatesgetTasksHandlerand adds tests.New Features
owner_email(string or null) fromaccount_emails.recent_runsandupcomingper task.Refactors
enrichTaskWithTriggerInfowithenrichTasksand updatedgetTasksHandlerto use it.enrichTasks.test.ts,getTasksHandler.test.ts; removedenrichTaskWithTriggerInfoand its test.Written for commit 650512b. Summary will update on new commits.
Summary by CodeRabbit