-
Notifications
You must be signed in to change notification settings - Fork 59
feat(tasks): implement bulk task actions (complete/delete) #225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(tasks): implement bulk task actions (complete/delete) #225
Conversation
93f0aa5 to
6014972
Compare
its-me-abhishek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the current behavior might work, there are a couple of edge cases worth considering:
- If a task is already completed or deleted, and the user selects it again for deletion, the operation becomes a false positive-- the task is already gone, so it only adds unnecessary load on the server.
- Marking a deleted task as completed would also require switching its status back first, which creates an avoidable extra step.
6014972 to
a5a3942
Compare
|
Hey @its-me-abhishek, thanks for the feedback! I've addressed both edge cases you mentioned:
This prevents unnecessary server load from re-completing already completed tasks or attempting operations on deleted tasks. Let me know if you'd like any adjustments! |
|
Marking as draft for now, will re review once #236 gets merged |
- Added checkboxes to task rows and "Select All" in header - Implemented floating action panel for selected tasks - Added backend endpoints for bulk complete and delete - Updated frontend state to track selected UUIDs - Added confirmation dialogs and loading states - Added comprehensive tests for bulk selection and actions - Fixes: CCExtractor#178
a5a3942 to
46274a9
Compare
|
Hi @its-me-abhishek , since #236 is merged, I've resolved the conflicts and the PR is now ready. Please review it once you get a chance! |
|
Hi @its-me-abhishek, since #236 is merged and the conflicts are resolved, could you please mark this PR as ready for review when you get a chance? Thanks! |
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created new endpoint for bulk task completion. uses a single job for all UUIDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a O(n) operation with lots of Config->Operate->Sync tasks, preferably what shall be done could be that- add another CMD operation for this called CompleteTasksInTaskwarrior, and then operate that loop there, instead of this API that initiates the Job. So they would have a single SetConfig->MultipleOperations->SingleSync as the final workflow. Similar shall be done for delete so as to reduce server overheads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@its-me-abhishek
Thanks for the suggestion! I understand, I'll refactor to move the loop into a single CompleteTasksInTaskwarrior function with the flow: SetConfig -> Loop operations -> SingleSync.
Quick question: for tracking which individual tasks fail within the batch, should I return a map[string]string (uuid -> error) or define a struct like FailedTask{UUID, Error}? Both work, just wondering if you have a preference.
frontend/src/components/HomeComponents/Tasks/__tests__/tasks-utils.test.ts
Show resolved
Hide resolved
ShivaGupta-14
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@its-me-abhishek self-review completed! ready for your review. (Note: As mentioned on Zulip, merging this before conflicting PRs would be helpful due to the filter-repo issue.)
Description
This PR introduces bulk task actions to CCSync, allowing users to efficiently manage multiple tasks at once. Users can now select multiple tasks via checkboxes and perform bulk operations like "Mark as Completed" or "Delete" through a floating action panel.
Key Enhancements
POST /complete-tasks) and bulk delete (POST /delete-tasks)Implementation Details
The bulk selection state is managed locally within the
Taskscomponent using aSetof UUIDs to ensure O(1) lookups and efficient updates.selectedTaskUUIDsstate tracks which tasks are currently selected.selectedTaskUUIDs.size > 0./complete-tasksand/delete-tasks.uuids.Fixes: #178
Checklist
npx prettier --write .(for formatting)gofmt -w .(for Go backend)npm test(for JS/TS testing)Additional Notes
Video: Click here to see
Screenshots: