fix: add unarchive support for archived tasks#1894
fix: add unarchive support for archived tasks#1894ahmed-hassan19 wants to merge 3 commits intoAndyMik90:developfrom
Conversation
📝 WalkthroughWalkthroughAdds unarchive support: exported Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskCard
participant TaskStore
participant IPC as electronAPI
participant Backend
User->>TaskCard: Click "Unarchive"
TaskCard->>TaskCard: handleUnarchive()
TaskCard->>TaskStore: unarchiveTasks(projectId, [taskId])
TaskStore->>IPC: window.electronAPI.unarchiveTasks(...)
IPC->>Backend: RPC unarchiveTasks
Backend-->>IPC: success
IPC-->>TaskStore: response
TaskStore->>TaskStore: loadTasks(projectId)
TaskStore-->>TaskCard: tasks reloaded / updated
TaskCard->>User: UI reflects unarchived task
sequenceDiagram
participant User
participant KanbanBoard
participant TaskStore
participant IPC as electronAPI
participant Backend
User->>KanbanBoard: Change task status (drag/drop)
KanbanBoard->>TaskStore: apply status change
TaskStore-->>KanbanBoard: status update success
note over KanbanBoard,TaskStore: if task.metadata.archivedAt && projectId
KanbanBoard->>TaskStore: unarchiveTasks(projectId, [taskId])
TaskStore->>IPC: window.electronAPI.unarchiveTasks(...)
IPC->>Backend: RPC unarchiveTasks
Backend-->>IPC: success
IPC-->>TaskStore: response
TaskStore->>TaskStore: loadTasks(projectId)
TaskStore-->>KanbanBoard: refreshed
KanbanBoard->>User: UI updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ahmed-hassan19, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances task management by providing two intuitive ways to unarchive tasks. Users can now explicitly unarchive tasks via a dedicated button on archived task cards or implicitly by dragging an archived task from the 'Done' column back into an active workflow state. These changes improve the flexibility and user experience for managing task lifecycles within the application. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to unarchive tasks, both through a manual button and automatically when a task is moved from the 'Done' column. The implementation is sound, covering both UI and state management. My feedback focuses on improving performance by optimizing state updates to avoid full data reloads, and enhancing robustness by adding user-facing error handling for a more resilient user experience.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/KanbanBoard.tsx (1)
32-32:⚠️ Potential issue | 🔴 Critical
unarchiveTasksis called on line 992 but missing from the import on line 32 — TypeScript compile error.
unarchiveTasksis not present in thetask-storeimport on line 32. This causes a compile-time error (unarchiveTasks is not defined) and breaks the build.🐛 Fix: add `unarchiveTasks` to the import
-import { persistTaskStatus, forceCompleteTask, archiveTasks, deleteTasks, useTaskStore, isQueueAtCapacity, DEFAULT_MAX_PARALLEL_TASKS } from '../stores/task-store'; +import { persistTaskStatus, forceCompleteTask, archiveTasks, unarchiveTasks, deleteTasks, useTaskStore, isQueueAtCapacity, DEFAULT_MAX_PARALLEL_TASKS } from '../stores/task-store';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/renderer/components/KanbanBoard.tsx` at line 32, The build fails because unarchiveTasks is used in KanbanBoard.tsx but not imported; update the existing import statement that currently lists persistTaskStatus, forceCompleteTask, archiveTasks, deleteTasks, useTaskStore, isQueueAtCapacity, DEFAULT_MAX_PARALLEL_TASKS to also include unarchiveTasks from '../stores/task-store'; if unarchiveTasks is not exported from task-store, export it there so the import resolves.
🤖 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/frontend/src/renderer/components/KanbanBoard.tsx`:
- Around line 989-993: The call to unarchiveTasks is awaited but its
non-exception failure path is ignored, leaving the task moved but still
archived; update the block where unarchiveTasks(projectId, [taskId]) is called
(inside the auto-unarchive branch that checks result.success &&
task?.metadata?.archivedAt && projectId) to capture the returned value, check if
returned.success is false, and log an error (including projectId, taskId and the
returned response) — you can mirror handleArchiveAll’s behavior (console.error)
and optionally surface a user-visible notification if unarchive fails.
---
Outside diff comments:
In `@apps/frontend/src/renderer/components/KanbanBoard.tsx`:
- Line 32: The build fails because unarchiveTasks is used in KanbanBoard.tsx but
not imported; update the existing import statement that currently lists
persistTaskStatus, forceCompleteTask, archiveTasks, deleteTasks, useTaskStore,
isQueueAtCapacity, DEFAULT_MAX_PARALLEL_TASKS to also include unarchiveTasks
from '../stores/task-store'; if unarchiveTasks is not exported from task-store,
export it there so the import resolves.
Add an "Unarchive" button on archived task cards so users can restore them directly. Includes the unarchiveTasks store action, ArchiveRestore icon, and i18n keys for both English and French. Co-Authored-By: Claude Opus 4.6 <[email protected]>
When an archived task is dragged from Done to another column (backlog, queue, in_progress), automatically unarchive it — moving an archived task is a clear signal the user wants it active again. Co-Authored-By: Claude Opus 4.6 <[email protected]>
ecacbc3 to
1378198
Compare
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/frontend/src/renderer/components/KanbanBoard.tsx`:
- Around line 989-993: The call to unarchiveTasks discards its result so
non-exceptional failures leave task.archivedAt set while the UI shows it active;
update the block where you call unarchiveTasks (the code that checks
result.success and task?.metadata?.archivedAt with projectId and taskId) to
capture the returned value, verify its success flag, and on failure log an error
(e.g., console.error with context: taskId/projectId and the returned result) and
optionally revert the UI change or surface an error to the user to avoid
inconsistent state.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/frontend/src/renderer/components/KanbanBoard.tsx`:
- Around line 989-993: The auto-unarchive call to unarchiveTasks in
KanbanBoard.tsx is unchecked and can silently fail; update the block that runs
when result.success && task?.metadata?.archivedAt && projectId to capture the
return value of unarchiveTasks(taskId array) (e.g., const unarchiveResult =
await unarchiveTasks(projectId, [taskId])), check unarchiveResult.success, and
on failure mirror handleArchiveAll’s behavior by logging the error and/or
surfacing a user-facing error (toast or similar) and, if appropriate, revert the
UI change so the archived badge remains consistent.
Address PR review feedback: check unarchiveTasks() return value in KanbanBoard and show a toast on failure, and replace console.error with a toast in TaskCard since console output is invisible in bundled Electron apps. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/KanbanBoard.tsx (1)
260-266:⚠️ Potential issue | 🟡 MinorInconsistency:
handleArchivestill usesconsole.errorinstead of a toast.The new
handleUnarchive(in TaskCard, line 268-278) and the auto-unarchive block (line 993-999) both surface failures via toasts, buthandleArchive(line 263-265) logs toconsole.error. In bundled Electron apps,console.erroris invisible to users. This was also noted in the commit message as the intended fix direction.♻️ Suggested fix: surface archive failure via toast
const handleArchive = async (e: React.MouseEvent) => { e.stopPropagation(); const result = await archiveTasks(task.projectId, [task.id]); if (!result.success) { - console.error('[TaskCard] Failed to archive task:', task.id, result.error); + toast({ + title: t('common:errors.operationFailed'), + description: result.error, + variant: 'destructive', + }); } };Note: this pre-existing
console.erroris in thehandleArchivedefined at lines 260-266 ofTaskCard.tsx, notKanbanBoard.tsx. I'm flagging it here for visibility since it's the same pattern the PR is fixing elsewhere. As per coding guidelines: "Never useconsole.logfor debugging production issues in the Electron app. Use Sentry for error tracking and diagnostics in production."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/renderer/components/KanbanBoard.tsx` around lines 260 - 266, The handleArchive function in TaskCard.tsx currently calls console.error on failure; change it to surface errors via the same toast used by handleUnarchive (e.g., toast.error with a user-friendly message) and also report the error to Sentry (e.g., Sentry.captureException) or the app’s error-reporting utility, mirroring the pattern in handleUnarchive and the auto-unarchive block so failures are visible to users and tracked in production.
🤖 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/frontend/src/renderer/components/TaskCard.tsx`:
- Around line 268-278: The toast on failure in handleUnarchive currently uses
the action label t('tasks:actions.unarchive') as the title; change it to a
generic error heading (use t('common:errors.operationFailed')) so the toast
reads like an error (update the toast call inside handleUnarchive that follows
the unarchiveTasks call and references toast, t, and result to use the common
error key instead of the action label).
---
Outside diff comments:
In `@apps/frontend/src/renderer/components/KanbanBoard.tsx`:
- Around line 260-266: The handleArchive function in TaskCard.tsx currently
calls console.error on failure; change it to surface errors via the same toast
used by handleUnarchive (e.g., toast.error with a user-friendly message) and
also report the error to Sentry (e.g., Sentry.captureException) or the app’s
error-reporting utility, mirroring the pattern in handleUnarchive and the
auto-unarchive block so failures are visible to users and tracked in production.
---
Duplicate comments:
In `@apps/frontend/src/renderer/components/KanbanBoard.tsx`:
- Around line 989-1000: The auto-unarchive code correctly checks result.success
and shows a destructive toast on failure; no code changes needed. Ensure the
block using result.success, task?.metadata?.archivedAt, projectId,
unarchiveTasks(projectId, [taskId]) and toast(...) remains as-is and that
unarchiveResult.error is a user-friendly string; if not, convert or localize the
error before passing it to toast.
| const handleUnarchive = async (e: React.MouseEvent) => { | ||
| e.stopPropagation(); | ||
| const result = await unarchiveTasks(task.projectId, [task.id]); | ||
| if (!result.success) { | ||
| toast({ | ||
| title: t('tasks:actions.unarchive'), | ||
| description: result.error, | ||
| variant: 'destructive', | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Toast title on failure uses the action label instead of an error message.
handleUnarchive uses t('tasks:actions.unarchive') (which resolves to "Unarchive") as the toast title on failure. This reads oddly as a toast heading — the user sees "Unarchive" with an error description below. For consistency with the KanbanBoard auto-unarchive toast (line 995), consider using t('common:errors.operationFailed').
♻️ Suggested fix
if (!result.success) {
toast({
- title: t('tasks:actions.unarchive'),
+ title: t('common:errors.operationFailed'),
description: result.error,
variant: 'destructive',
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/renderer/components/TaskCard.tsx` around lines 268 - 278,
The toast on failure in handleUnarchive currently uses the action label
t('tasks:actions.unarchive') as the title; change it to a generic error heading
(use t('common:errors.operationFailed')) so the toast reads like an error
(update the toast call inside handleUnarchive that follows the unarchiveTasks
call and references toast, t, and result to use the common error key instead of
the action label).
Base Branch
developbranch (required for all feature/fix PRs)Description
Adds unarchive support for archived tasks: an "Unarchive" button on archived task cards, and auto-unarchive when dragging archived tasks from Done to another column. Includes error handling with toast notifications and i18n keys for English and French.
Type of Change
Area
AI Disclosure
Tool(s) used: Claude Code (Claude Opus 4.6)
Testing level:
Untested -- AI output not yet verified
Lightly tested -- ran the app / spot-checked key paths
Fully tested -- all tests pass, manually verified behavior
I understand what this PR does and how the underlying code works
Checklist
developbranchPlatform Testing Checklist
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)If you only have access to one OS: CI now tests on all platforms. Ensure all checks pass before submitting.
CI/Testing Requirements
Test Plan
Feature Toggle
Breaking Changes
Breaking: No
🤖 Generated with Claude Code