From 6b530d15672bcfb5ce898f0d21d83e05b0129312 Mon Sep 17 00:00:00 2001 From: Tiberiu Ichim Date: Fri, 6 Feb 2026 22:21:22 +0200 Subject: [PATCH] Fix move bugs --- .../move-to-library-specification.md | 153 ++++++++++++++++++ server/controllers/LibraryItemController.js | 55 ++++++- server/scanner/LibraryItemScanner.js | 3 +- server/scanner/LibraryScanner.js | 9 +- 4 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 artifacts/2026-02-06/move-to-library-specification.md diff --git a/artifacts/2026-02-06/move-to-library-specification.md b/artifacts/2026-02-06/move-to-library-specification.md new file mode 100644 index 000000000..e32cd6fab --- /dev/null +++ b/artifacts/2026-02-06/move-to-library-specification.md @@ -0,0 +1,153 @@ +# Move to Library Feature Documentation + +**Date:** 2026-02-06 + +## Overview + +This feature allows users to move audiobooks (and podcasts) between libraries of the same type via a context menu option. It supports both single-item moves and batch moves for multiple selected items. + +## API Endpoints + +### Single Item Move + +``` +POST /api/items/:id/move +``` + +### Batch Move + +``` +POST /api/items/batch/move +``` + +**Request Body (Single):** + +```json +{ + "targetLibraryId": "uuid-of-target-library", + "targetFolderId": "uuid-of-target-folder" // optional, uses first folder if not provided +} +``` + +**Request Body (Batch):** + +```json +{ + "libraryItemIds": ["uuid1", "uuid2"], + "targetLibraryId": "uuid-of-target-library", + "targetFolderId": "uuid-of-target-folder" // optional +} +``` + +**Permissions:** Requires delete permission (`canDelete`) + +**Validations:** + +- Target library must exist +- Target library must have same `mediaType` as source (book ↔ book, podcast ↔ podcast) +- Cannot move to the same library +- Destination path must not already exist (checked per item) + +**Response (Single):** Returns updated library item JSON on success **Response (Batch):** Returns summary of successes, failures, and error details + +--- + +## Files Modified + +### Backend + +| File | Description | +| --------------------------------------------- | ------------------------------------------------------------------ | +| `server/controllers/LibraryItemController.js` | Implementation of `handleMoveLibraryItem`, `move`, and `batchMove` | +| `server/routers/ApiRouter.js` | Route registration for single and batch move | + +### Frontend + +| File | Description | +| ------------------------------------------------------ | ------------------------------------------------ | +| `client/components/modals/item/MoveToLibraryModal.vue` | Modal component (handles single and batch modes) | +| `client/components/app/Appbar.vue` | Added "Move to library" to batch context menu | +| `client/store/globals.js` | State management for move modal visibility | +| `client/components/cards/LazyBookCard.vue` | Single item context menu integration | +| `client/pages/item/_id/index.vue` | Single item page context menu integration | +| `client/layouts/default.vue` | Modal registration | +| `client/strings/en-us.json` | Localization strings | + +### Localization Strings Added + +- `ToastItemsMoved`, `ToastItemsMoveFailed` +- `LabelMovingItems` +- (Legacy) `ToastItemMoved`, `ToastItemMoveFailed`, `LabelMovingItem`, etc. + +--- + +## Implementation Details + +### Shared Moving Logic (`handleMoveLibraryItem`) + +To ensure consistency, the core logic is encapsulated in a standalone function `handleMoveLibraryItem` in `LibraryItemController.js`. This prevents "this" binding issues when called from `ApiRouter`. + +Steps performed for each item: + +1. Fetch target library with folders +2. Select target folder (first if not specified) +3. Calculate new path: `targetFolder.path + itemFolderName` +4. Check destination doesn't exist +5. Move files using `fs.move(oldPath, newPath)` +6. Update database: `libraryId`, `libraryFolderId`, `path`, `relPath` +7. Update `libraryFiles` paths +8. Update media specific paths (`audioFiles`, `ebookFile`, `podcastEpisodes`) +9. Handle Series and Authors: + - Moves/merges series and authors to target library + - Copies metadata and images if necessary + - Deletes source series/authors if they become empty +10. Emit socket events: `item_removed` (old library), `item_added` (new library) +11. Reset filter data for both libraries + +### Batch Move Strategy + +The `batchMove` endpoint iterates through the provided IDs and calls `handleMoveLibraryItem` for each valid item. It maintains a success/fail count and collects error messages for the final response. + +### Frontend Modal Behavior + +The `MoveToLibraryModal` automatically detects if it's in batch mode by checking if `selectedMediaItems` has content and no single `selectedLibraryItem` is set. It dynamically adjusts its titles and labels (e.g., "Moving items" vs "Moving item"). + +--- + +## Testing + +1. **Single Move**: Verify via context menu on a book card. +2. **Batch Move**: + - Select multiple items using checkboxes + - Use "Move to library" in the top batch bar ⋮ menu + - Verify all items are moved correctly in the UI and filesystem. +3. **Incompatible Move**: Try moving a book to a podcast library (should be blocked). + +## Bug Analysis & Refinements (2026-02-06) + +Following the initial implementation, a deep dive into the code identified several critical areas for improvement: + +### 1. Socket Event Omissions +- **Issue**: Source series and authors are destroyed in the DB when empty, but no `series_removed` or `author_removed` events are emitted. +- **Fix**: Add `SocketAuthority.emitter` calls for these events in `handleMoveLibraryItem`. + +### 2. Batch Move Efficiency +- **Issue**: `Database.resetLibraryIssuesFilterData` is called inside the loop for every item. +- **Fix**: Move these calls out of `handleMoveLibraryItem` and into the parent `move` and `batchMove` controllers, ensuring they only run once per request. + +### 3. Async/Await Inconsistency +- **Issue**: Metadata `save()` calls for newly created series/authors were not awaited. +- **Fix**: Ensure all `.save()` calls are properly awaited to prevent race conditions. + +### 4. Transactional Integrity +- **Issue**: The move logic is not wrapped in a DB transaction. +- **Fix**: Wrap the DB update portion of `handleMoveLibraryItem` in a transaction to ensure atomicity. + +--- + +## Known Limitations / Future Work + +- Does not support moving to different folder within same library. +- Rollback is per-item; a failure in a batch move does not roll back successfully moved previous items. +- No overall progress bar for large batch moves (it's sequential and blocking). +- Moves currently flatten nested structures (uses `basename` of the item path) instead of preserving source relative paths. diff --git a/server/controllers/LibraryItemController.js b/server/controllers/LibraryItemController.js index 3b467a6db..2ed7a479d 100644 --- a/server/controllers/LibraryItemController.js +++ b/server/controllers/LibraryItemController.js @@ -13,6 +13,10 @@ const { getAudioMimeTypeFromExtname, encodeUriPath } = require('../utils/fileUti const LibraryItemScanner = require('../scanner/LibraryItemScanner') const AudioFileScanner = require('../scanner/AudioFileScanner') const Scanner = require('../scanner/Scanner') +const Watcher = require('../Watcher') + +const libraryItemsBookFilters = require('../utils/queries/libraryItemsBookFilters') +const libraryItemsPodcastFilters = require('../utils/queries/libraryItemsPodcastFilters') const RssFeedManager = require('../managers/RssFeedManager') const CacheManager = require('../managers/CacheManager') @@ -59,6 +63,11 @@ async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) { } try { + Watcher.addIgnoreDir(oldPath) + Watcher.addIgnoreDir(newPath) + + const oldRelPath = libraryItem.relPath + // Move files on disk Logger.info(`[LibraryItemController] Moving item "${libraryItem.media.title}" from "${oldPath}" to "${newPath}"`) await fs.move(oldPath, newPath) @@ -68,13 +77,20 @@ async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) { libraryItem.libraryFolderId = targetFolder.id libraryItem.path = newPath libraryItem.relPath = newRelPath + libraryItem.isMissing = false + libraryItem.isInvalid = false libraryItem.changed('updatedAt', true) await libraryItem.save() // Update library files paths if (libraryItem.libraryFiles?.length) { libraryItem.libraryFiles = libraryItem.libraryFiles.map((lf) => { - lf.metadata.path = lf.metadata.path.replace(oldPath, newPath) + if (lf.metadata?.path) { + lf.metadata.path = lf.metadata.path.replace(oldPath, newPath) + } + if (lf.metadata?.relPath) { + lf.metadata.relPath = lf.metadata.relPath.replace(oldRelPath, newRelPath) + } return lf }) libraryItem.changed('libraryFiles', true) @@ -89,6 +105,9 @@ async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) { if (af.metadata?.path) { af.metadata.path = af.metadata.path.replace(oldPath, newPath) } + if (af.metadata?.relPath) { + af.metadata.relPath = af.metadata.relPath.replace(oldRelPath, newRelPath) + } return af }) libraryItem.media.changed('audioFiles', true) @@ -96,14 +115,34 @@ async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) { // Update ebookFile path if (libraryItem.media.ebookFile?.metadata?.path) { libraryItem.media.ebookFile.metadata.path = libraryItem.media.ebookFile.metadata.path.replace(oldPath, newPath) + if (libraryItem.media.ebookFile.metadata?.relPath) { + libraryItem.media.ebookFile.metadata.relPath = libraryItem.media.ebookFile.metadata.relPath.replace(oldRelPath, newRelPath) + } libraryItem.media.changed('ebookFile', true) } + // Update coverPath + if (libraryItem.media.coverPath) { + libraryItem.media.coverPath = libraryItem.media.coverPath.replace(oldPath, newPath) + } await libraryItem.media.save() } else if (libraryItem.isPodcast) { + // Update coverPath + if (libraryItem.media.coverPath) { + libraryItem.media.coverPath = libraryItem.media.coverPath.replace(oldPath, newPath) + } + await libraryItem.media.save() // Update podcast episode audio file paths for (const episode of libraryItem.media.podcastEpisodes || []) { + let episodeUpdated = false if (episode.audioFile?.metadata?.path) { episode.audioFile.metadata.path = episode.audioFile.metadata.path.replace(oldPath, newPath) + episodeUpdated = true + } + if (episode.audioFile?.metadata?.relPath) { + episode.audioFile.metadata.relPath = episode.audioFile.metadata.relPath.replace(oldRelPath, newRelPath) + episodeUpdated = true + } + if (episodeUpdated) { episode.changed('audioFile', true) await episode.save() } @@ -205,6 +244,17 @@ async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) { await Database.resetLibraryIssuesFilterData(oldLibraryId) await Database.resetLibraryIssuesFilterData(targetLibrary.id) + // Clear filter data cache for both libraries to ensure authors/series/genres are updated + if (Database.libraryFilterData[oldLibraryId]) delete Database.libraryFilterData[oldLibraryId] + if (Database.libraryFilterData[targetLibrary.id]) delete Database.libraryFilterData[targetLibrary.id] + + // Clear count cache to ensure the library page shows the correct item count + if (libraryItem.isBook) { + libraryItemsBookFilters.clearCountCache('move_item') + } else if (libraryItem.isPodcast) { + libraryItemsPodcastFilters.clearCountCache('podcast', 'move_item') + } + Logger.info(`[LibraryItemController] Successfully moved item "${libraryItem.media.title}" to library "${targetLibrary.name}"`) } catch (error) { Logger.error(`[LibraryItemController] Failed to move item "${libraryItem.media.title}"`, error) @@ -219,6 +269,9 @@ async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) { } } throw error + } finally { + Watcher.removeIgnoreDir(oldPath) + Watcher.removeIgnoreDir(newPath) } } diff --git a/server/scanner/LibraryItemScanner.js b/server/scanner/LibraryItemScanner.js index 501df4274..d1b70bdfd 100644 --- a/server/scanner/LibraryItemScanner.js +++ b/server/scanner/LibraryItemScanner.js @@ -58,7 +58,8 @@ class LibraryItemScanner { const libraryItemPath = updateLibraryItemDetails?.path || fileUtils.filePathToPOSIX(libraryItem.path) const folder = library.libraryFolders[0] - const libraryItemScanData = await this.getLibraryItemScanData(libraryItemPath, library, folder, updateLibraryItemDetails?.isFile || false) + const isSingleMediaItem = updateLibraryItemDetails?.isFile !== undefined ? updateLibraryItemDetails.isFile : !!libraryItem.isFile + const libraryItemScanData = await this.getLibraryItemScanData(libraryItemPath, library, folder, isSingleMediaItem) let libraryItemDataUpdated = await libraryItemScanData.checkLibraryItemData(libraryItem, scanLogger) diff --git a/server/scanner/LibraryScanner.js b/server/scanner/LibraryScanner.js index 640c82d76..a3014396e 100644 --- a/server/scanner/LibraryScanner.js +++ b/server/scanner/LibraryScanner.js @@ -659,6 +659,7 @@ function isSingleMediaFile(fileUpdateGroup, itemDir) { } async function findLibraryItemByItemToItemInoMatch(libraryId, fullPath) { + if (!(await fs.pathExists(fullPath))) return null const ino = await fileUtils.getIno(fullPath) if (!ino) return null const existingLibraryItem = await Database.libraryItemModel.findOneExpanded({ @@ -672,6 +673,7 @@ async function findLibraryItemByItemToItemInoMatch(libraryId, fullPath) { async function findLibraryItemByItemToFileInoMatch(libraryId, fullPath, isSingleMedia) { if (!isSingleMedia) return null // check if it was moved from another folder by comparing the ino to the library files + if (!(await fs.pathExists(fullPath))) return null const ino = await fileUtils.getIno(fullPath) if (!ino) return null const existingLibraryItem = await Database.libraryItemModel.findOneExpanded( @@ -696,8 +698,11 @@ async function findLibraryItemByFileToItemInoMatch(libraryId, fullPath, isSingle // check if it was moved from the root folder by comparing the ino to the ino of the scanned files let itemFileInos = [] for (const itemFile of itemFiles) { - const ino = await fileUtils.getIno(Path.posix.join(fullPath, itemFile)) - if (ino) itemFileInos.push(ino) + const filePath = Path.posix.join(fullPath, itemFile) + if (await fs.pathExists(filePath)) { + const ino = await fileUtils.getIno(filePath) + if (ino) itemFileInos.push(ino) + } } if (!itemFileInos.length) return null const existingLibraryItem = await Database.libraryItemModel.findOneExpanded({