refactor: task helper and tons of other stuff#4479
Merged
Conversation
- Implement shared utility functions for task handling in `shared.ts` - Create sorting logic for task sort keys in `sort-keys.ts` - Develop basic task hooks for creating, updating, and deleting tasks in `task-hooks-basic.ts` - Add functionality for moving tasks between lists and boards in `task-hooks-move.ts` - Introduce task operations for creating, updating, and moving tasks in `task-operations.ts`
…prove task list operations
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPropagates workspace-first (wsId) semantics across task flows, adds many workspace-scoped task-helper modules (operations, relationships, bulk actions, recycle bin, sort-keys), creates a task-board-status-templates API route, extends tasks GET options, updates internal-api exports, and wires wsId through numerous UI hooks and components. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (UI)
participant Route as /workspaces/:wsId/task-drafts/:draftId/convert (API)
participant Auth as Supabase Auth
participant Admin as Supabase Admin / Internal API
participant DB as Database / workspace APIs
Client->>Route: POST convert draft
Note right of Route: createClient(request) & normalizeWorkspaceId
Route->>Auth: supabase.auth.getUser()
Auth-->>Route: user (or 401)
Route->>Admin: delete draft (claim) -> delete().select('*').maybeSingle()
Admin-->>Route: claimed draft (or 404/500)
Route->>Admin: createTask(wsId, listId, payload with assignee_ids,label_ids,project_ids)
Admin->>DB: createWorkspaceTask + relations
DB-->>Admin: created task
Route->>Admin: (if failure) restoreDraft(sbAdmin, draft)
Route-->>Client: 200 { task, ... } or appropriate error
Note: diagram rectangles use default styling; interactions show the claim-then-create-then-restore flow. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
Contributor
🌐 i18n Check Report✅ All i18n checks passed!
🤖 Auto-generated by i18n Check workflow • Last updated: 3/20/2026, 8:09:12 PM
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4479 +/- ##
==========================================
+ Coverage 47.26% 47.35% +0.09%
==========================================
Files 501 509 +8
Lines 34318 34221 -97
Branches 12315 12336 +21
==========================================
- Hits 16220 16207 -13
+ Misses 14763 14660 -103
- Partials 3335 3354 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 36 files
Confidence score: 1/5
- Critical merge-blocker:
apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.tsappears to remove the RLS authorization boundary by using an admin client without equivalent membership/ownership checks, which can let any authenticated user read or mutate other users’ task embeddings. - There are multiple additional user-facing correctness risks in task moves and recovery flows (wrong identifier in
packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts, stale UI after partial failures inpackages/utils/src/task-helper/bulk-actions.ts, and ignored restore/delete failures inpackages/utils/src/task-helper/recycle-bin.ts). - Given the high severity and high confidence of the authorization issue (9/10, 9/10), this is not safe to merge until safeguards are restored; the remaining issues are important but secondary.
- Pay close attention to
apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts,packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts,packages/utils/src/task-helper/bulk-actions.ts,packages/utils/src/task-helper/recycle-bin.ts- authorization enforcement, identifier correctness, and rollback/invalidation consistency.
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="packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts">
<violation number="1" location="packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts:475">
P1: moveTaskToBoard expects a workspace ID; passing boardId here will make the move API call use the wrong identifier.</violation>
</file>
<file name="packages/utils/src/task-helper/task-hooks-move.ts">
<violation number="1" location="packages/utils/src/task-helper/task-hooks-move.ts:45">
P2: Use the same `task_lists` query key as the rest of the codebase; `task-lists` causes a cache miss and prevents correct list status/archiving during optimistic updates.</violation>
</file>
<file name="packages/utils/src/task-helper/bulk-actions.ts">
<violation number="1" location="packages/utils/src/task-helper/bulk-actions.ts:141">
P1: After a partial move failure, `onError` rolls back the optimistic update but never invalidates queries. Tasks that were successfully moved server-side will appear stuck in their original list until the user manually triggers a refetch. Add `queryClient.invalidateQueries` in `onError` (or add an `onSettled` handler) so the cache re-syncs with the server after partial failures.</violation>
</file>
<file name="packages/utils/src/task-helper/board.ts">
<violation number="1" location="packages/utils/src/task-helper/board.ts:175">
P2: Handle a null board response before dereferencing. `payload.board` can be null (see getTaskBoard), so accessing `board.id` will throw when the board is missing.</violation>
</file>
<file name="packages/utils/src/task-helper/task-hooks-basic.ts">
<violation number="1" location="packages/utils/src/task-helper/task-hooks-basic.ts:186">
P2: Spreading `task` after the optimistic fields lets caller-provided `id`/`list_id` override the generated optimistic values, which can prevent the optimistic row from being replaced on success. Spread `task` first (or set the fixed fields after the spread) so the optimistic id/list_id always win.</violation>
</file>
<file name="apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts">
<violation number="1" location="apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts:41">
P0: Switching to the admin client without adding an explicit workspace-membership or task-ownership check removes the RLS authorization boundary. Any authenticated user can now read and mutate any task's embedding regardless of workspace membership.
If the admin client is needed (e.g., to write to a column the user can't update directly), add an explicit authorization gate — for example, verify the user belongs to `wsId` and that the task belongs to that workspace before proceeding.</violation>
</file>
<file name="packages/utils/src/task-helper/recycle-bin.ts">
<violation number="1" location="packages/utils/src/task-helper/recycle-bin.ts:63">
P2: The restore loop ignores update failures when no fallbackListId is provided, so failed restores disappear from the deleted-tasks cache without being restored. Throw on failure to trigger onError rollback instead of continuing silently.</violation>
<violation number="2" location="packages/utils/src/task-helper/recycle-bin.ts:155">
P2: Permanent delete swallows per-task failures, so the mutation succeeds even when deletions fail and the deleted-tasks cache stays pruned. Re-throw to trigger rollback (or otherwise surface the failure).</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Task UI (Web App)
participant Utils as Modular Task Helpers
participant IntAPI as Internal API Client
participant Server as Next.js API Routes
participant DB as Supabase DB
participant AI as AI Embedding Service
Note over UI,AI: CHANGED: All task operations now strictly scoped by Workspace ID (wsId)
UI->>Utils: useBoardConfig(boardId, wsId)
Utils->>IntAPI: listWorkspaceBoardsWithLists(wsId)
IntAPI->>Server: GET /api/v1/workspaces/[wsId]/boards
Server->>DB: Fetch board configuration
DB-->>UI: Return board metadata & status templates
Note over UI,AI: NEW: Task Creation with Automated Embedding
UI->>Utils: CHANGED: createTask(wsId, listId, data)
Utils->>IntAPI: createWorkspaceTask(wsId, data)
IntAPI->>Server: POST /api/v1/workspaces/[wsId]/tasks
Server->>DB: Insert task record
Server-->>IntAPI: Task created
opt NEW: Side effect (Client-side trigger)
Utils->>IntAPI: NEW: triggerWorkspaceTaskEmbedding(wsId, taskId)
IntAPI->>Server: POST /api/v1/workspaces/[wsId]/tasks/[taskId]/embedding
Server->>Server: NEW: createAdminClient()
Server->>AI: AI SDK: embed(task name + description)
AI-->>Server: Vector data
Server->>DB: Update task with embedding string
end
IntAPI-->>UI: Return final Task object
Note over UI,AI: Task Movement & Sort Key Management
UI->>Utils: useKanbanDnd (Drag and Drop)
Utils->>Utils: calculateSortKeyWithRetry(wsId, prev, next)
alt Gap Exhausted or Collision
Utils->>IntAPI: NEW: normalizeListSortKeys(wsId, listId)
IntAPI->>Server: Update multiple tasks (bulk sort_key update)
Server->>DB: Re-index list positions
Utils->>Utils: Recalculate sort key
end
Utils->>IntAPI: CHANGED: moveWorkspaceTask(wsId, taskId, targetData)
IntAPI->>Server: POST /api/v1/workspaces/[wsId]/tasks/[taskId]/move
Server->>DB: Update list_id and sort_key
IntAPI-->>UI: Broadcast 'task:upsert' via Realtime
Note over UI,AI: Recycle Bin Flow
UI->>Utils: NEW: useDeletedTasks(boardId, wsId)
Utils->>IntAPI: listWorkspaceTasks(wsId, { includeDeleted: 'only' })
IntAPI->>Server: GET /api/v1/workspaces/[wsId]/tasks?includeDeleted=only
Server->>DB: Fetch tasks where deleted_at IS NOT NULL
DB-->>UI: Display deleted tasks in Recycle Bin Panel
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts
Show resolved
Hide resolved
packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts (2)
29-30:⚠️ Potential issue | 🟡 MinorMissing request parameter for mobile Bearer token support.
createClient()should becreateClient(request)to support mobile clients using Bearer token authentication.🔧 Proposed fix
-export async function POST(_: Request, { params }: Params) { +export async function POST(request: Request, { params }: Params) { try { // Check if API key is available const hasApiKey = !!process.env.GOOGLE_GENERATIVE_AI_API_KEY; ... - const supabase = await createClient(); + const supabase = await createClient(request);As per coding guidelines: "Use
createClient(request)in web API routes to support mobile Bearer token auth".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/v1/workspaces/`[wsId]/tasks/[taskId]/embedding/route.ts around lines 29 - 30, The Supabase client is created without the incoming request so mobile Bearer token auth fails; update the call in the route handler to pass the incoming request into createClient (replace createClient() with createClient(request)) so the supabase instance created for this endpoint (variable supabase) can pick up Bearer tokens; ensure this change is applied where params and taskId are used in apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts.
41-48:⚠️ Potential issue | 🟠 MajorMissing workspace scope verification on task access.
The route extracts
wsIdfrom params but never uses it. The task fetch and update operations don't verify that the task belongs to the specified workspace, allowing any authenticated user to potentially modify embeddings for tasks in workspaces they don't have access to.🛡️ Proposed fix to add workspace scoping
const sbAdmin = await createAdminClient(); + const { wsId, taskId } = await params; + + // Verify workspace membership + const { data: membership, error: membershipError } = await supabase + .from('workspace_members') + .select('ws_id') + .eq('ws_id', wsId) + .eq('user_id', user.id) + .maybeSingle(); + + if (membershipError) { + console.error('Membership lookup failed:', membershipError); + return NextResponse.json({ message: 'Internal server error' }, { status: 500 }); + } + + if (!membership) { + return NextResponse.json({ message: 'Forbidden' }, { status: 403 }); + } + // Fetch the task const { data: task, error: fetchError } = await sbAdmin .from('tasks') .select('id, name, description') .eq('id', taskId) + .eq('list_id', /* join through task_lists to verify board belongs to wsId */) .single();Note: You may need to join through
task_lists→workspace_boardsto verify the task belongs to the workspace, or addws_idcheck if tasks have it directly.As per coding guidelines: "Keep read-side verification on
sbAdmin, but always scope those reads with explicit workspace/resource checks before mutating".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/v1/workspaces/`[wsId]/tasks/[taskId]/embedding/route.ts around lines 41 - 48, The task query and subsequent embedding update must be scoped to the workspace: when using sbAdmin (createAdminClient) to fetch the task (the .from('tasks').select(...).eq('id', taskId).single() call), add an explicit workspace ownership check using wsId—either by joining through task_lists → workspace_boards to ensure the task's task_list belongs to the workspace or by adding an explicit ws_id equality if tasks store it directly; only proceed with embedding changes if that scoped query returns the task (otherwise return an authorization/not found error). Ensure both the initial fetch and any update paths reference this scoped query (symbols: sbAdmin, taskId, wsId, task_lists, workspace_boards) so no mutation occurs without workspace verification.apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.ts (2)
39-48:⚠️ Potential issue | 🟡 MinorMembership lookup error handling is incomplete.
The membership check only verifies
!membershipbut does not handle the queryerrorexplicitly. If the membership lookup fails for a database reason, the route returns 403 instead of 500, which misrepresents the error.🛡️ Proposed fix
- const { data: membership } = await supabase + const { data: membership, error: membershipError } = await supabase .from('workspace_members') .select('ws_id') .eq('ws_id', wsId) .eq('user_id', user.id) .single(); + if (membershipError) { + console.error('Membership lookup failed:', membershipError); + return NextResponse.json({ error: 'Internal server error' }, { status: 500 }); + } + if (!membership) { return NextResponse.json({ error: 'Forbidden' }, { status: 403 }); }As per coding guidelines: "In API routes gating access with
workspace_memberslookups, handle queryerrorexplicitly and return500for membership lookup failures".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/v1/workspaces/`[wsId]/task-drafts/[draftId]/convert/route.ts around lines 39 - 48, The membership lookup currently only checks for a missing membership and returns 403, but doesn't handle DB errors; update the supabase call in route.ts to destructure the returned error (e.g., const { data: membership, error } = await supabase.from('workspace_members')...), check if error is set first and return NextResponse.json({ error: 'Internal Server Error' }, { status: 500 }) (and optionally log the error) before checking for !membership and returning 403 to correctly differentiate DB failures from unauthorized access.
27-28:⚠️ Potential issue | 🟠 MajorMissing
normalizeWorkspaceIdand request-based client initialization.The route extracts
wsIddirectly from params without normalization, which breaks 'personal' workspace handling. Additionally,createClient()should becreateClient(request)to support mobile Bearer token auth.🔧 Proposed fix
+import { normalizeWorkspaceId } from '@tuturuuu/utils/id'; // or appropriate path export async function POST( request: NextRequest, { params }: { params: Promise<{ wsId: string; draftId: string }> } ) { try { - const { wsId, draftId } = await params; - const supabase = await createClient(); + const { wsId: rawWsId, draftId } = await params; + const supabase = await createClient(request); const sbAdmin = await createAdminClient(); + + const wsId = await normalizeWorkspaceId(rawWsId, supabase);As per coding guidelines: "Use
normalizeWorkspaceId(wsId)in API routes to handle 'personal' vs UUID workspace identifiers" and "UsecreateClient(request)in web API routes to support mobile Bearer token auth".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/v1/workspaces/`[wsId]/task-drafts/[draftId]/convert/route.ts around lines 27 - 28, The route currently pulls wsId from params and calls createClient() directly; fix by normalizing the workspace id and creating the Supabase client from the incoming request: call normalizeWorkspaceId on the extracted wsId (so 'personal' is handled) and pass request into createClient (i.e., createClient(request)) when initializing supabase; also ensure normalizeWorkspaceId is imported and keep draftId from params as before.packages/ui/src/components/ui/tu-do/boards/boardId/task.tsx (1)
524-585:⚠️ Potential issue | 🟠 MajorMake the duplicate flow atomic.
After
createTask(...)succeeds, label/assignee/project cloning is still best-effort client-side work that only logs failures. That means this path can cache and toast a “successful” duplicate that is missing some relations. Move the whole clone behind one internal API helper, or roll back the new task on the first relation insert failure. As per coding guidelines, "Treatpackages/internal-apias the canonical client boundary for app APIs".Also applies to: 595-624
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/tu-do/boards/boardId/task.tsx` around lines 524 - 585, The duplication flow is non-atomic: createTask(...) returns before label/assignee/project inserts (supabase.from('task_labels'), 'task_assignees', 'task_project_tasks') run and their failures are only logged; fix by making the whole clone atomic — either call a single internal-api endpoint in packages/internal-api that creates the task and all relations inside a DB transaction, or if keeping client-side relation inserts, on the first insert error for any of the relations delete/rollback the newly created task (use the same identifier returned by createTask) and surface the error instead of logging; apply the same change for the second block handling tasksToDuplicate (lines around 595-624) to ensure consistent rollback behavior.packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts (1)
474-479:⚠️ Potential issue | 🔴 CriticalReconcile only successful cross-board moves.
onMutateremoves every requested task from the source cache, butonSuccessstill treats the originaltaskIdsas all-success and broadcaststask:deletefor failures too. In a partial success, failed tasks vanish locally and on other clients even though they never moved. Also,if (!old) return [data.movedTasks]seeds['tasks', targetBoardId]withTask[][]instead ofTask[]. Keep a separate success id list, restore failures frompreviousTasks, and returndata.movedTasksdirectly for an empty target cache.Also applies to: 494-500, 503-552, 568-599
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts` around lines 474 - 479, onMutate currently removes all requested taskIds from the source cache and onSuccess assumes every id moved, causing failed cross-board moves to disappear and incorrect broadcasting; modify the mutation flow in the bulk move handlers (the onMutate/onSuccess around moveTaskToBoard) to track a separate list of actually successful ids returned from the server (e.g., data.movedTasks ids) instead of using the original taskIds, restore any failed tasks from previousTasks back into the source cache (previousTasks captured in onMutate), only broadcast 'task:delete' for the successful ids, and when seeding or updating the target cache (['tasks', targetBoardId]) return data.movedTasks directly (ensuring it remains a Task[] not Task[][]) when the target cache was empty; apply the same fix pattern for the other affected handlers referenced in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/apis/src/tu-do/tasks/route.ts`:
- Around line 167-174: The query currently always applies the
"task_lists.deleted = false" filter which hides tasks whose original list was
archived even when includeDeletedMode is 'only' or 'all'; update the
query-building logic that uses includeDeletedParam/includeDeletedMode so that
when includeDeletedMode === 'only' (and for 'all' if appropriate) you relax or
remove the unconditional task_lists.deleted = false predicate (the filter
referencing task_lists.deleted) so archived lists' deleted tasks are included in
the recycle-bin results; apply the same change to the second occurrence of this
logic noted around the other block (the other query at the later section).
In
`@packages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.ts`:
- Around line 19-21: The UseKanbanDndProps interface and the useKanbanDnd hook
currently treat wsId as required causing workspace-scoped reorder/sort-key calls
to run with undefined; change wsId in the UseKanbanDndProps signature to be
optional (wsId?: string | null) and update useKanbanDnd to early-return/no-op
(disable DnD handlers and side-effects) whenever wsId is falsy before running
normalization, reorder, or workspace-scoped API calls (references:
UseKanbanDndProps, wsId, boardId, and the useKanbanDnd hook code paths around
the reorder/normalize logic).
In `@packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx`:
- Around line 205-207: Replace the hard-coded success message passed to
toast.success in list-actions.tsx with a localized string key (e.g., use the
project's i18n translator function such as t(...) or useTranslation().t) and
format it with movedTasks.length to preserve pluralization; update both en.json
and vi.json to include the new key and appropriate plural/variable variants so
English and Vietnamese users see translated messages. Locate the toast.success
call and the movedTasks variable in the same file (list-actions.tsx) and ensure
the translation key supports pluralization or interpolation for the count.
- Around line 181-188: The loop that calls moveTaskMutation.mutateAsync for each
task (using variables movedTasks, tasks, closedListId) commits each change
immediately so a later failure leaves earlier tasks moved while the outer
mutation reports failure; replace the per-task immediate commits with an atomic
batch move: add or use a server-side batch mutation (e.g., moveTasksBatch or
moveMultipleTasks) that accepts an array of taskIds and a newListId and call
that once instead of per-task mutateAsync, or if a server batch cannot be added,
run all moves via Promise.allSettled and then perform a single authoritative
refetch (and surface partial success) rather than aborting after the first
error; update the outer mutation handler to call the batch endpoint (or handle
settled results) and always trigger a refetch/invalidate to ensure UI reflects
the authoritative state.
In `@packages/ui/src/components/ui/tu-do/shared/board-views.tsx`:
- Line 607: The prop passed to RecycleBinPanel uses board.ws_id directly which
can be null/undefined; change the prop to use the same fallback pattern as
elsewhere (board.ws_id ?? workspace.id) so RecycleBinPanel(wsId) always receives
a string; update the JSX prop where wsId={board.ws_id} to wsId={board.ws_id ??
workspace.id} and ensure types remain consistent with the RecycleBinPanel
signature.
In `@packages/utils/src/task-helper/board.ts`:
- Around line 174-184: The code dereferences payload.board without a null check;
update the function that calls getWorkspaceTaskBoardFromApi (the block using
payload and board) to guard payload.board and return null (or an appropriate
nullable value) when payload.board is missing or falsy instead of accessing
board.id; ensure you check payload.board before building/casting the BoardConfig
and only construct/return the BoardConfig when payload.board exists (otherwise
return null) to match getTaskBoard()'s nullable handling.
- Around line 87-89: In useCreateBoardWithTemplate
(packages/utils/src/task-helper/board.ts) the onSuccess callback calls
queryClient.invalidateQueries with the non-canonical key ['workspace-boards',
wsId]; change this to use the canonical board-list key ['boards', wsId] so it
matches the update mutation (the other invalidate at/near the update mutation
around the same file) and prevents stale lists—replace the ['workspace-boards',
wsId] invalidateQueries call in the onSuccess handler with ['boards', wsId].
In `@packages/utils/src/task-helper/bulk-actions.ts`:
- Around line 42-68: The loop using moveWorkspaceTask over tasksToMove can
produce partial success and leave client cache desynced because onError rolls
back a snapshot; change this to either call a bulk-move API (e.g., implement and
use moveWorkspaceTasksBulk(wsId, tasksPayload, getBrowserApiOptions)) so the
server performs atomic moves, or keep the per-task calls but remove the
all-or-nothing rollback: on any partial failure compute movedTaskIds from
results and trigger an authoritative refetch of the affected lists/boards (e.g.,
refetch source list, target list, and workspace tasks) instead of restoring the
snapshot; update usages of results, movedTaskIds, moveWorkspaceTask, tasksToMove
and onError accordingly.
In `@packages/utils/src/task-helper/recycle-bin.ts`:
- Around line 148-160: mutationFn currently swallows per-task errors causing
failed deletes to be removed from the local recycle-bin cache; update the
mutationFn in recycle-bin.ts to track failed task IDs by pushing taskId into a
failedIds array when deleteWorkspaceTask(wsId, taskId, getBrowserApiOptions())
throws, and after the loop if failedIds.length > 0 throw a new Error (or reject)
that includes the failedIds (or their JSON) so the caller's onError/onSettled
can restore those IDs back into ['deleted-tasks', boardId]; otherwise return {
count } as before for full success.
- Around line 53-79: Before attempting restores, capture the pre-mutation task
objects (the ones removed in onMutate) and, inside the loop that calls
updateWorkspaceTask, ensure that any failed restore—whether caught in the
fallback branch or when fallbackListId is absent—is pushed back into
restoredTasks (or a failedRestores array that you merge into restoredTasks
before return) so onSuccess will re-add them to ['tasks', boardId];
specifically, preserve originals for each taskId and in the catch branch where
you currently just continue, push the preserved original task for that taskId
into restoredTasks (use the same task object shape returned by
updateWorkspaceTask) so failed restores are not dropped from the cache.
In `@packages/utils/src/task-helper/relationships.ts`:
- Around line 74-98: The function deleteTaskRelationship currently accepts
relationshipId but never uses it, which is misleading and unsafe; either (A) use
relationshipId to first fetch the relationship details (e.g., call a helper like
getWorkspaceTaskRelationshipById or equivalent) and then pass the fetched
source_task_id/target_task_id/type into deleteWorkspaceTaskRelationship to
ensure you delete the intended edge, or (B) remove the unused relationshipId
parameter from deleteTaskRelationship and update the error message and any
callers to only pass the relationship object (source_task_id, target_task_id,
type); pick one approach, update the function signature and the thrown error
text accordingly, and adjust all callers to match.
- Line 121: The _boardId parameter in useCreateTaskRelationship (and the second
occurrence at the other signature) is unused; remove the dead _boardId argument
from the public signatures (delete the parameter from useCreateTaskRelationship
and the other function, then update all call sites and exported type signatures)
OR if backward compatibility requires keeping it, make the contract explicit by
adding a JSDoc comment marking _boardId as `@deprecated/unused` and keep it only
to preserve call-compatibility (and add a unit-test or code comment stating when
it can be removed); ensure you update function declarations, exported types, and
any callers accordingly.
- Around line 54-68: The catch block in createTaskRelationship currently throws
hard-coded English messages and is bypassed by useCreateTaskRelationship calling
the raw API; change createTaskRelationship to rethrow normalized errors with
stable machine-readable codes (e.g., code: 'REL_DUPLICATE' /
'REL_MULTIPLE_PARENT' / 'REL_CIRCULAR') instead of plain English, update
useCreateTaskRelationship to call createTaskRelationship (not the raw internal
API) so consumers receive those coded errors, and add corresponding entries for
each user-facing message in both en.json and vi.json so the UI can localize
based on the error code.
In `@packages/utils/src/task-helper/sort-keys.ts`:
- Around line 262-273: The current code fires one updateWorkspaceTask per task
in a single Promise.all which can overwhelm the API; change this to cap
concurrency when sending sort_key updates by processing updates in bounded
parallelism (e.g., use a p-limit with a configurable concurrency like 5, or
split updates into chunks and await each chunk sequentially) while still using
getMutationApiOptions() and calling updateWorkspaceTask(wsId, update.id, {
sort_key: update.sort_key }, options); ensure you await and propagate errors
per-update (or collect and surface failures) instead of letting a large
unbounded Promise.all leave the list half-renumbered.
- Around line 18-25: The comparator currently treats null/undefined priority as
weight 5 which, combined with the descending sort (return valueB - valueA),
places unprioritized tasks first; update getOrderValue to return the lowest
weight (e.g., 0) for missing priority instead of 5 so missing priorities rank
lowest: change the default branch in getOrderValue (used for priorityA and
priorityB and referencing priorityOrder) to return 0 (or the numeric minimum of
your priorityOrder scale) so the sort order correctly ranks
critical/high/normal/low above unprioritized tasks.
In `@packages/utils/src/task-helper/task-hooks-basic.ts`:
- Around line 123-164: The mutation accepts description_yjs_state but never
forwards it to the API; update the mutationFn payload passed to
createWorkspaceTask to include description_yjs_state: take description_yjs_state
from the incoming task parameter (the async ({ listId, task }) handler) and add
description_yjs_state: task.description_yjs_state ?? null (or appropriate
default) to the object passed to createWorkspaceTask so the Yjs document state
is persisted.
- Around line 38-55: The code only fetches blockedTaskIdsPromise when
updates.completed_at/closed_at are non-null, which misses cases where those
fields are set back to null (reopening); change the conditional so
blockedTaskIdsPromise is created whenever updates has completed_at or closed_at
defined (i.e., checks for !== undefined rather than !== null), keeping the wsId
guard, and still call getWorkspaceTaskRelationships(wsId, taskId,
getBrowserApiOptions()) inside that branch (handle errors as before) so
blocked-task invalidation runs on both completion and reopening paths; apply the
same fix to the other occurrence around lines 95-114.
In `@packages/utils/src/task-helper/task-hooks-move.ts`:
- Around line 44-47: The optimistic move path is using the wrong cache key
string: change the key passed to queryClient.getQueryData in task-hooks-move.ts
(the call that assigns targetList) from ['task-lists', boardId] to the populated
key ['task_lists', boardId] so the lookup hits the same cache used by the other
task helpers and allows shouldArchive to become true; update any analogous
places in the same file that use 'task-lists' to use 'task_lists' instead.
In `@packages/utils/src/task-helper/task-operations.ts`:
- Around line 149-184: The new syncTaskArchivedStatus(...) function isn't
invoked for same-board moves, so moveTask(...) (and the useMoveTask path that
calls it) only updates list_id and can leave closed_at stale; after the code
that updates a task's list_id in moveTask (the call that ultimately calls
updateWorkspaceTask with the new list_id) add an awaited call to
syncTaskArchivedStatus(wsId, taskId, listId) using the same wsId/taskId/listId
values (and await it or propagate the promise) so that closed_at is reconciled
after same-board moves; ensure both the synchronous moveTask implementation and
the useMoveTask mutation success handler invoke syncTaskArchivedStatus to cover
optimistic updates.
---
Outside diff comments:
In
`@apps/web/src/app/api/v1/workspaces/`[wsId]/task-drafts/[draftId]/convert/route.ts:
- Around line 39-48: The membership lookup currently only checks for a missing
membership and returns 403, but doesn't handle DB errors; update the supabase
call in route.ts to destructure the returned error (e.g., const { data:
membership, error } = await supabase.from('workspace_members')...), check if
error is set first and return NextResponse.json({ error: 'Internal Server Error'
}, { status: 500 }) (and optionally log the error) before checking for
!membership and returning 403 to correctly differentiate DB failures from
unauthorized access.
- Around line 27-28: The route currently pulls wsId from params and calls
createClient() directly; fix by normalizing the workspace id and creating the
Supabase client from the incoming request: call normalizeWorkspaceId on the
extracted wsId (so 'personal' is handled) and pass request into createClient
(i.e., createClient(request)) when initializing supabase; also ensure
normalizeWorkspaceId is imported and keep draftId from params as before.
In `@apps/web/src/app/api/v1/workspaces/`[wsId]/tasks/[taskId]/embedding/route.ts:
- Around line 29-30: The Supabase client is created without the incoming request
so mobile Bearer token auth fails; update the call in the route handler to pass
the incoming request into createClient (replace createClient() with
createClient(request)) so the supabase instance created for this endpoint
(variable supabase) can pick up Bearer tokens; ensure this change is applied
where params and taskId are used in
apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts.
- Around line 41-48: The task query and subsequent embedding update must be
scoped to the workspace: when using sbAdmin (createAdminClient) to fetch the
task (the .from('tasks').select(...).eq('id', taskId).single() call), add an
explicit workspace ownership check using wsId—either by joining through
task_lists → workspace_boards to ensure the task's task_list belongs to the
workspace or by adding an explicit ws_id equality if tasks store it directly;
only proceed with embedding changes if that scoped query returns the task
(otherwise return an authorization/not found error). Ensure both the initial
fetch and any update paths reference this scoped query (symbols: sbAdmin,
taskId, wsId, task_lists, workspace_boards) so no mutation occurs without
workspace verification.
In
`@packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts`:
- Around line 474-479: onMutate currently removes all requested taskIds from the
source cache and onSuccess assumes every id moved, causing failed cross-board
moves to disappear and incorrect broadcasting; modify the mutation flow in the
bulk move handlers (the onMutate/onSuccess around moveTaskToBoard) to track a
separate list of actually successful ids returned from the server (e.g.,
data.movedTasks ids) instead of using the original taskIds, restore any failed
tasks from previousTasks back into the source cache (previousTasks captured in
onMutate), only broadcast 'task:delete' for the successful ids, and when seeding
or updating the target cache (['tasks', targetBoardId]) return data.movedTasks
directly (ensuring it remains a Task[] not Task[][]) when the target cache was
empty; apply the same fix pattern for the other affected handlers referenced in
the comment.
In `@packages/ui/src/components/ui/tu-do/boards/boardId/task.tsx`:
- Around line 524-585: The duplication flow is non-atomic: createTask(...)
returns before label/assignee/project inserts (supabase.from('task_labels'),
'task_assignees', 'task_project_tasks') run and their failures are only logged;
fix by making the whole clone atomic — either call a single internal-api
endpoint in packages/internal-api that creates the task and all relations inside
a DB transaction, or if keeping client-side relation inserts, on the first
insert error for any of the relations delete/rollback the newly created task
(use the same identifier returned by createTask) and surface the error instead
of logging; apply the same change for the second block handling tasksToDuplicate
(lines around 595-624) to ensure consistent rollback behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81c475cc-4aff-4e02-b039-ebd9caab07e6
📒 Files selected for processing (36)
apps/web/src/app/api/v1/task-board-status-templates/route.tsapps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.tsapps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.tsapps/web/src/components/command/add-task-form.tsxpackages/apis/src/tu-do/tasks/route.tspackages/internal-api/src/index.tspackages/internal-api/src/tasks.tspackages/ui/src/components/ui/calendar-app/components/quick-task-dialog.tsxpackages/ui/src/components/ui/text-editor/task-mention-chip.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/kanban.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.tspackages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/kanban-sort-helpers.tspackages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.tspackages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/task-actions.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/task-form.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/task.tsxpackages/ui/src/components/ui/tu-do/my-tasks/__tests__/use-my-tasks-state.test.tspackages/ui/src/components/ui/tu-do/my-tasks/use-my-tasks-state.tspackages/ui/src/components/ui/tu-do/shared/board-views.tsxpackages/ui/src/components/ui/tu-do/shared/recycle-bin-panel.tsxpackages/ui/src/components/ui/tu-do/shared/task-edit-dialog/hooks/use-task-data.tspackages/ui/src/components/ui/tu-do/shared/task-edit-dialog/hooks/use-task-save.tspackages/utils/src/__tests__/task-helper-board-api-routing.test.tspackages/utils/src/__tests__/task-helper-create-task.test.tspackages/utils/src/task-helper.tspackages/utils/src/task-helper/board.tspackages/utils/src/task-helper/bulk-actions.tspackages/utils/src/task-helper/recycle-bin.tspackages/utils/src/task-helper/relationships.tspackages/utils/src/task-helper/shared.tspackages/utils/src/task-helper/sort-keys.tspackages/utils/src/task-helper/task-hooks-basic.tspackages/utils/src/task-helper/task-hooks-move.tspackages/utils/src/task-helper/task-operations.tspackages/utils/src/task/reorder.ts
packages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.ts
Show resolved
Hide resolved
packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx
Outdated
Show resolved
Hide resolved
Contributor
🔧 Biome Check ReportTotal Issues Found: 0🔧 Format Check Results ✅
🔍 Lint Check Results ✅
🎉 All Issues Resolved!Your code is now 100% clean! Great job! 🏆
🤖 Auto-generated by Biome Check workflow • Last updated: 3/20/2026, 8:09:12 PM
|
Contributor
There was a problem hiding this comment.
1 issue found across 28 files (changes from recent commits).
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="packages/utils/src/task-helper/bulk-actions.ts">
<violation number="1" location="packages/utils/src/task-helper/bulk-actions.ts:79">
P2: Partial task-move failures are now swallowed as success, so callers that rely on mutation rejection no longer handle incomplete moves.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts (2)
33-40:⚠️ Potential issue | 🟠 MajorAuthenticate and validate IDs before
normalizeWorkspaceId.
normalizeWorkspaceId(rawWsId, supabase)runs beforeauth.getUser(), andtaskIdis sent straight into a UUID filter with no shared schema check. Unauthenticated/personal/...requests and malformed task IDs can therefore surface as500from normalization/PostgREST instead of the expected401/400.As per coding guidelines, "In request handlers that call
normalizeWorkspaceId(...), authenticate before normalization when possible" and "In API routes, validate UUID path params with sharedzodschemas (z.guid()+safeParse) instead of ad-hoc checks."Also applies to: 49-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/v1/workspaces/`[wsId]/tasks/[taskId]/embedding/route.ts around lines 33 - 40, Authenticate before calling normalizeWorkspaceId by moving the supabase.auth.getUser() call (and its user check) to precede normalizeWorkspaceId(rawWsId, supabase); then validate the path param taskId using the shared zod UUID schema (e.g., call sharedSchema.guid().safeParse(taskId) or the project’s existing zod validator) and return a 400 on parse failure before using taskId in any DB/PostgREST calls; update handling in this file’s route (and the similar block at lines 49–65) to reject unauthenticated requests with 401 and malformed task IDs with 400 rather than letting normalizeWorkspaceId or PostgREST surface a 500.
33-65:⚠️ Potential issue | 🔴 CriticalVerify workspace membership before the admin read/write.
This handler only authenticates the caller. The
sbAdminread/update proves the task belongs towsId, but never proves the caller belongs to that workspace, so any signed-in user who learns a task UUID can regenerate embeddings for another workspace’s task. Checkworkspace_memberswith the user-scoped client first, return500on membership lookup failure and403when no membership exists, then usesbAdminfor the mutation.As per coding guidelines, "If a route must write with
createAdminClientbecause RLS can block the mutation, fetch the caller withcreateClient(request)first..." and "In API routes that gate access withworkspace_memberslookups, always handle queryerrorexplicitly and return500for membership lookup failures; return403only when no membership exists."Also applies to: 110-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/v1/workspaces/`[wsId]/tasks/[taskId]/embedding/route.ts around lines 33 - 65, Before using the admin client to read/update the task, enforce workspace membership with the user-scoped client: use the supabase instance returned by createClient(request) (after normalizeWorkspaceId) to query the workspace_members table for the current user and wsId, explicitly check the query error and return a 500 on lookup failure, return a 403 if no membership row exists, and only then call createAdminClient()/sbAdmin to fetch/update the tasks; update the handler around the normalizeWorkspaceId/supabase usage and the sbAdmin usage (references: createClient(request), normalizeWorkspaceId, supabase, workspace_members, createAdminClient, sbAdmin) to implement this control flow.packages/ui/src/components/ui/tu-do/shared/list-view.tsx (2)
153-165:⚠️ Potential issue | 🟡 MinorClear the selection when the workspace switches.
selectedTaskssurvives workspace changes, but the bulk hook now mutates against the currentworkspaceId. A reusedListViewcan carry old task IDs into the new workspace scope and keep showing a stale bulk-action bar until the user clears it manually.Based on learnings, "In multi-tab mobile or web views backed by workspace-scoped data, reset all tab datasets and invalidate in-flight request tokens when the workspace changes."Suggested fix
+ useEffect(() => { + setSelectedTasks(new Set()); + }, [workspaceId]);Also applies to: 217-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/tu-do/shared/list-view.tsx` around lines 153 - 165, When workspaceId changes, reset the bulk selection and cancel any in-flight workspace-scoped requests so stale task IDs don't persist across workspaces: add an effect in ListView that watches workspaceId and calls the selection reset handler (clearSelection or setSelectedTasks([])) and uses the queryClient to cancel or invalidate relevant queries (e.g., queryClient.cancelQueries / queryClient.invalidateQueries for workspace-scoped keys) and notify useBulkOperations of the reset so bulkUpdateDueDate / bulkUpdatePriority won't operate on old IDs.
153-165:⚠️ Potential issue | 🟠 MajorFinish routing bulk delete through the workspace-scoped API layer.
This change makes due-date/priority mutations workspace-aware, but
handleBulkDeleteConfirmedstill deletes throughcreateClient().from('tasks').delete(). That leaves one bulk action outside the internal-api migration and will break as soon as task CRUD depends on the app API/proxy-only permission path.As per coding guidelines, "When app code needs authenticated workspace/user CRUD, add or extend the route and consume it through
packages/internal-api/src/*instead of scattering rawfetch('/api/...')calls."Also applies to: 173-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/tu-do/shared/list-view.tsx` around lines 153 - 165, handleBulkDeleteConfirmed is performing deletes directly via createClient().from('tasks').delete(), leaving the bulk delete outside the workspace-scoped internal API; update the code to call a workspace-aware bulkDelete provided by useBulkOperations instead. Add/extend useBulkOperations to export a bulkDelete (or bulkDeleteTasks) function that invokes the internal-api workspace-scoped route (via the packages/internal-api handlers) rather than raw Supabase client calls, then replace the direct delete in handleBulkDeleteConfirmed (and the similar call at 173-184) to call that bulkDelete and handle success/error/selection clearing the same way.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/calendar/messages/en.json`:
- Around line 1695-1696: Add Vietnamese translations for the two new message
keys introduced in English: "archived_tasks_partially" and
"archived_tasks_successfully". Open apps/calendar/messages/vi.json and add
entries with those exact keys, providing proper Vietnamese pluralized strings
that match the English intent (one vs other forms), ensuring the ICU/plural
syntax mirrors the en.json forms so Vietnamese users won't see fallback text.
In `@apps/calendar/messages/vi.json`:
- Around line 1695-1696: Add matching English entries for the new Vietnamese
keys by adding "archived_tasks_partially" and "archived_tasks_successfully" to
apps/calendar/messages/en.json; use equivalent ICU plural-form messages (e.g.,
"archived_tasks_partially": "{moved, plural, one {Archived 1 task} other
{Archived # tasks}}, {failed, plural, one {1 task failed} other {# tasks
failed}}" and "archived_tasks_successfully": "{count, plural, one {Archived
successfully 1 task} other {Archived successfully # tasks}}") so the en.json and
vi.json locales remain in sync and the plural formatting matches the vi.json
patterns.
In `@apps/finance/messages/en.json`:
- Around line 1678-1679: Add the two new translation keys introduced in
en.json—"archived_tasks_partially" and "archived_tasks_successfully"—to the
Vietnamese locale file (apps/finance/messages/vi.json) with equivalent message
patterns and pluralization placeholders ({moved, plural,...}, {failed,
plural,...}, {count, plural,...}) preserved so interpolation still works;
provide correct Vietnamese copy for both the singular and plural branches while
keeping the same variable names and plural structure as in en.json.
In `@apps/rewise/messages/en.json`:
- Around line 1682-1683: You added two new English message keys
archived_tasks_partially and archived_tasks_successfully but did not add their
Vietnamese counterparts; open the Vietnamese messages file (vi.json) and add
matching keys with appropriate Vietnamese translations for both
archived_tasks_partially and archived_tasks_successfully so both locales have
the same keys and values translated; ensure plural formatting mirrors the
English ICU-style placeholders ({moved, plural,...}, {failed, plural,...}, and
{count, plural,...}) to preserve runtime interpolation.
In `@apps/web/messages/en.json`:
- Around line 1938-1939: Add the two new i18n keys introduced in
en.json—"archived_tasks_partially" and "archived_tasks_successfully"—to
apps/web/messages/vi.json with appropriate Vietnamese translations mirroring the
pluralization patterns from English; update vi.json so the keys exist and use
ICU plural forms matching the English structure (one/other) and provide accurate
Vietnamese text for both cases to maintain locale parity and avoid fallbacks.
In
`@apps/web/src/app/api/v1/workspaces/`[wsId]/task-drafts/[draftId]/convert/route.ts:
- Around line 28-31: Extract params and create the public supabase client
(createClient(request)) first, then perform the authentication check using that
client and only after a successful auth create the admin client
(createAdminClient()) and call normalizeWorkspaceId(rawWsId, supabase); in other
words, move the auth block above the call to normalizeWorkspaceId and the
createAdminClient invocation so unauthenticated requests return 401 before any
workspace normalization logic runs.
In `@packages/ui/src/components/ui/tu-do/boards/boardId/kanban.tsx`:
- Around line 238-239: The component mixes workspaceId and workspace.id: make
workspaceId the canonical identifier (prop name wsId where used for DnD) and
replace all uses of workspace.id with workspaceId across this file (including
props passed to child components and calls at the locations referenced around
lines 367–368 and 401); explicitly thread wsId (and taskId where applicable)
into every async helper and useEffect dependency list and into any
workspace-scoped hooks or child props so all board children receive the same
wsId value and no scope/key mismatches occur.
In
`@packages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.ts`:
- Around line 625-628: The two queryClient.invalidateQueries calls
(queryClient.invalidateQueries({ queryKey: ['tasks', boardId] }) and
queryClient.invalidateQueries({ queryKey: ['tasks', data.targetBoardId] })) are
causing an immediate refetch that breaks the optimistic reconciliation and
broadcast-driven sync; remove these invalidations from the Kanban move flow in
bulk-operations.ts and rely on the existing optimistic update +
BoardBroadcastContext broadcast path to propagate changes instead, ensuring the
move handler (the function performing the optimistic reconciliation for the
Kanban move) does not trigger any other query invalidation or postgres_changes
logic for task sync.
In `@packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx`:
- Around line 80-96: The delete/edit actions in this component call
deleteListMutation (which uses updateWorkspaceTaskList) but allow the UI to open
those dialogs when wsId or boardId are missing; change behavior so actions are
gated: either make wsId and boardId required props for this component (so
callers must pass them) or, more safely, early-return/disable and hide the
edit/delete menu items and their dialogs until both wsId and boardId are
present; specifically, add checks around the UI elements that trigger
deleteListMutation and the edit mutation (the menu items and buttons referenced
around deleteListMutation and the block at lines ~106-121) to disable or not
render them when !wsId || !boardId, and ensure any click handlers avoid calling
deleteListMutation/updateWorkspaceTaskList when IDs are absent.
In `@packages/ui/src/components/ui/tu-do/boards/boardId/task.tsx`:
- Around line 532-553: The duplicate-task flow in handleDuplicateTask currently
calls createTask(...) directly with await, bypassing TanStack Query mutation
lifecycle; refactor by creating a useMutation hook (e.g.,
useDuplicateTaskMutation or reuse existing task mutation hook) that wraps
createTask(effectiveWorkspaceId, sourceTask.list_id, taskData) and returns
mutateAsync; move the taskData construction (name, description, priority,
start_date, end_date, estimation_points, label_ids, assignee_ids, project_ids)
into the mutation input or call site, then replace the direct await
createTask(...) with await mutateAsync({ workspaceId: effectiveWorkspaceId,
listId: sourceTask.list_id, data: taskData }) and handle success/error via the
mutation callbacks or returned promise so the operation integrates with TanStack
Query state management.
In `@packages/utils/src/task-helper/bulk-actions.ts`:
- Around line 277-301: The mutationFn currently logs per-task errors but always
returns { count } which hides total failures; modify mutationFn that uses
listAllActiveTasksForList and updateWorkspaceTask so that after iterating you
(1) collect failed task IDs (or errors) alongside the success count, (2) if
tasks.length > 0 and count === 0 throw an Error containing contextual info
(e.g., "failed to clear any assignees" plus failed IDs/count), and (3) when
partial successes occurred return an object like { count, failedIds } so callers
can distinguish full success, partial success, and total failure; apply the same
change to the other mutationFn blocks referenced (the ones around lines 335–357
and 390–412).
- Around line 195-208: The code calls queryClient.invalidateQueries for
['tasks', currentBoardId] and ['task_lists', currentBoardId] (and for
variables.targetBoardId) which forces full board reloads and breaks the
Broadcast-based realtime model; instead, remove these invalidateQueries calls in
bulk-actions.ts and update the optimistic cache entries directly (reconcile
affected tasks/task_lists in the query cache via the same logic already used in
the mutation) and emit a board-level event through BoardBroadcastContext to
notify other clients; specifically, stop invalidating in the blocks referencing
queryClient.invalidateQueries, use the existing optimistic update routines to
adjust the ['tasks', ...] and ['task_lists', ...] cache entries, and call the
BoardBroadcastContext broadcaster to sync changes when variables.targetBoardId
or currentBoardId are affected.
In `@packages/utils/src/task-helper/sort-keys.ts`:
- Around line 176-197: hasSortKeyCollisions currently requires full Task objects
but only reads the sort_key field, forcing unsafe caller casts; change its
parameter type from Task[] to a minimal interface like Array<{ sort_key?: number
| null | undefined }> (or a named SortKeyLike type) so callers (currently
casting tasks as unknown as Task[]) can pass their arrays without double casts,
keep the function body and usage of SORT_KEY_MIN_GAP unchanged, and update any
imports/types if you introduce a named type.
In `@packages/utils/src/task-helper/task-hooks-basic.ts`:
- Around line 174-188: The fallback "|| 'New Task'" on the optimisticTask.name
is dead code because earlier validation already throws for empty/whitespace
names; remove the fallback and set name directly from task.name in the
optimisticTask object (i.e., change name: task.name || 'New Task' to name:
task.name) so the optimisticTask reflects the validated input (refer to the
optimisticTask object and the preceding validation logic that throws on invalid
task.name).
- Around line 221-289: The optimistic update in useDeleteTask sets a
client-generated deleted_at in onMutate but lacks an onSuccess to reconcile the
cache with the server response; add an onSuccess handler for the mutation
(alongside the existing mutationFn and onMutate) that receives the returned task
from updateWorkspaceTask and replaces or prepends the server task into the
['deleted-tasks', boardId] query data (using queryClient.setQueryData) so the
cache uses the server-assigned deleted_at and any other server fields; ensure it
also removes the task from ['tasks', boardId] if present or refreshes both
queries as appropriate.
In `@packages/utils/src/task-helper/task-operations.ts`:
- Around line 82-90: The embedding trigger is skipped on server because of the
`typeof window !== 'undefined'` guard; instead, call
triggerWorkspaceTaskEmbedding(wsId, createdTask.id, apiOptions) for both
environments by reusing the same API options object passed to the createTask
mutation (remove the window check and replace getBrowserApiOptions() with the
API options variable used for the mutation) so server-side-created tasks also
enqueue embedding generation; keep the existing .catch error handling.
---
Outside diff comments:
In `@apps/web/src/app/api/v1/workspaces/`[wsId]/tasks/[taskId]/embedding/route.ts:
- Around line 33-40: Authenticate before calling normalizeWorkspaceId by moving
the supabase.auth.getUser() call (and its user check) to precede
normalizeWorkspaceId(rawWsId, supabase); then validate the path param taskId
using the shared zod UUID schema (e.g., call
sharedSchema.guid().safeParse(taskId) or the project’s existing zod validator)
and return a 400 on parse failure before using taskId in any DB/PostgREST calls;
update handling in this file’s route (and the similar block at lines 49–65) to
reject unauthenticated requests with 401 and malformed task IDs with 400 rather
than letting normalizeWorkspaceId or PostgREST surface a 500.
- Around line 33-65: Before using the admin client to read/update the task,
enforce workspace membership with the user-scoped client: use the supabase
instance returned by createClient(request) (after normalizeWorkspaceId) to query
the workspace_members table for the current user and wsId, explicitly check the
query error and return a 500 on lookup failure, return a 403 if no membership
row exists, and only then call createAdminClient()/sbAdmin to fetch/update the
tasks; update the handler around the normalizeWorkspaceId/supabase usage and the
sbAdmin usage (references: createClient(request), normalizeWorkspaceId,
supabase, workspace_members, createAdminClient, sbAdmin) to implement this
control flow.
In `@packages/ui/src/components/ui/tu-do/shared/list-view.tsx`:
- Around line 153-165: When workspaceId changes, reset the bulk selection and
cancel any in-flight workspace-scoped requests so stale task IDs don't persist
across workspaces: add an effect in ListView that watches workspaceId and calls
the selection reset handler (clearSelection or setSelectedTasks([])) and uses
the queryClient to cancel or invalidate relevant queries (e.g.,
queryClient.cancelQueries / queryClient.invalidateQueries for workspace-scoped
keys) and notify useBulkOperations of the reset so bulkUpdateDueDate /
bulkUpdatePriority won't operate on old IDs.
- Around line 153-165: handleBulkDeleteConfirmed is performing deletes directly
via createClient().from('tasks').delete(), leaving the bulk delete outside the
workspace-scoped internal API; update the code to call a workspace-aware
bulkDelete provided by useBulkOperations instead. Add/extend useBulkOperations
to export a bulkDelete (or bulkDeleteTasks) function that invokes the
internal-api workspace-scoped route (via the packages/internal-api handlers)
rather than raw Supabase client calls, then replace the direct delete in
handleBulkDeleteConfirmed (and the similar call at 173-184) to call that
bulkDelete and handle success/error/selection clearing the same way.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0e936cf-76a8-4207-ad86-b12331496588
📒 Files selected for processing (29)
AGENTS.mdapps/calendar/messages/en.jsonapps/calendar/messages/vi.jsonapps/finance/messages/en.jsonapps/finance/messages/vi.jsonapps/rewise/messages/en.jsonapps/rewise/messages/vi.jsonapps/tasks/messages/en.jsonapps/tasks/messages/vi.jsonapps/track/messages/en.jsonapps/track/messages/vi.jsonapps/web/messages/en.jsonapps/web/messages/vi.jsonapps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.tsapps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.tspackages/apis/src/tu-do/tasks/route.tspackages/internal-api/src/index.tspackages/ui/src/components/ui/tu-do/boards/boardId/kanban.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.tspackages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/task.tsxpackages/ui/src/components/ui/tu-do/shared/board-views.tsxpackages/ui/src/components/ui/tu-do/shared/list-view.tsxpackages/utils/src/task-helper/board.tspackages/utils/src/task-helper/bulk-actions.tspackages/utils/src/task-helper/sort-keys.tspackages/utils/src/task-helper/task-hooks-basic.tspackages/utils/src/task-helper/task-hooks-move.tspackages/utils/src/task-helper/task-operations.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.ts (1)
96-102: 🧹 Nitpick | 🔵 TrivialUse a ref for
lastTargetListId.
processDragOvernow changes identity wheneverhoverTargetListIdorwsIdchanges. That means(processDragOver as any).lastTargetListIdis thrown away mid-drag, and the effect below re-subscribes the global pointer listeners on every hover update. Move that sentinel into auseRefand use functional state updates so the callback can stay stable.Also applies to: 176-186, 190-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.ts` around lines 96 - 102, processDragOver currently stores a sentinel on the function object ((processDragOver as any).lastTargetListId), which is lost whenever hoverTargetListId or wsId changes causing the callback to be re-created; replace that pattern by creating a ref (e.g., lastTargetListIdRef = useRef<string | null>(null)) and read/write lastTargetListIdRef.current inside processDragOver, switch any state updates to functional updates (setHoverTargetListId(id => ...)) so processDragOver can be memoized/stable, and apply the same ref-based fix to the other drag handlers mentioned around the later blocks (the handlers referenced around lines 176-186 and 190-206) to avoid re-subscribing global pointer listeners mid-drag.
♻️ Duplicate comments (2)
apps/finance/messages/en.json (1)
1678-1679:⚠️ Potential issue | 🟠 MajorAdd matching Vietnamese translations for the new archive/move result keys.
Line 1678, Line 1679, Line 2003, and Line 2004 introduce new user-facing
common.*keys inen.json, but matchingapps/finance/messages/vi.jsonentries are not present in this PR context. Please add the corresponding Vietnamese keys with the same ICU variables/plural structure to avoid missing locale entries.Suggested
vi.jsonentries{ "common": { "archived_tasks_partially": "{moved, plural, one {Đã lưu trữ 1 công việc} other {Đã lưu trữ # công việc}}, {failed, plural, one {1 công việc thất bại} other {# công việc thất bại}}", "archived_tasks_successfully": "{count, plural, one {Đã lưu trữ thành công 1 công việc} other {Đã lưu trữ thành công # công việc}}", "moved_tasks_partially": "{moved, plural, one {Đã di chuyển 1 công việc} other {Đã di chuyển # công việc}}, {failed, plural, one {1 công việc thất bại} other {# công việc thất bại}}", "moved_tasks_successfully": "{count, plural, one {Đã di chuyển thành công 1 công việc} other {Đã di chuyển thành công # công việc}}" } }As per coding guidelines: Add translation keys to both
en.jsonANDvi.jsonfor new UI elements.Also applies to: 2003-2004
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/finance/messages/en.json` around lines 1678 - 1679, Add the missing Vietnamese translations in apps/finance/messages/vi.json for the four new common.* keys introduced in en.json: archived_tasks_partially, archived_tasks_successfully, moved_tasks_partially, and moved_tasks_successfully; ensure each entry uses the same ICU plural variables and structure (e.g., "{moved, plural, one {...} other {...}}", "{failed, plural, one {...} other {...}}", and "{count, plural, one {...} other {...}}") and match the suggested Vietnamese strings so locales remain consistent.apps/calendar/messages/vi.json (1)
1695-1696:⚠️ Potential issue | 🟠 MajorAdd matching
en.jsonkeys for newly introduced Vietnamese strings.
vi.jsonadds four new user-facing keys, but correspondingapps/calendar/messages/en.jsonentries are not present in the provided changes. Please add matching English keys for:
common.archived_tasks_partiallycommon.archived_tasks_successfullycommon.moved_tasks_partiallycommon.moved_tasks_successfullyAs per coding guidelines: ALWAYS provide translations for both English (
en.json) AND Vietnamese (vi.json) for all user-facing strings.Also applies to: 2020-2021
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/calendar/messages/vi.json` around lines 1695 - 1696, Add matching English entries in apps/calendar/messages/en.json for the four new Vietnamese keys: common.archived_tasks_partially, common.archived_tasks_successfully, common.moved_tasks_partially, and common.moved_tasks_successfully; use ICU plural message format mirroring the vi.json patterns (e.g., "{moved, plural, one {Archived 1 task} other {Archived # tasks}}, {failed, plural, one {1 task failed} other {# tasks failed}}" for archived_tasks_partially and "{count, plural, one {Archived 1 task successfully} other {Archived # tasks successfully}}" for archived_tasks_successfully, and analogous "Moved" variants for the moved_* keys) so English and Vietnamese files stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/calendar/messages/en.json`:
- Around line 2020-2021: Add the missing Vietnamese translations for the two new
ICU plural keys introduced in English: add "moved_tasks_partially" and
"moved_tasks_successfully" entries to apps/calendar/messages/vi.json using the
same ICU plural structure as in en.json but with proper Vietnamese text (e.g.,
translate "Moved X task(s), Y failed" and "Moved X task(s) successfully" while
keeping the {moved, plural,...} and {failed, plural,...} / {count, plural,...}
placeholders and plural forms intact) so the keys match exactly those in
en.json.
In `@apps/track/messages/en.json`:
- Around line 1678-1679: Add the missing Vietnamese translations for the new
message keys introduced in en.json: add entries for "archived_tasks_partially"
and "archived_tasks_successfully" (and the two other new keys referenced around
lines 2003–2004) into vi.json using equivalent pluralization patterns and
Vietnamese strings so keys are present in both locales; ensure the plural ICU
syntax matches the English keys exactly (using {moved, plural, ...}, {failed,
plural, ...} or {count, plural, ...} as appropriate) and provide correct
Vietnamese text for each plural branch.
In
`@apps/web/src/app/api/v1/workspaces/`[wsId]/task-drafts/[draftId]/convert/route.ts:
- Around line 122-132: The current route blindly casts draft relation fields to
string[] (draft.assignee_ids, draft.label_ids, draft.project_ids) which can pass
mixed or non-string values into createTask; update this file to import the
proper draft type from `@tuturuuu/types/db` (do not hand-create ad-hoc types),
change the route handler signature to use that typed draft, and replace the
forced casts with normalization logic that: checks Array.isArray(...), filters
each element to typeof === 'string', removes falsy/empty strings, and maps to
trimmed IDs before passing them to createTask; ensure malformed drafts fail
validation at the route boundary (return 400) rather than being forwarded to
createTask.
In `@packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx`:
- Around line 168-181: The code currently hardcodes the Archived list name when
creating a fallback list in the closedListId assignment; replace the literal
'Archived' with a localized string lookup (e.g., use your app's translation
helper such as t('archived') or i18n.t('archived')) when calling
createWorkspaceTaskList so the created list name respects user locale; ensure
the translation key "archived" is present in the locale files (e.g., en.json and
vi.json), and update any imports or context needed so
createWorkspaceTaskList(wsId, boardId, { name: t('archived'), status: 'closed',
color: 'PURPLE' }, options) works correctly while leaving existingClosedList,
wsId, boardId and options usage unchanged.
In `@packages/ui/src/components/ui/tu-do/shared/list-view.tsx`:
- Line 120: The isLoading state declared as const [isLoading] = useState(false)
is dead (no setter) and causes unreachable loading paths; either remove
isLoading and all branches that depend on it (the loading skeleton render block
and the !isLoading check in the scroll handler) and also remove it from the
effect dependencies, or replace it by deriving loading from an existing state
like bulkWorking or a real loading flag: update the declaration (e.g., use
bulkWorking or realLoading), adjust the scroll handler's !isLoading check to use
that derived flag, update the effect dependency list to include the new loading
variable, and remove or repurpose the skeleton render block (lines referenced
around the loading skeleton and the scroll handler) so there is no unused state
left.
In `@packages/utils/src/task-helper/sort-keys.ts`:
- Around line 270-279: The current use of Promise.all in the chunked update loop
(chunk.map -> updateWorkspaceTask) will abort on the first rejection and leave
later chunks unprocessed; replace Promise.all with Promise.allSettled for each
chunk so every update attempt runs, then inspect the settled results, collect
any errors from rejected entries, and after processing all chunks aggregate and
throw or log a combined error (or return failures) so callers can see
partial-failure details; update the code paths around updateWorkspaceTask and
the chunk processing to perform this result-check and error-aggregation.
In `@packages/utils/src/task-helper/task-hooks-basic.ts`:
- Around line 256-280: onError currently only restores ['deleted-tasks',
boardId] when context.previousDeletedTasks exists, leaving an
optimistically-added deleted task in place if there was no prior snapshot;
update the onError handler to also handle the case where
context.previousDeletedTasks is undefined but context.deletedTask is present:
read the current deleted list via queryClient.getQueryData(['deleted-tasks',
boardId]), filter out the task with id === context.deletedTask.id (or set
undefined if the resulting list is empty), and write the filtered result back
with queryClient.setQueryData; reference the onError handler,
context.previousDeletedTasks, context.deletedTask, queryClient.getQueryData and
queryClient.setQueryData when making the change.
- Around line 169-200: onMutate currently seeds ['tasks', boardId] with an
optimisticTask but onError only restores when context.previousTasks is truthy,
leaving temp tasks when previousTasks was undefined; update onError (handler
next to onMutate) to handle the undefined snapshot by either restoring
context.previousTasks if present or removing the optimisticTask from the cache:
fetch current tasks via queryClient.getQueryData(['tasks', boardId]) and
setQueryData to filter out the task with id === context.optimisticTask.id (or
where id startsWith 'temp-'), ensuring you reference onMutate/onError,
queryClient, optimisticTask and previousTasks in the change.
In `@packages/utils/src/task-helper/task-operations.ts`:
- Around line 162-170: The helper syncTaskArchivedStatus currently maps both
list.status 'done' and 'closed' into closed_at; update it so it distinguishes
the two: when list.status === 'done' set completed_at to now and clear
closed_at, when list.status === 'closed' set closed_at to now and clear
completed_at, and when moving out of either clear the respective timestamp;
adjust the payload passed to updateWorkspaceTask in syncTaskArchivedStatus (and
ensure moveTask use still calls it) to include completed_at or closed_at
appropriately rather than always toggling closed_at.
---
Outside diff comments:
In
`@packages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.ts`:
- Around line 96-102: processDragOver currently stores a sentinel on the
function object ((processDragOver as any).lastTargetListId), which is lost
whenever hoverTargetListId or wsId changes causing the callback to be
re-created; replace that pattern by creating a ref (e.g., lastTargetListIdRef =
useRef<string | null>(null)) and read/write lastTargetListIdRef.current inside
processDragOver, switch any state updates to functional updates
(setHoverTargetListId(id => ...)) so processDragOver can be memoized/stable, and
apply the same ref-based fix to the other drag handlers mentioned around the
later blocks (the handlers referenced around lines 176-186 and 190-206) to avoid
re-subscribing global pointer listeners mid-drag.
---
Duplicate comments:
In `@apps/calendar/messages/vi.json`:
- Around line 1695-1696: Add matching English entries in
apps/calendar/messages/en.json for the four new Vietnamese keys:
common.archived_tasks_partially, common.archived_tasks_successfully,
common.moved_tasks_partially, and common.moved_tasks_successfully; use ICU
plural message format mirroring the vi.json patterns (e.g., "{moved, plural, one
{Archived 1 task} other {Archived # tasks}}, {failed, plural, one {1 task
failed} other {# tasks failed}}" for archived_tasks_partially and "{count,
plural, one {Archived 1 task successfully} other {Archived # tasks
successfully}}" for archived_tasks_successfully, and analogous "Moved" variants
for the moved_* keys) so English and Vietnamese files stay in sync.
In `@apps/finance/messages/en.json`:
- Around line 1678-1679: Add the missing Vietnamese translations in
apps/finance/messages/vi.json for the four new common.* keys introduced in
en.json: archived_tasks_partially, archived_tasks_successfully,
moved_tasks_partially, and moved_tasks_successfully; ensure each entry uses the
same ICU plural variables and structure (e.g., "{moved, plural, one {...} other
{...}}", "{failed, plural, one {...} other {...}}", and "{count, plural, one
{...} other {...}}") and match the suggested Vietnamese strings so locales
remain consistent.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70ba909a-c084-4fc5-8dfe-a64c8f8bd3ed
📒 Files selected for processing (24)
apps/calendar/messages/en.jsonapps/calendar/messages/vi.jsonapps/finance/messages/en.jsonapps/finance/messages/vi.jsonapps/rewise/messages/en.jsonapps/rewise/messages/vi.jsonapps/tasks/messages/en.jsonapps/tasks/messages/vi.jsonapps/track/messages/en.jsonapps/track/messages/vi.jsonapps/web/messages/en.jsonapps/web/messages/vi.jsonapps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.tspackages/ui/src/components/ui/tu-do/boards/boardId/kanban.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/kanban/bulk/bulk-operations.tspackages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.tspackages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsxpackages/ui/src/components/ui/tu-do/boards/boardId/task.tsxpackages/ui/src/components/ui/tu-do/shared/list-view.tsxpackages/utils/src/task-helper/bulk-actions.tspackages/utils/src/task-helper/recycle-bin.tspackages/utils/src/task-helper/sort-keys.tspackages/utils/src/task-helper/task-hooks-basic.tspackages/utils/src/task-helper/task-operations.ts
apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (7)
apps/rewise/messages/en.json (1)
1682-1683:⚠️ Potential issue | 🟠 MajorAdd matching
vi.jsonentries for newcommon.*keys.These new user-facing keys were added in
en.json, but this diff does not show corresponding additions inapps/rewise/messages/vi.json. Please add matching Vietnamese keys to avoid missing translations.#!/bin/bash set -euo pipefail python - <<'PY' import json from pathlib import Path en_path = Path("apps/rewise/messages/en.json") vi_path = Path("apps/rewise/messages/vi.json") en = json.loads(en_path.read_text(encoding="utf-8")) vi = json.loads(vi_path.read_text(encoding="utf-8")) keys = [ "common.archived_tasks_partially", "common.archived_tasks_successfully", "common.moved_tasks_partially", "common.moved_tasks_successfully", "common.task_relationship_already_exists", "common.task_relationship_circular", "common.task_relationship_single_parent", ] def get(obj, path): cur = obj for p in path.split("."): if not isinstance(cur, dict) or p not in cur: return None cur = cur[p] return cur missing = [k for k in keys if get(vi, k) is None] if missing: print("Missing keys in apps/rewise/messages/vi.json:") for k in missing: print("-", k) raise SystemExit(1) print("All new keys exist in vi.json.") PYAs per coding guidelines
**/messages/{en,vi}.json: “ALWAYS provide translations for both English (en.json) AND Vietnamese (vi.json) for all user-facing strings” and “Add translation keys to bothen.jsonANDvi.jsonfor new UI elements”.Also applies to: 2008-2009, 2220-2222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/rewise/messages/en.json` around lines 1682 - 1683, Add the missing Vietnamese translations for the new common.* keys added in en.json by inserting corresponding entries into vi.json for these keys: common.archived_tasks_partially, common.archived_tasks_successfully, common.moved_tasks_partially, common.moved_tasks_successfully, common.task_relationship_already_exists, common.task_relationship_circular, and common.task_relationship_single_parent; ensure the vi.json values preserve the same pluralization structure and placeholders (e.g., {moved, plural,...}, {count, plural,...}, {failed, plural,...}) as the English variants so runtime interpolation and plural forms remain consistent.apps/calendar/messages/vi.json (1)
1695-1696:⚠️ Potential issue | 🟠 MajorAdd matching English locale keys for the new archive result messages.
These new Vietnamese user-facing keys should be mirrored in
apps/calendar/messages/en.jsonto keep locales in sync.As per coding guidelines: ALWAYS provide translations for both English (en.json) AND Vietnamese (vi.json) for all user-facing strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/calendar/messages/vi.json` around lines 1695 - 1696, New Vietnamese locale keys archived_tasks_partially and archived_tasks_successfully were added but not mirrored in English; add matching keys with the same ICU plural structure to en.json so locales stay in sync. Specifically, create keys named "archived_tasks_partially" and "archived_tasks_successfully" in the English messages file and provide equivalent English text using the same plural/placeholder structure (e.g., "Archived 1 task" vs "Archived # tasks" / "%d failed" variants) so the format and placeholders exactly match the vi keys.apps/track/messages/en.json (1)
1678-1679:⚠️ Potential issue | 🟠 MajorAdd matching
vi.jsonentries for these new user-facing keys.Line 1678, Line 1679, Line 2003, Line 2004, Line 2215, Line 2216, and Line 2217 add new strings in
en.json, but this PR context does not include corresponding keys invi.json. This breaks locale parity for Vietnamese users.As per coding guidelines: add translation keys to both
en.jsonANDvi.jsonfor new UI elements in**/messages/{en,vi}.json.Also applies to: 2003-2004, 2215-2217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/track/messages/en.json` around lines 1678 - 1679, The Vietnamese locale file is missing translations for newly added English keys; add matching entries for the JSON keys archived_tasks_partially and archived_tasks_successfully (and the other keys added at the same PR locations) into apps/track/messages/vi.json with appropriate Vietnamese strings mirroring the pluralization patterns used in en.json so the keys and ICU plural forms match (use the same key names: "archived_tasks_partially", "archived_tasks_successfully", plus the other new keys referenced at the PR locations).apps/calendar/messages/en.json (1)
1695-1696:⚠️ Potential issue | 🟠 MajorAdd matching Vietnamese translations for the new bulk archive/move result keys.
The new English keys at Line 1695-Line 1696 and Line 2020-Line 2021 need matching entries in
apps/calendar/messages/vi.jsonto avoid fallback/missing text for Vietnamese users.Suggested `vi.json` additions
+ "common": { + ... + "archived_tasks_partially": "{moved, plural, one {Đã lưu trữ 1 tác vụ} other {Đã lưu trữ # tác vụ}}, {failed, plural, one {1 tác vụ thất bại} other {# tác vụ thất bại}}", + "archived_tasks_successfully": "{count, plural, one {Đã lưu trữ thành công 1 tác vụ} other {Đã lưu trữ thành công # tác vụ}}", + ... + "moved_tasks_partially": "{moved, plural, one {Đã di chuyển 1 tác vụ} other {Đã di chuyển # tác vụ}}, {failed, plural, one {1 tác vụ thất bại} other {# tác vụ thất bại}}", + "moved_tasks_successfully": "{count, plural, one {Đã di chuyển thành công 1 tác vụ} other {Đã di chuyển thành công # tác vụ}}", + ... + }As per coding guidelines: "ALWAYS provide translations for both English (en.json) AND Vietnamese (vi.json) for all user-facing strings" and "Add translation keys to both
en.jsonANDvi.jsonfor new UI elements".Also applies to: 2020-2021
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/calendar/messages/en.json` around lines 1695 - 1696, Add Vietnamese translations for the new English keys "archived_tasks_partially" and "archived_tasks_successfully" into apps/calendar/messages/vi.json (and likewise add the matching translations for the other new keys referenced around lines 2020-2021) so Vietnamese users don't fallback to English; ensure the vi.json entries mirror the ICU plural structure used in en.json (use {moved, plural,...} and {failed, plural,...} for archived_tasks_partially and {count, plural,...} for archived_tasks_successfully) and provide correct Vietnamese text for each plural branch.apps/finance/messages/en.json (1)
1678-1679:⚠️ Potential issue | 🟠 MajorAdd matching
vi.jsonentries for all newly added task-related keys.These new user-facing English keys need corresponding Vietnamese keys to keep locale parity and avoid missing translations at runtime.
As per coding guidelines: Add translation keys to both
en.jsonANDvi.jsonfor new UI elements.Also applies to: 2003-2004, 2215-2217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/finance/messages/en.json` around lines 1678 - 1679, Add matching Vietnamese translation entries for the two new English keys "archived_tasks_partially" and "archived_tasks_successfully" by inserting corresponding keys into vi.json with appropriate Vietnamese strings that mirror the pluralization structure (i.e., include {moved, plural, ...} and {failed, plural, ...} for archived_tasks_partially and {count, plural, ...} for archived_tasks_successfully); ensure pluralization tokens and placeholders match exactly the English keys so runtime lookup and interpolation succeed, and do the same for the other referenced ranges (the duplicate keys around 2003-2004 and 2215-2217) to maintain parity across locales.packages/utils/src/task-helper/task-hooks-basic.ts (1)
306-323:⚠️ Potential issue | 🟠 MajorAlso reconcile the live task list on delete success.
This handler updates
['deleted-tasks', boardId]but never re-removes the task from['tasks', boardId]. If a refetch repopulates the live list betweenonMutateandonSuccess, the deleted task can stay visible until a later refresh.♻️ Proposed fix
onSuccess: (deletedTask) => { + queryClient.setQueryData( + ['tasks', boardId], + (old: Task[] | undefined) => + old?.filter((task) => task.id !== deletedTask.id) + ); + queryClient.setQueryData( ['deleted-tasks', boardId], (old: Task[] | undefined) => { if (!old) return [deletedTask];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/task-helper/task-hooks-basic.ts` around lines 306 - 323, The onSuccess handler for the delete mutation updates ['deleted-tasks', boardId] but fails to remove the deletedTask from the live list, so ensure you also reconcile ['tasks', boardId]: in the onSuccess function (the block handling deletedTask and calling queryClient.setQueryData for ['deleted-tasks', boardId]) add a second queryClient.setQueryData call for ['tasks', boardId] that filters out the task with id === deletedTask.id (or replaces it appropriately) so any refetch or race between onMutate and onSuccess won't leave the deleted task visible; reference the existing deletedTask variable and use the same boardId key.apps/web/messages/en.json (1)
1938-1939:⚠️ Potential issue | 🟠 MajorAdd matching Vietnamese keys for all newly introduced
commonmessages.These user-facing strings were added in
apps/web/messages/en.jsonbut the correspondingapps/web/messages/vi.jsonadditions are not included here. Please add the same keys invi.jsonto avoid fallback/missing translations at runtime.As per coding guidelines: “ALWAYS provide translations for both English (en.json) AND Vietnamese (vi.json) for all user-facing strings” and “Add translation keys to both
en.jsonANDvi.jsonfor new UI elements”.Also applies to: 2275-2276, 2493-2495
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/messages/en.json` around lines 1938 - 1939, Add the missing Vietnamese translations for the newly introduced common message keys by adding identical keys to the vi.json translations file: include "archived_tasks_partially" and "archived_tasks_successfully" (and the other missing keys referenced around the later diffs) with appropriate Vietnamese strings mirroring the English plural forms so the localization lookup won’t fallback; update the vi.json entries to match the exact key names used in en.json (e.g., "archived_tasks_partially", "archived_tasks_successfully") and ensure plural/message placeholders are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/calendar/messages/en.json`:
- Around line 2232-2234: The three new English message keys
task_relationship_already_exists, task_relationship_circular, and
task_relationship_single_parent were added but no Vietnamese translations were
provided; add corresponding entries to the Vietnamese JSON (vi.json) with
appropriate Vietnamese text for each key (e.g., translations conveying "This
relationship already exists.", "This would create a circular relationship, which
is not allowed.", and "A task can only have one parent.") ensuring the keys
exactly match the English keys so the localization lookup works.
In `@apps/calendar/messages/vi.json`:
- Around line 2020-2021: The new Vietnamese keys moved_tasks_partially and
moved_tasks_successfully (and related task_relationship_* keys referenced in the
comment) were added only to apps/calendar/messages/vi.json; add equivalent
English entries to apps/calendar/messages/en.json using the same key names
(moved_tasks_partially, moved_tasks_successfully and any task_relationship_*
keys mentioned) with appropriate English strings so both en.json and vi.json
contain the same keys and avoid missing translation fallbacks.
In `@apps/tasks/messages/en.json`:
- Around line 1777-1778: Add the new English message keys to the Vietnamese
messages file by adding matching entries for "archived_tasks_partially" and
"archived_tasks_successfully" in apps/tasks/messages/vi.json (and also add
equivalents for the other listed key ranges: the keys around lines 2102-2103 and
2315-2317) — create Vietnamese translations that mirror the plural ICU format
used in en.json (retain the same plural structure and placeholders like {moved},
{failed}, and {count}) so the keys and tokens match exactly to the functions
that look up these keys at runtime.
In
`@apps/web/src/app/api/v1/workspaces/`[wsId]/task-drafts/[draftId]/convert/route.ts:
- Around line 98-104: Replace the initial SELECT of the draft with an atomic
UPDATE that "claims" the draft and returns the row; e.g., call
sbAdmin.from('task_drafts').update({ claimed_by: user.id, claimed_at: new
Date().toISOString() }).eq('id', draftId).eq('ws_id', wsId).eq('creator_id',
user.id').single() (or use a DELETE ...select/returning variant if you prefer
removing immediately), then stop and return an error when no row is returned (no
row matched) instead of proceeding to createTask; apply the same pattern to the
other similar block referenced (lines 176-194) so follow-up side effects only
run when a row was actually claimed.
In `@apps/web/src/app/api/v1/workspaces/`[wsId]/tasks/[taskId]/embedding/route.ts:
- Around line 139-142: The UPDATE against the tasks table using sbAdmin
currently doesn't request returning rows so we can't detect no-op mutations;
modify the update call that sets embedding (the sbAdmin.from('tasks').update({
embedding: JSON.stringify(embedding) }).eq('id', task.id)) to request returned
rows (e.g., returning('*')) and then inspect the returned data to detect if no
row matched (empty result) and abort any follow-up side effects when that
happens; ensure you still handle and log updateError if present and early-return
when returned rows is empty.
In `@packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx`:
- Around line 124-134: Before performing the optimistic update in the onMutate
handler, cancel any in-flight queries for the same key to avoid race conditions:
call queryClient.cancelQueries(['task_lists', boardId]) at the top of onMutate,
then proceed to use queryClient.setQueryData(...) (as currently done) and
optionally capture and return the previous value so onError can roll back;
update the onMutate in list-actions.tsx accordingly to mirror the pattern used
in board-layout-settings.tsx.
In `@packages/ui/src/components/ui/tu-do/shared/list-view.tsx`:
- Around line 171-180: The catch in handleBulkDeleteConfirmed is duplicating
user-facing error notifications because bulkDeleteTasks already toasts failures;
remove the toast call in handleBulkDeleteConfirmed and either just
console.error(error) or rethrow the error after logging so only bulkDeleteTasks
reports to the user. Locate handleBulkDeleteConfirmed and bulkDeleteTasks to
implement this change and ensure no other UI toast is emitted here on failure.
- Around line 190-204: The effect currently only clears selection when workspace
changes (it returns early if previousWorkspaceIdRef.current === workspaceId), so
switching boards within the same workspace leaves stale selected task IDs;
update the early-return condition in the useEffect to only return when both
workspaceId and boardId are unchanged (i.e., check
previousWorkspaceIdRef.current === workspaceId && previousBoardIdRef.current ===
boardId), and otherwise update previousWorkspaceIdRef/previousBoardIdRef, call
clearSelection(), and cancel queries with queryClient.cancelQueries(['tasks',
previousBoardId]) and queryClient.cancelQueries(['deleted-tasks',
previousBoardId]) as already done.
In `@packages/utils/src/task-helper/relationships.ts`:
- Around line 201-217: The query-backed task search builds clientOptions without
cache control, causing stale results; update the clientOptions object used when
calling listWorkspaceTasks (and the similar usage around the block referencing
lines 287-290) to include cache: 'no-store' (e.g., ensure the clientOptions
created in this module includes { baseUrl: window.location.origin, cache:
'no-store' } when window is defined, and pass that same clientOptions into
listWorkspaceTasks and any other listWorkspaceTasks calls in this file so
useWorkspaceTasks' queryFn always fetches with cache: 'no-store').
- Around line 193-217: The loop currently drops `q` when `ticketLikeSearch` is
true and then client-side filters, causing many API calls; instead extend
`listWorkspaceTasks` to accept an identifier filter (e.g., identifier or
ticketIdentifier param) that the server will use to match `display_number` /
`ticket_prefix`, and call `listWorkspaceTasks(wsId, { identifier:
options?.searchQuery, limit: requestLimit, offset, includeRelationshipSummary:
true }, clientOptions)` when `isTicketIdentifierLikeQuery(normalizedSearch)` is
true; update the server handler for `listWorkspaceTasks` to perform efficient DB
filtering on `display_number`/`ticket_prefix` so the pagination remains
server-filtered and the client loop no longer needs to scan the whole workspace.
In `@packages/utils/src/task-helper/sort-keys.ts`:
- Around line 222-245: The current guard treats an empty visualOrderTasks array
as “not provided” because it checks `visualOrderTasks && visualOrderTasks.length
> 0`, causing a refetch and potential renumbering; change the condition to
detect presence instead of truthiness (e.g., check for !== undefined && !== null
or use Array.isArray(visualOrderTasks)) and assign `tasks = visualOrderTasks`
whenever the caller provided an array (including []). Update the block around
the `visualOrderTasks` assignment (the code that currently calls
`listAllActiveTasksForList` and later maps/sorts) so that an explicit empty
array is honored and only when `visualOrderTasks` is truly absent do you call
`listAllActiveTasksForList`.
In `@packages/utils/src/task-helper/task-hooks-basic.ts`:
- Around line 76-88: The current onSuccess handler in the
queryClient.setQueryData call for ['tasks', boardId] is overwriting
server-returned relation fields by copying assignees, labels, and projects from
the cached task into updatedTask; change the mapping in the onSuccess callback
so you do not replace updatedTask.assignees, updatedTask.labels, or
updatedTask.projects with the old cached values—instead return updatedTask as-is
(or merge cached scalar fields into updatedTask but keep its relation fields),
so the server-normalized relations from updatedTask are preserved when updating
the cache in the onSuccess mapping.
- Around line 215-223: onSuccess's cache reconciliation only replaces an item
matching context.optimisticTask.id which drops the created task when the
optimistic row has been evicted; change the update passed to
queryClient.setQueryData(['tasks', boardId]) so that after mapping you detect
whether any item was replaced and, if not, insert newTask into the resulting
array (while also deduplicating by id to avoid duplicates if the server response
already arrived); update the logic around onSuccess, queryClient.setQueryData,
context.optimisticTask.id and newTask to implement this append-or-replace
behavior.
In `@packages/utils/src/task-helper/task-operations.ts`:
- Around line 190-195: The catch block in syncTaskArchivedStatus (around the
updateWorkspaceTask call) currently swallows errors by only logging them, which
lets moveTask proceed with a partial/incorrect state; change the handler to
propagate failures so callers can retry or abort: either remove the try/catch so
the thrown updateError bubbles up from syncTaskArchivedStatus, or after logging
re-throw the caught updateError (or throw a new Error that wraps it) so
moveTask() will fail when the archive-state sync fails; apply the same fix to
the other similar catch block covering lines 198-215 to ensure all sync failures
are propagated rather than treated as success.
---
Duplicate comments:
In `@apps/calendar/messages/en.json`:
- Around line 1695-1696: Add Vietnamese translations for the new English keys
"archived_tasks_partially" and "archived_tasks_successfully" into
apps/calendar/messages/vi.json (and likewise add the matching translations for
the other new keys referenced around lines 2020-2021) so Vietnamese users don't
fallback to English; ensure the vi.json entries mirror the ICU plural structure
used in en.json (use {moved, plural,...} and {failed, plural,...} for
archived_tasks_partially and {count, plural,...} for
archived_tasks_successfully) and provide correct Vietnamese text for each plural
branch.
In `@apps/calendar/messages/vi.json`:
- Around line 1695-1696: New Vietnamese locale keys archived_tasks_partially and
archived_tasks_successfully were added but not mirrored in English; add matching
keys with the same ICU plural structure to en.json so locales stay in sync.
Specifically, create keys named "archived_tasks_partially" and
"archived_tasks_successfully" in the English messages file and provide
equivalent English text using the same plural/placeholder structure (e.g.,
"Archived 1 task" vs "Archived # tasks" / "%d failed" variants) so the format
and placeholders exactly match the vi keys.
In `@apps/finance/messages/en.json`:
- Around line 1678-1679: Add matching Vietnamese translation entries for the two
new English keys "archived_tasks_partially" and "archived_tasks_successfully" by
inserting corresponding keys into vi.json with appropriate Vietnamese strings
that mirror the pluralization structure (i.e., include {moved, plural, ...} and
{failed, plural, ...} for archived_tasks_partially and {count, plural, ...} for
archived_tasks_successfully); ensure pluralization tokens and placeholders match
exactly the English keys so runtime lookup and interpolation succeed, and do the
same for the other referenced ranges (the duplicate keys around 2003-2004 and
2215-2217) to maintain parity across locales.
In `@apps/rewise/messages/en.json`:
- Around line 1682-1683: Add the missing Vietnamese translations for the new
common.* keys added in en.json by inserting corresponding entries into vi.json
for these keys: common.archived_tasks_partially,
common.archived_tasks_successfully, common.moved_tasks_partially,
common.moved_tasks_successfully, common.task_relationship_already_exists,
common.task_relationship_circular, and common.task_relationship_single_parent;
ensure the vi.json values preserve the same pluralization structure and
placeholders (e.g., {moved, plural,...}, {count, plural,...}, {failed,
plural,...}) as the English variants so runtime interpolation and plural forms
remain consistent.
In `@apps/track/messages/en.json`:
- Around line 1678-1679: The Vietnamese locale file is missing translations for
newly added English keys; add matching entries for the JSON keys
archived_tasks_partially and archived_tasks_successfully (and the other keys
added at the same PR locations) into apps/track/messages/vi.json with
appropriate Vietnamese strings mirroring the pluralization patterns used in
en.json so the keys and ICU plural forms match (use the same key names:
"archived_tasks_partially", "archived_tasks_successfully", plus the other new
keys referenced at the PR locations).
In `@apps/web/messages/en.json`:
- Around line 1938-1939: Add the missing Vietnamese translations for the newly
introduced common message keys by adding identical keys to the vi.json
translations file: include "archived_tasks_partially" and
"archived_tasks_successfully" (and the other missing keys referenced around the
later diffs) with appropriate Vietnamese strings mirroring the English plural
forms so the localization lookup won’t fallback; update the vi.json entries to
match the exact key names used in en.json (e.g., "archived_tasks_partially",
"archived_tasks_successfully") and ensure plural/message placeholders are
preserved.
In `@packages/utils/src/task-helper/task-hooks-basic.ts`:
- Around line 306-323: The onSuccess handler for the delete mutation updates
['deleted-tasks', boardId] but fails to remove the deletedTask from the live
list, so ensure you also reconcile ['tasks', boardId]: in the onSuccess function
(the block handling deletedTask and calling queryClient.setQueryData for
['deleted-tasks', boardId]) add a second queryClient.setQueryData call for
['tasks', boardId] that filters out the task with id === deletedTask.id (or
replaces it appropriately) so any refetch or race between onMutate and onSuccess
won't leave the deleted task visible; reference the existing deletedTask
variable and use the same boardId key.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f96bca9-f83e-4350-b6ec-74094ec2690c
📒 Files selected for processing (24)
apps/calendar/messages/en.jsonapps/calendar/messages/vi.jsonapps/finance/messages/en.jsonapps/finance/messages/vi.jsonapps/rewise/messages/en.jsonapps/rewise/messages/vi.jsonapps/tasks/messages/en.jsonapps/tasks/messages/vi.jsonapps/track/messages/en.jsonapps/track/messages/vi.jsonapps/web/messages/en.jsonapps/web/messages/vi.jsonapps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.tsapps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.tspackages/types/src/db.tspackages/ui/src/components/ui/tu-do/boards/boardId/kanban/dnd/use-kanban-dnd.tspackages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsxpackages/ui/src/components/ui/tu-do/hooks/useTaskCardRelationships.tspackages/ui/src/components/ui/tu-do/shared/list-view.tsxpackages/ui/src/components/ui/tu-do/shared/task-edit-dialog/hooks/use-task-dependencies.tspackages/utils/src/task-helper/relationships.tspackages/utils/src/task-helper/sort-keys.tspackages/utils/src/task-helper/task-hooks-basic.tspackages/utils/src/task-helper/task-operations.ts
apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.ts
Outdated
Show resolved
Hide resolved
apps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
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="apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.ts">
<violation number="1" location="apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.ts:146">
P2: Deleting the draft before creating the task introduces a data-loss path when task creation fails and draft restoration also fails.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx (1)
440-451: 🧹 Nitpick | 🔵 TrivialRedundant
wsIdcheck.
canManageListis alreadyBoolean(wsId && boardId), so the additional&& wsIdcondition is redundant.♻️ Simplified condition
- {canManageList && wsId && ( + {canManageList && ( <BoardSelector open={isMoveAllDialogOpen}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx` around lines 440 - 451, The rendering condition for BoardSelector redundantly checks wsId even though canManageList is already computed as Boolean(wsId && boardId); remove the extra && wsId so the JSX uses only canManageList to gate rendering. Update the conditional around BoardSelector (the block that uses props wsId, boardId, listId, tasks, handleBulkMove, moveAllTasksFromListMutation) to rely solely on canManageList, ensuring BoardSelector still receives wsId as a prop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/api/v1/workspaces/`[wsId]/task-drafts/[draftId]/convert/route.ts:
- Around line 77-79: Validate the draftId path param with the shared zod UUID
schema before using it in DB calls: call the shared zod schema (z.guid()) and
safeParse on the extracted draftId from params (where draftId is read alongside
rawWsId and createClient is called) and return a 400 response if validation
fails; then use the validated value in the supabase query (the place that does
eq('id', draftId) / claim delete) and apply the same validation for the later
block around the code referenced at 144-151 to avoid Postgres 500s.
In `@packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx`:
- Around line 215-236: The three handlers handleDelete, handleUpdate, and
handleArchiveAllTasks are declared async but only call .mutate()
(fire-and-forget) so the async keyword is unnecessary; remove async from the
function declarations for handleDelete, handleUpdate, and handleArchiveAllTasks
in list-actions.tsx and keep the existing logic (including the early return in
handleUpdate and the setIsArchiveDialogOpen(false) path) so they remain
synchronous and return void.
In `@packages/utils/src/task-helper/sort-keys.ts`:
- Around line 222-248: The current branch assigns tasks = visualOrderTasks and
persists sort_key values directly from that array, which may include
filtered/sorted views; instead always base persistence on the canonical active
task list returned by listAllActiveTasksForList and only use visualOrderTasks as
a temporary UI ordering map. Concretely, in the visualOrderTasks !== undefined
branch (and the similar logic at 263-266) remove the early return/assignment to
tasks and call listAllActiveTasksForList(wsId, listId) to produce the canonical
tasks array (mapping to {id, sort_key, created_at}); then use visualOrderTasks
only to compute new positions for items present in that canonical list (skip or
ignore hidden/filtered items) before updating sort_key so you never persist
order from a filtered/sorted visual view.
In `@packages/utils/src/task-helper/task-operations.ts`:
- Around line 205-216: moveTask is non-atomic: it calls updateWorkspaceTask,
then syncTaskArchivedStatus, then getWorkspaceTask so a mid-way failure leaves
the task moved but callers see an error. Fix by combining the two writes into a
single atomic update — either extend updateWorkspaceTask (or add a new API like
updateWorkspaceTaskWithArchive) to accept both list_id and the archived/status
changes and perform them in one server-side transaction, then call that single
method from moveTask and remove the separate syncTaskArchivedStatus call; keep
the final getWorkspaceTask only after the single successful update.
- Around line 162-185: The code updates only completed_at/closed_at timestamps
but never updates the task.completed boolean, causing out-of-sync state; modify
the same update object in task-operations.ts (the updates variable used
alongside list.status, task.completed_at and task.closed_at) to set
updates.completed = true when list.status === 'done' (mirroring when you set
completed_at) and set updates.completed = false when list.status === 'closed' or
in the else branch (mirroring when you null completed_at/closed_at), ensuring
the boolean is toggled in the same conditional branches that update
completed_at/closed_at so state stays consistent.
- Around line 82-88: The helper currently enqueues embeddings twice:
createWorkspaceTask() (the workspace-task POST handler) already triggers
embedding generation after DB insert, so the extra call to
triggerWorkspaceTaskEmbedding(wsId, createdTask.id, options) in
createTask/createWorkspaceTask helper causes duplicates; remove this
unconditional triggerWorkspaceTaskEmbedding invocation (or add a clear boolean
option like skipEmbedding and check it before calling) so only the POST handler
enqueues embeddings; update references to createdTask, createWorkspaceTask and
triggerWorkspaceTaskEmbedding accordingly.
---
Outside diff comments:
In `@packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx`:
- Around line 440-451: The rendering condition for BoardSelector redundantly
checks wsId even though canManageList is already computed as Boolean(wsId &&
boardId); remove the extra && wsId so the JSX uses only canManageList to gate
rendering. Update the conditional around BoardSelector (the block that uses
props wsId, boardId, listId, tasks, handleBulkMove,
moveAllTasksFromListMutation) to rely solely on canManageList, ensuring
BoardSelector still receives wsId as a prop.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8e5e1ebd-e375-46f9-9b53-a78481601b5c
📒 Files selected for processing (10)
apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.tsapps/web/src/app/api/v1/workspaces/[wsId]/tasks/[taskId]/embedding/route.tspackages/apis/src/tu-do/tasks/route.tspackages/internal-api/src/tasks.tspackages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsxpackages/ui/src/components/ui/tu-do/shared/list-view.tsxpackages/utils/src/task-helper/relationships.tspackages/utils/src/task-helper/sort-keys.tspackages/utils/src/task-helper/task-hooks-basic.tspackages/utils/src/task-helper/task-operations.ts
apps/web/src/app/api/v1/workspaces/[wsId]/task-drafts/[draftId]/convert/route.ts
Show resolved
Hide resolved
packages/ui/src/components/ui/tu-do/boards/boardId/list-actions.tsx
Outdated
Show resolved
Hide resolved
vhpx
approved these changes
Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Refactored task helpers into modular utilities and moved all task operations to
@tuturuuu/internal-apiwith workspace-scoped calls. Added status template API, identifier search/counts, stronger deleted-task handling, and hardened embedding/draft conversion, plus translation parity across apps.Refactors
@tuturuuu/utils/task-helperinto focused modules:board,bulk-actions,recycle-bin,relationships,shared,sort-keys,task-hooks-basic,task-hooks-move,task-operations.@tuturuuu/internal-api(e.g.,createWorkspaceTask,updateWorkspaceTask,moveWorkspaceTask,triggerWorkspaceTaskEmbedding).wsIdthrough hooks and UI (e.g.,useBoardConfig(boardId, wsId),useKanbanDnd({ wsId, ... }), bulk ops, quick-create,TaskMentionChip,TaskActions).useDeletedTasks(boardId, wsId, ...);RecycleBinPanelacceptswsId.listWorkspaceTasksaddsidentifier,includeDeleted(none|only|all), andincludeCount; response may includecount. Added GET/api/v1/task-board-status-templatesand exportedlistTaskBoardStatusTemplates.wsId, uses admin client; draft conversion normalizeswsIdand relation IDs.archived_tasks_partially,archived_tasks_successfully,moved_tasks_partiallyacross apps; enforced “Shared UI Translation Parity” inAGENTS.md.Migration
wsIdto:createTask(wsId, listId, data),useBoardConfig(boardId, wsId),useKanbanDnd({ wsId, ... }),useUpdateTask(boardId, wsId),useMoveTask(boardId, wsId),useDeletedTasks(boardId, wsId, options), andRecycleBinPanelvia newwsIdprop.@tuturuuu/internal-apiand the new@tuturuuu/utils/task-helper/*modules.wsId: useuseCreateTaskRelationship(wsId)anduseDeleteTaskRelationship(wsId)(dropboardId).Written for commit d86c16c. Summary will update on new commits. Review in cubic