fix(import): warn about existing-file conflict in Manual Import preview - #187
Open
jordanfelle wants to merge 1 commit into
Open
jordanfelle wants to merge 1 commit into
jordanfelle wants to merge 1 commit into
Conversation
…eview (Chaptarr#184) The Manual Import preview never checked whether the computed destination already had a tracked file at it - that check only ever ran at actual-import time, inside ImportApprovedBooks.Import() via GetAdditionalCopyPathCollisionReason. So a book that's already in the library under a different download showed a clean "Ready now (local)" status with no warning, in both Combine and Replace mode - the user had no way to know importing would either create a duplicate copy or silently overwrite the existing file until after clicking Import (Replace) or discovering the automatic pipeline had already blocked it (the automatic path does run this check, just never surfaces it in the preview a human would look at first). Adds IImportApprovedBooks.CheckExistingDestinationConflict, a read-only, single-item version of the same collision check the real import already runs, so ManualImportService.MapItem can call it during the preview and attach a Temporary/warning-severity Rejection when there's a conflict - informational only, doesn't block either mode, matches how other *arr apps surface this in their own Manual Import UI. Confirmed live: a book already in the library (previously imported under a different release) reproducibly showed "Ready now (local)" with zero warning in the modal, in both Combine and Replace mode, before this fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Manual Import preview modal shows a clean "Ready now (local)" status
with zero warning even when the book already has an existing tracked file
at the managed destination (via a different download) — in both
Combine and Replace mode.
This happens because the "does a file already occupy this book's managed
destination" collision check
(
ImportApprovedBooks.GetAdditionalCopyPathCollisionReason) is only everinvoked at actual import execution time
(
ImportApprovedBooks.Import()), never during the preview(
ManualImportService.GetMediaFiles→MapItem). So the preview can'twarn about something the real import will reject/overwrite a moment
later.
Fix
IImportApprovedBooks.CheckExistingDestinationConflict(...), anon-mutating, single-item version of the existing collision-check logic
(reuses the same destination-path computation and file-lookup — doesn't
duplicate it).
ManualImportService.MapItemnow calls it and, if a conflict is found,appends a
RejectionwithRejectionType.Temporary(
CanBypass = true, Warning severity) — informational only, doesn'tblock either Combine or Replace mode, matching how other *arr apps
surface this in their Manual Import UI.
?.) since some existing test fixtures constructManualImportServicewithoutIImportApprovedBookswired.Verified live against a real duplicate (a book already imported under a
different download) — the preview now shows the conflict warning in both
Combine and Replace mode, where before it showed neither.
Test plan
interface method's trivial stub implementations to the two
hand-written fixtures that construct
ImportApprovedBooksmanually, and after making the new call site null-safe.
running instance: preview now returns a
rejectionsentry("An existing file is already imported at the managed
destination: ...") in both
replaceExistingFiles=falseandreplaceExistingFiles=truemodes.