Import .bloomSource files into a collection, with duplicate-by-id handling (BL-16502)#8029
Draft
hatton wants to merge 12 commits into
Draft
Import .bloomSource files into a collection, with duplicate-by-id handling (BL-16502)#8029hatton wants to merge 12 commits into
hatton wants to merge 12 commits into
Conversation
…dling (BL-16502) Adds a collection-screen command to import one or more .bloomSource (and legacy .bloom) files into the current editable collection, either as editable books or as new derivatives. When an imported book's bookInstanceId already exists in the collection, the user is prompted to replace the existing book or add an independent numbered copy (new id). - CollectionModel: import/extract/validate/duplicate-resolution pipeline; a pre-flight AnyBloomSourceIsAlreadyInCollection check drives the dialog. - CollectionApi: chooseBloomSourceFilesToImport / anyChosenBloomSourceAlreadyInCollection / importBloomSource endpoints. - CollectionsTabPane + RadioChoiceDialog: edit-vs-derivative and replace-vs-addCopy dialogs. - BloomOpenFileDialog: multi-select support for the import picker. Tests (CollectionModelTests) cover valid import, duplicate detection keyed on bookInstanceId (proving same-title/different-id is NOT a duplicate and same-id/different-title IS), replace/add-copy/cancel resolution, and the malformed-file guards. Fixed FakeCollectionModel so GetBookInfos yields real BookInfos (a null BookSelection was turning every book into an ErrorBookInfo, which silently defeated id-based duplicate detection in tests). Preflight review fixes: - Stage extraction on the collection's volume so the move into the collection can't fail cross-volume (system temp is often on a different drive). - In the Replace case, recycle the existing book only after the imported book is safely in place, so a mid-import failure can't lose the original. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
| Filename | Overview |
|---|---|
| src/BloomExe/CollectionTab/CollectionModel.cs | Adds ~650 lines implementing the full import pipeline: file picker, zip validation (zip-slip guard, Bloom Pack shape detection), extraction to same-volume staging, duplicate-by-bookInstanceId resolution (Replace/AddCopy/Cancel), within-batch collision tracking via a HashSet, and post-import cleanup safely isolated in its own try/catch after the successful move. |
| src/BloomBrowserUI/collectionsTab/CollectionsTabPane.tsx | Adds the "Import .bloomSource File(s)" menu item and wires up the two-dialog flow: the server picks files first, then the UI calls bloomSourceImportDuplicateInfo to choose which dialog to show (edit/derivative when no duplicates, replace/add-copy when any exist). |
| src/BloomBrowserUI/collectionsTab/RadioChoiceDialog.tsx | New reusable dialog component offering mutually-exclusive radio choices with OK (disabled until a choice is made) and Cancel; resets choice state via useEffect when reopened, with an appropriate justification comment. |
| src/BloomExe/web/controllers/CollectionApi.cs | Registers three new endpoints: chooseBloomSourceFilesToImport (blocking file picker), bloomSourceImportDuplicateInfo (pre-flight duplicate/replaceability check), and importBloomSource (async, runs behind progress dialog). |
| src/BloomTests/CollectionTab/CollectionModelTests.cs | Adds ~530 lines of thorough import tests covering valid import, duplicate ID (add-copy, replace, cancel), same-title/different-id non-duplicate, same-id/different-title duplicate, within-batch collision, pre-flight duplicate checks, checkout guard, corrupt file, no-book-inside, Bloom-Pack shape, and zip-slip protection. |
| src/BloomTests/TestDoubles/CollectionTab/FakeCollectionModel.cs | Fixed two bugs: null BookSelection was turning every enumerated book into an ErrorBookInfo (breaking id-based duplicate detection), and null WebSocketServer would NullReferenceException when a book is added/removed during import. |
| src/BloomExe/MiscUI/BloomOpenFileDialog.cs | Extended to support multi-select: the FileOk filter-check now loops over all selected files (not just the first), and a FileNames property is exposed to callers. |
| src/BloomExe/Book/BookStorage.cs | Minor defensive fix: caps the instance-ID suffix at 8 characters rather than throwing if the ID (possibly from a malformed imported book) is shorter than 8 characters. |
| src/BloomBrowserUI/react_components/muiRadio.tsx | Adds an optional description prop rendered as a smaller, muted secondary line below the radio label, and adds disabled prop forwarding to FormControlLabel. |
| DistFiles/localization/en/Bloom.xlf | Adds 13 correctly-marked translate="no" string entries for all UI surfaces of the import feature. |
| src/BloomBrowserUI/react_components/icons/ImportIcon.tsx | New SVG icon component for the import menu item; uses currentColor so it inherits menu text color. |
Reviews (9): Last reviewed commit: "Disable "Replace" for un-checked-out Tea..." | Re-trigger Greptile
…6502) - ValidateBloomSourceZip: dispose the ZipFile via `using` instead of a finally/Close (matches ReadBookInstanceIdFromBloomSource). - RadioChoiceDialog: use `useState<string>()` per src/BloomBrowserUI/AGENTS.md rather than the explicit `useState<string | undefined>(undefined)` form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Following review feedback: 1. Import no longer blocks the UI thread. importBloomSource is now an async, non-UI-thread endpoint that runs the batch behind the collection tab's existing EmbeddedProgressDialog (per-file progress), so a large batch neither freezes the collection screen nor lets the browser request time out. SelectBook is marshaled back to the UI thread. 2. "Add a copy" now reuses the Duplicate Book convention instead of a parallel scheme: the copy gets a new id and the "<name> - Copy-<id>" folder name, and the title is left unchanged (as Duplicate does). Removed the hand-rolled PrefixCaptionWithUniqueNumber / ComputeUniqueCaptionNumber and the addNumberPrefix plumbing. 3. The derivative-import path now mirrors the normal "make a book from this source" flow (CreateFromSourceBook): defer the Created history event via PendingCreationSource and add the new book in memory, instead of an immediate history event plus a reload-and-lookup with a silent not-found fallback. 5. GetUniqueBookFolderName no longer throws on an imported book carrying a malformed/short instance id (guarded the Substring). 6. The pre-flight duplicate check reads the id with BookMetaData (same parser as the import), so it can't disagree with the import about a book's id. Tests updated for the new add-copy behavior (folder-name convention, no caption renumbering) and the removed caption helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (BL-16502) New ImportIcon (arrow-into-a-page glyph, from the provided design SVG) rendered via MUI SvgIcon with currentColor so it follows the menu text color. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…L-16502) - ImportBloomSourceFileToCollectionFolder: guard the catch-block cleanup with an importSucceeded flag so that a failure while recycling the replaced duplicate (in the Replace path) can no longer delete the book that was just successfully imported. - Move the misplaced ImportBloomSourceFiles doc comment down onto that method (the async wrapper had two summaries and the real method had none). - Add the missing CollectionTab.ImportBloomSource.Importing XLF entry for the import progress-dialog title (previously only had a code fallback). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…502) ImportBloomSourceFiles previously called ReloadEditableCollection() inside the per-file helper (ImportOneBloomSourceFile), so importing N files re-scanned the whole collection from disk N times (O(files x collection size)). Restructure the edit path to move every book into place first, then reload exactly once and turn each imported folder into a Book with its "Imported from" history event. The derivative path was already reload-free (it adds its Book in memory) and is unchanged. Removes the now-unused ImportOneBloomSourceFile; the file-system method the tests drive (ImportBloomSourceFileToCollectionFolder) is untouched. Addresses preflight decision B on PR #8029. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… (BL-16502)
Greptile P1: in Replace mode, once the imported book is moved into place the
import has succeeded. Previously the post-import cleanup (deselect, recycle the
old book, drop it from the collection) ran in the main try block, so if any step
threw (locked folder, recycle bin unavailable on a network drive, etc.) the
exception bubbled up as a generic "Bloom was not able to import" error even
though the book was imported — and because the method threw instead of returning
the destination folder, that folder was never reloaded, so the imported book
stayed invisible until a manual refresh.
Wrap the recycle cleanup in its own try/catch that reports a NonFatalProblem
("imported, but couldn't remove the older copy") and still returns the
destination folder, so the import is reported accurately and the book shows up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When two .bloomSource files selected together carry the same bookInstanceId, the collection is only reloaded once (after the whole batch), so the per-file duplicate check can't see a book imported earlier in the same batch. Track the ids imported so far in the batch and treat a within-batch repeat as an independent copy (fresh id, " - Copy-" folder), the same as the AddCopy case. Refactors the AddCopy id/folder logic into a shared makeIndependentCopy path and adds a unit test plus the missing param doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…16502)
Show a single import dialog chosen up front by whether any book is already in
the collection: edit-vs-derivative when none are present, replace-vs-add-copy
when any are. Refresh the dialog copy ("Import books", shorter option labels)
and give the replace/add-copy options an explanatory secondary line. Switch
RadioChoiceDialog from raw MUI Radio to the Bloom MuiRadio component, extending
MuiRadio with an optional already-localized description line.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…16502) Gives the derivative radio option the same secondary-line treatment as the replace/add-copy options, explaining that original credits are preserved and attributed to the source book. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(BL-16502)
When importing a .bloomSource whose book is already in a Team Collection, the
existing book can only be recycled if it is checked out here. The collection
screen now asks C# (via the new bloomSourceImportDuplicateInfo endpoint) both
whether any chosen book is a duplicate and whether every duplicate is currently
replaceable; when any is not checked out it disables the "Replace" choice and
explains why ("All books must be checked out"). ImportBloomSourceFileToCollectionFolder
also guards the Replace path (throws if !IsSaveable) so a not-checked-out book
can never be deleted even if the UI guard is bypassed. Adds RadioChoice/MuiRadio
"disabled" support and five unit tests.
Also documents (per review) why we don't offer edit-vs-derivative when a book is
already in the collection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
[Claude Opus 4.8] Draft opened by an automated preflight run.
What
Adds a collection-screen command to import one or more
.bloomSource(and legacy.bloom) files into the current editable collection — either as editable books or as new derivatives. When an imported book'sbookInstanceIdalready matches a book already in the collection, the user is prompted to replace the existing book or add an independent numbered copy (which gets a fresh id).Duplicate detection is keyed on
bookInstanceId, not title/folder name — significant because a shared id causes real bugs and controls re-upload to the Bloom library.Notable pieces
CollectionModel: import / extract / validate / duplicate-resolution pipeline; a pre-flightAnyBloomSourceIsAlreadyInCollectioncheck decides whether to show the duplicate dialog.CollectionApi:chooseBloomSourceFilesToImport/anyChosenBloomSourceAlreadyInCollection/importBloomSourceendpoints.CollectionsTabPane+RadioChoiceDialog: edit-vs-derivative and replace-vs-addCopy dialogs.BloomOpenFileDialog: multi-select for the import picker.Tests
CollectionModelTestscovers valid import, duplicate detection keyed onbookInstanceId(same-title/different-id is NOT a duplicate; same-id/different-title IS), replace/add-copy/cancel resolution, and malformed-file guards. Also fixedFakeCollectionModel(a nullBookSelectionwas turning every enumerated book into anErrorBookInfo, silently defeating id-based duplicate detection in tests).Preflight review fixes included
Ref: https://issues.bloomlibrary.org/issue/BL-16502
🤖 Generated with Claude Code
Devin review
This change is