feat(queue): add bulk clear buttons (All/TV/Movies) and action using …#60
feat(queue): add bulk clear buttons (All/TV/Movies) and action using …#60TheMrClaus wants to merge 10 commits intonzbdav-dev:mainfrom
Conversation
…existing removeFromQueue;
There was a problem hiding this comment.
Pull Request Overview
This PR implements bulk clear functionality for the queue page, allowing users to clear all items, only TV shows, or only movies from their download queue with a single action.
- Adds three new buttons (Clear All, Clear TV, Clear Movies) with confirmation dialogs
- Implements backend logic to filter queue items by category and remove them in bulk
- Uses existing
removeFromQueueAPI in a loop to avoid creating new backend endpoints
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
frontend/app/routes/queue/route.tsx
Outdated
| console.error("removeFromQueue failed for", id, err); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Using a sequential for-loop to remove items one by one can be slow for large queues. Consider implementing a bulk removal API endpoint or using Promise.allSettled() to process removals concurrently.
| } | |
| const removalPromises = wanted.map(s => { | |
| const id = s?.nzo_id || s?.nzoId || s?.id; // support slight shape differences | |
| if (!id) return Promise.resolve(); | |
| return backendClient.removeFromQueue(id) | |
| .catch(err => { | |
| // Swallow per-item errors so one failure doesn't block the whole clear op | |
| console.error("removeFromQueue failed for", id, err); | |
| }); | |
| }); | |
| await Promise.allSettled(removalPromises); |
There was a problem hiding this comment.
Yes, this should be done in a single database operation, rather than multiple sequential round-trips.
The SABnzbd api spec allows for bulk deletions by passing multiple nzo_id values
But currently nzbdav assumes only a single nzo_id is given. It would need to be updated here to support multiple nzo_ids at once:
There was a problem hiding this comment.
Ty for the PR!
Typically, bulk operations on a table allow for more granular control for which rows will be acted upon, such as with Radarr's checkboxes. The "Select All" checkbox on the Table header can be used in conjunction with Filters to select which rows should be acted upon.
That's what I had in mind for Issue #55, but I'll merge this PR as a stop-gap until I can get around to implementing the filters and granular row selection :)
You can ignore most of my comments below, they're all optional. The one thing I will ask is to apply a proper bulk operation in the backend, so that items can be removed in a single call to the database rather than looping through each item and removing them one at a time
frontend/app/routes/queue/route.tsx
Outdated
| {/* Bulk-clear controls */} | ||
| <Form method="post" className="mb-3" onSubmit={(e) => { | ||
| const target = e.nativeEvent.submitter as HTMLButtonElement | null; | ||
| const action = target?.value ?? ""; | ||
| const label = action === "all" ? "all items" | ||
| : action === "tv" ? "all TV items" | ||
| : action === "movies" ? "all Movies items" | ||
| : "items"; | ||
| if (!window.confirm(`Are you sure you want to clear ${label} from the queue?`)) { | ||
| e.preventDefault(); | ||
| } | ||
| }}> | ||
| <input type="hidden" name="__intent" value="bulk-clear" /> | ||
| <ButtonGroup> | ||
| <Button variant="outline-danger" type="submit" name="clear" value="all"> | ||
| Clear All | ||
| </Button> | ||
| <Button variant="outline-warning" type="submit" name="clear" value="tv"> | ||
| Clear TV | ||
| </Button> | ||
| <Button variant="outline-primary" type="submit" name="clear" value="movies"> | ||
| Clear Movies | ||
| </Button> | ||
| </ButtonGroup> | ||
| </Form> |
There was a problem hiding this comment.
- [required] I think we may need to fix the indenting of these lines here.
- [optional] Since the buttons have different colors, would it make more sense to move them out of the button-group so they can have some spacing between them? The borders colors otherwise overlap each other.
- [optional] Also, maybe we could hide the buttons until the queue table shows up (until there are items in the queue)
- [optional] Is there any indication that work is being performed after clicking the button? Is there anything to prevent multiple clicks to the same button before the previous operation has finished?
Let me know your thoughts!
frontend/app/routes/queue/route.tsx
Outdated
| console.error("removeFromQueue failed for", id, err); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Yes, this should be done in a single database operation, rather than multiple sequential round-trips.
The SABnzbd api spec allows for bulk deletions by passing multiple nzo_id values
But currently nzbdav assumes only a single nzo_id is given. It would need to be updated here to support multiple nzo_ids at once:
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Remove duplicate bulk-clear logic - Fix missing closing braces causing unexpected EOF error - Restructure action function with proper control flow - Add error handling for bulk-clear operations - Separate bulk-clear and NZB upload logic for better organization
…SON body; single DB transaction
…o_id via SAB 'value') in single DB transaction
Very sensible comments, I believe I have now addressed all concerns and pushed my commits. I have implemented a single API call with multiple IDs to the DB. I could not verify with multiple items yet because they go very fast and I can't clear them fast enough. I couldn't get them to pile up like before. Next I will try to implement checkboxes. |
Adds Clear All, Clear TV, and Clear Movies to the Queue page.
Closes #55