Skip to content

fix: fail cleanly on empty manual track selection (closes #173) - #186

Merged
negativeeddy merged 1 commit into
masterfrom
fix/issue-#173
Sep 5, 2026
Merged

fix: fail cleanly on empty manual track selection (closes #173)#186
negativeeddy merged 1 commit into
masterfrom
fix/issue-#173

Conversation

@negativeeddy

Copy link
Copy Markdown
Owner

Automated Fix

Fixes #173

Problem

Deselecting all tracks in Manual Selection mode submits ManualSelectionTrackNumbers = "[]" (a non-empty string), so the apply block runs, sets every Process=false, and the rip loop has zero eligible tracks. The post-rip check then throws the confusing "MakeMKV rip produced no ripped tracks" error and the job fails.

Changes

  • API (ApiController.SubmitManualSelection): reject an empty trackNumbers array with a clear error. The job stays parked in ManualSelectionStarted so the user can correct the selection and resubmit (or cancel the job).
  • Pipeline (ArmRipperService): defensive check — if an empty selection somehow reaches the pipeline, fail the job cleanly with "No tracks selected" instead of the confusing downstream error.
  • UI (JobDetail.cshtml): client-side guard in submitManualSelection() that alerts immediately when no tracks are checked, avoiding a round-trip.

Testing

  • New test ManualSelection_EmptySelection_FailsJobCleanly (pipeline fails cleanly, no rip runs)
  • New test SubmitManualSelection_EmptySelection_RejectsAndKeepsJobWaiting (API rejects, job stays waiting)
  • All existing tests pass (Core: 294, WebUi: 80)

@negativeeddy negativeeddy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and approved. The fix matches the issue's proposed fix: the API rejects empty selections (job stays parked for correction), the pipeline fails cleanly with a clear 'No tracks selected' message as a defensive guard, and the UI alerts client-side. Build passes (0 warnings/errors); all tests pass (Core 294, WebUi 80) including two new regression tests. Ready for merge.

@negativeeddy
negativeeddy merged commit 1b4563d into master Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Medium: Empty manual selection (deselect all tracks) fails the job with a confusing error

1 participant