feat(ui): task archive, restore, and admin hard-delete modal (SPEC-004, Task 84bf3f12) - #112
Conversation
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 817b16cb68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| creationSource: CreationSource | null; | ||
| }>) => req<Task>(`/api/tasks/${id}`, { method: 'PUT', body: JSON.stringify(data) }); | ||
| archived: boolean; | ||
| }>) => req<Task>(`/api/tasks/${id}`, { method: 'PATCH', body: JSON.stringify(data) }); |
There was a problem hiding this comment.
Changing the shared updateTask request to PATCH breaks every existing edit flow, including title, description, assignment, and scheduling saves. The server in backend/src/routes/tasks.ts:463 only registers PUT /:id and has no PATCH /:id handler, so calls routed through useTasks.updateTask now receive a 404 instead of persisting changes; retain PUT for ordinary updates or add the corresponding server route before switching this client.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7c16936: ordinary updateTask is back on PUT /api/tasks/:id. Archive/restore uses a dedicated PATCH that only accepts { archived }.
| export const archiveTask = (id: string) => | ||
| req<Task>(`/api/tasks/${id}`, { method: 'PATCH', body: JSON.stringify({ archived: true }) }); | ||
| export const restoreTask = (id: string) => | ||
| req<Task>(`/api/tasks/${id}`, { method: 'PATCH', body: JSON.stringify({ archived: false }) }); |
There was a problem hiding this comment.
Both archive helpers target a route that does not exist in this commit: backend/src/routes/tasks.ts has no PATCH handler, its update column map has no archive field, and the database migrations contain no archived_at columns. Consequently, clicking Archive or Restore always fails rather than changing task state; the server route and schema migration need to accompany these client calls.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7c16936: added archived_at / archived_by / archived_by_id plus PATCH /api/tasks/:id archive/restore. Default list queries exclude archived tasks unless includeArchived=true.
| onClick={() => onTaskOpen ? onTaskOpen(task) : setSelectedTask(task)} | ||
| onDragStart={e => handleDragStart(e, task.id)} | ||
| onArchive={handleArchiveCard} | ||
| onDelete={task => setDeleteTaskTarget(task)} |
There was a problem hiding this comment.
The board passes onDelete to every card without checking the current user's role, and the task-detail danger zone likewise renders its delete button unconditionally. This is not merely cosmetic: the current DELETE /:id handler in backend/src/routes/tasks.ts:582-600 has no admin authorization check, so a non-admin user can click this newly exposed action and permanently delete a task. Gate the control and, critically, enforce the admin requirement in the server handler.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7c16936: DELETE /api/tasks/:id requires admin (or auth disabled). The card and detail delete controls only render when canHardDeleteTasks() is true.
| await api.deleteTask(deleteTaskTarget.id, reason); | ||
| showToast('Task permanently deleted'); | ||
| // Trigger update/removal callback | ||
| onUpdateTask(deleteTaskTarget.id, { _deleted: true } as any); |
There was a problem hiding this comment.
After a successful hard-delete, this invokes the normal update callback with an unsupported _deleted field instead of removing the task locally. In the only KanbanBoard caller, onUpdateTask maps to useTasks.updateTask, so it sends another API update (which either 404s under the current verb mismatch or returns No fields to update) and leaves the now-nonexistent task card visible until a full refetch; use the existing delete/removal state path or a dedicated local mutation callback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7c16936: hard-delete now calls useTasks.deleteTask, which removes the card from local board state. It no longer sends a fake _deleted update.
| <textarea | ||
| value={reason} | ||
| onChange={(e) => setReason(e.target.value)} | ||
| placeholder="Provide a mandatory reason for hard deleting this task (e.g. duplicate creation, test cleanup)..." | ||
| rows={3} | ||
| maxLength={500} | ||
| required |
There was a problem hiding this comment.
For every hard-delete, any nonempty reason enables submission, so the modal omits the destructive-action safeguard required by docs/specs/SPEC-004-archive-delete-affordance.md:166-180: the user must type the task title and the delete button must remain disabled until it matches case-insensitively. Keep the audit reason, but add the separate title-confirmation gate so an accidental click cannot immediately remove the task.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7c16936: DeleteTaskModal requires a case-insensitive title confirmation and keeps the 1-500 character audit reason. Delete stays disabled until both match.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Keep ordinary task updates on PUT. Add PATCH { archived } plus
archived_* columns, exclude archived tasks from default lists,
require an admin + title confirmation + audit reason for delete,
and remove deleted cards from board state.
1 participant
Summary