mirror of
https://github.com/advplyr/audiobookshelf.git
synced 2026-03-02 14:09:43 +00:00
Fix move bugs
This commit is contained in:
parent
7cc476a6ed
commit
6b530d1567
4 changed files with 216 additions and 4 deletions
153
artifacts/2026-02-06/move-to-library-specification.md
Normal file
153
artifacts/2026-02-06/move-to-library-specification.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue