Import .bloomSource files into a collection, with duplicate-by-id handling (BL-16502)#8029
Open
hatton wants to merge 17 commits into
Open
Import .bloomSource files into a collection, with duplicate-by-id handling (BL-16502)#8029hatton wants to merge 17 commits into
hatton wants to merge 17 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.BloomSourceImport.cs | New partial class implementing the full .bloomSource import pipeline: file-picker, zip validation (zip-slip + Bloom Pack guards), extraction staging, duplicate-by-bookInstanceId detection, replace/add-copy/cancel resolution, within-batch id collision tracking, and Team Collection status preservation. Error handling is layered correctly — BloomSourceImportException for user-friendly failures, generic catch for unexpected ones, and the replace path uses a backup-swap-restore pattern so mid-op failures never lose the original. |
| src/BloomExe/web/controllers/CollectionApi.cs | Two new endpoints added: chooseBloomSourceFilesToImport (handleOnUiThread: true — correct, opens a Windows Forms picker) and importBloomSource (handleOnUiThread: false — correct, runs behind a progress dialog on a background thread). The reply from the first endpoint bundles file paths, anyDuplicates, and canReplace in a single JSON object so the frontend can choose the right dialog without a second round-trip. |
| src/BloomBrowserUI/collectionsTab/CollectionsTabPane.tsx | Adds menu item and import dialog flow. State is minimal (importFiles, showImportSourceChoiceDialog, showImportDuplicateDialog, importCanReplace). The two dialogs are mutually exclusive based on anyDuplicates. All l10n strings are hoisted to top-level useL10n calls. Conforms to repo conventions. |
| src/BloomBrowserUI/collectionsTab/RadioChoiceDialog.tsx | New reusable dialog that offers mutually-exclusive radio choices with OK (disabled until a selection is made) and Cancel. The useEffect that resets choice on open is properly justified in an inline comment per the repo's useEffect guidance. |
| src/BloomExe/MiscUI/BloomOpenFileDialog.cs | FileOk handler now iterates FileNames instead of just FileName, correctly enforcing the filter for every file in a multi-select. Backward-compatible: FileNames contains one entry when Multiselect is false. |
| src/BloomTests/CollectionTab/CollectionModelTests.cs | 598 lines of new tests covering: valid import, duplicate-by-id (not title), AddCopy/Replace/Cancel resolution, within-batch same-id collision, canReplace flag, zip-slip, Bloom Pack rejection, corrupt file, no-htm, and Team Collection status preservation. |
| src/BloomTests/TestDoubles/CollectionTab/FakeCollectionModel.cs | Two critical fixes: BloomWebSocketServer now provided so CollectionChanged events don't NRE; BookCollection now gets a real BookSelection so AddBookInfo doesn't silently produce ErrorBookInfo objects with random ids. |
| src/BloomBrowserUI/react_components/muiRadio.tsx | Adds optional description prop rendered as a smaller, muted secondary line below the main label, and wires disabled down to FormControlLabel. Existing behaviour unchanged. |
| src/BloomExe/Book/BookStorage.cs | Defensive guard: Math.Min(8, idDigits.Length) prevents ArgumentOutOfRangeException when an imported book carries a short bookInstanceId. |
| DistFiles/localization/en/BloomMediumPriority.xlf | Adds 13 localization strings for the new import dialogs and progress title, all with translate=no and translator notes. |
| src/BloomExe/CollectionTab/CollectionModel.cs | Minimal change: a comment pointing to the new partial class file. No logic changes. |
| src/BloomBrowserUI/react_components/icons/ImportIcon.tsx | New SVG icon wrapped in MUI SvgIcon, uses currentColor to follow menu text color. |
Reviews (14): Last reviewed commit: "Merge remote-tracking branch 'origin/Ver..." | 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>
hatton
commented
Jul 6, 2026
hatton
commented
Jul 6, 2026
hatton
commented
Jul 6, 2026
hatton
commented
Jul 6, 2026
Member
Author
|
[Claude Opus 4.8] Consulted Devin on 2026-07-06 22:52 UTC up to commit |
…6502) Replacing a book that's already in the collection now overwrites the existing book folder in place, keeping its folder name and Team Collection checkout status, instead of importing into a new folder and deleting the old one. In a Team Collection the replaced book stays the same checked-out repo book (its new content is pushed on the next check-in) rather than becoming a new local book while the old one resurrects from the shared repo at the next sync. The set-aside old content is dot-prefixed and its cleanup is non-fatal, so a failed cleanup can't leave a scannable duplicate-id book or turn a successful replace into an "import failed" error. Also from preflight review decisions: - Record a derivative import's "Created" history event immediately, so every book in a multi-file batch gets one (only the last was ever selected, and the pending event only flushed on deselect). - Justify the RadioChoiceDialog reset useEffect per the repo useEffect rule. - When the collection is at a filesystem root, stage inside the collection folder rather than %TEMP% (which may be on another volume). Updated CollectionModelTests to cover overwrite-in-place and TC status preservation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
[Claude Opus 4.8] Consulted Devin on 2026-07-07 02:34 UTC up to commit |
Relocates the CollectionTab.ImportBloomSource* translation units from Bloom.xlf (high priority) to BloomMediumPriority.xlf. This commit lands a pre-existing uncommitted working-tree change that was already on the branch; the content was authored by the developer, not generated here.
…s (BL-16502)
Two refactors of the .bloomSource import feature, no behavior change:
1. Factor the import code (~740 lines) out of the already-bloated
CollectionModel.cs into a new partial-class file,
CollectionModel.BloomSourceImport.cs: the file picker, the duplicate
pre-flight check, and the import itself (edit / derivative / replace /
add-copy). Saving/exporting a book AS a .bloomSource stays in
CollectionModel.cs. Also drops two now-unused usings there.
2. Remove the server-side state the feature carried between three API calls.
Previously the chosen file paths lived in a _bloomSourceFilesToImport field
across chooseBloomSourceFilesToImport -> bloomSourceImportDuplicateInfo ->
importBloomSource. Now the backend is stateless commands that the front-end
drives:
- ChooseBloomSourceFilesToImport() returns the chosen string[] paths.
- The choose endpoint returns { files, anyDuplicates, canReplace } in one
reply, so the separate bloomSourceImportDuplicateInfo endpoint is gone.
- importBloomSource reads the paths from the POST body; the front-end holds
them in React state and passes them back.
The file-system mechanics (zip, validation, folder moves, Team Collection
handling, rollback) stay in C# where they belong; only the orchestration state
moved to the front-end.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hatton
commented
Jul 7, 2026
hatton
commented
Jul 7, 2026
hatton
commented
Jul 7, 2026
Member
Author
|
[Claude Opus 4.8] Consulted Devin on 2026-07-07 (re-review of the stateless-API refactor + master merge, analyzed at commit
Findings up to |
The new CollectionTab.ImportBloomSource.* entries placed the translator context <note> before the <note>ID: ...</note> line. The project's xlf-strings skill (and the rest of this file) require the ID note first, with context notes after it. Reorder all 13 entries; pure reordering, no text changes. Addresses the Devin re-review bug finding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hatton
force-pushed
the
BL-16502-Import-BloomSource
branch
from
969e537 to
5e0027a
Compare
StephenMcConnel
requested changes
Jul 8, 2026
StephenMcConnel
left a comment
StephenMcConnel
left a comment
Contributor
There was a problem hiding this comment.
One minor comment on a comment in a test file. Otherwise looks good.
@StephenMcConnel reviewed 15 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on hatton).
src/BloomTests/CollectionTab/CollectionModelTests.cs line 127 at r6 (raw file):
// Build a .bloomSource file the same way Bloom's export does: a zip of a book folder's // contents (flat at the root) plus a .bloomCollection settings file at the root.
The comments are bit misleading -- they caused me to think, we're zipping up files here instead of using the existing method to do so?? I would say something like
// Create a .bloomSource file for unit testing from a minimal set of dummy files.
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
2 participants
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