diff --git a/documentation/admin-features/request-deletion.md b/documentation/admin-features/request-deletion.md index 859d3f5..1fa482a 100644 --- a/documentation/admin-features/request-deletion.md +++ b/documentation/admin-features/request-deletion.md @@ -88,8 +88,10 @@ model Request { ``` 3. **Delete Media Files** - - Path: `[media_dir]/[author]/[title]/` + - Path: persisted `Audiobook.filePath` recorded during file organization + - Never rebuilds the path from the current `media_dir` or path template - **ONLY deletes title folder** (not author folder) + - Missing stored path: skips file deletion and returns `filesDeleted: false` - Handles missing folders gracefully 4. **Delete from Library Backend** @@ -214,6 +216,8 @@ where: { 13. ✅ **Plex deletion not enabled in settings** - Log error, continue with soft delete 14. ✅ **Title mismatch in plex_library** - ASIN-based deletion handles title variations (e.g., "(Unabridged)" suffix) 15. ✅ **No ASIN available** - Falls back to exact title/author matching +16. ✅ **Path template changed after organization** - Deletes only the persisted organized path +17. ✅ **Legacy row has no stored path** - Skips file deletion without guessing a path ## Fixed Issues ✅ @@ -224,6 +228,12 @@ where: { - **Fix:** Changed plex_library deletion to use ASIN-based matching (same as availability check) - **Result:** Books immediately show as NOT available after deletion, can be re-requested right away +**2. Changed Path Template Deletes an Unrelated Directory** +- **Issue:** Deletion rebuilt a directory from the current media path and template +- **Cause:** `Audiobook.filePath` was persisted during organization but omitted from the deletion query +- **Fix:** Delete only the persisted path; legacy rows without one skip file cleanup +- **Result:** Configuration changes cannot redirect deletion to a newly rendered directory + ## File Structure ``` @@ -250,7 +260,6 @@ Queries Updated (deletedAt: null filters): **No new config required** - uses existing: - `prowlarr_indexers` (seeding time per indexer) -- `media_dir` (file deletion path) ## Security @@ -258,6 +267,7 @@ Queries Updated (deletedAt: null filters): - **Audit Trail:** `deletedBy` tracks admin user ID - **Soft Delete:** Preserves history, prevents permanent data loss - **Confirmation Required:** Prevents accidental deletion +- **Stored Path Only:** File cleanup targets the path recorded by the organizer; no template-based fallback ## Monitoring & Logging diff --git a/src/lib/services/request-delete.service.ts b/src/lib/services/request-delete.service.ts index 77cdc08..3812d16 100644 --- a/src/lib/services/request-delete.service.ts +++ b/src/lib/services/request-delete.service.ts @@ -9,7 +9,6 @@ import { prisma } from '../db'; import * as fs from 'fs/promises'; import * as path from 'path'; import { RMABLogger } from '../utils/logger'; -import { buildAudiobookPath } from '../utils/file-organizer'; import { CLIENT_PROTOCOL_MAP, DownloadClientType } from '../interfaces/download-client.interface'; const logger = RMABLogger.create('RequestDelete'); @@ -66,6 +65,7 @@ export async function deleteRequest( audibleAsin: true, plexGuid: true, absItemId: true, + filePath: true, fileFormat: true, }, }, @@ -202,72 +202,46 @@ export async function deleteRequest( // For ebooks: delete only ebook files (leave audiobook files intact) let filesDeleted = false; try { - const { getConfigService } = await import('./config.service'); - const configService = getConfigService(); - const mediaDir = (await configService.get('media_dir')) || '/media/audiobooks'; - // Use ebook-specific template for ebook requests, with fallback to audiobook template - const audiobookTemplate = (await configService.get('audiobook_path_template')) || '{author}/{title} {asin}'; - const template = isEbook - ? (await configService.get('ebook_path_template')) || audiobookTemplate - : audiobookTemplate; + const titleFolderPath = request.audiobook.filePath; - // Fetch year from audible cache if ASIN is available - let year: number | undefined; - if (request.audiobook.audibleAsin) { - const audibleCache = await prisma.audibleCache.findUnique({ - where: { asin: request.audiobook.audibleAsin }, - select: { releaseDate: true }, - }); - if (audibleCache?.releaseDate) { - year = new Date(audibleCache.releaseDate).getFullYear(); - } - } + if (!titleFolderPath) { + logger.warn( + `Skipping media file deletion for request ${requestId}: no stored file path` + ); + } else { + // Check if the directory recorded during organization still exists. + try { + await fs.access(titleFolderPath); - // Build path using centralized function - const titleFolderPath = buildAudiobookPath( - mediaDir, - template, - { - author: request.audiobook.author, - title: request.audiobook.title, - narrator: request.audiobook.narrator || undefined, - asin: request.audiobook.audibleAsin || undefined, - year, - } - ); + if (isEbook) { + // For ebooks: only delete ebook files, leave audiobook files intact + const ebookExtensions = ['.epub', '.pdf', '.mobi', '.azw', '.azw3', '.fb2', '.cbz', '.cbr']; + const files = await fs.readdir(titleFolderPath); - // Check if folder exists - try { - await fs.access(titleFolderPath); - - if (isEbook) { - // For ebooks: only delete ebook files, leave audiobook files intact - const ebookExtensions = ['.epub', '.pdf', '.mobi', '.azw', '.azw3', '.fb2', '.cbz', '.cbr']; - const files = await fs.readdir(titleFolderPath); - - let deletedCount = 0; - for (const file of files) { - const ext = path.extname(file).toLowerCase(); - if (ebookExtensions.includes(ext)) { - const filePath = path.join(titleFolderPath, file); - await fs.unlink(filePath); - logger.info(`Deleted ebook file: ${file}`); - deletedCount++; + let deletedCount = 0; + for (const file of files) { + const ext = path.extname(file).toLowerCase(); + if (ebookExtensions.includes(ext)) { + const filePath = path.join(titleFolderPath, file); + await fs.unlink(filePath); + logger.info(`Deleted ebook file: ${file}`); + deletedCount++; + } } - } - filesDeleted = deletedCount > 0; - logger.info(`Deleted ${deletedCount} ebook file(s) from: ${titleFolderPath}`); - } else { - // For audiobooks: delete the entire title folder - await fs.rm(titleFolderPath, { recursive: true, force: true }); - logger.info(`Deleted media directory: ${titleFolderPath}`); - filesDeleted = true; + filesDeleted = deletedCount > 0; + logger.info(`Deleted ${deletedCount} ebook file(s) from: ${titleFolderPath}`); + } else { + // For audiobooks: delete the entire title folder + await fs.rm(titleFolderPath, { recursive: true, force: true }); + logger.info(`Deleted media directory: ${titleFolderPath}`); + filesDeleted = true; + } + } catch (accessError) { + // Folder doesn't exist - that's okay + logger.info(`Media directory not found: ${titleFolderPath}`); + filesDeleted = false; } - } catch (accessError) { - // Folder doesn't exist - that's okay - logger.info(`Media directory not found: ${titleFolderPath}`); - filesDeleted = false; } } catch (error) { logger.error( diff --git a/tests/services/request-delete.service.test.ts b/tests/services/request-delete.service.test.ts index 48deec6..68d8a7f 100644 --- a/tests/services/request-delete.service.test.ts +++ b/tests/services/request-delete.service.test.ts @@ -38,13 +38,6 @@ vi.mock('@/lib/services/audiobookshelf/api', () => ({ deleteABSItem: vi.fn(), })); -vi.mock('@/lib/utils/file-organizer', () => ({ - buildAudiobookPath: vi.fn((mediaDir: string, template: string, data: any) => { - // Simple mock implementation that mimics the real behavior for tests - return path.join(mediaDir, data.author, `${data.title} ${data.asin}`); - }), -})); - describe('deleteRequest', () => { beforeEach(() => { vi.clearAllMocks(); @@ -74,6 +67,7 @@ describe('deleteRequest', () => { audibleAsin: 'ASIN1', plexGuid: 'plex-1', absItemId: null, + filePath: path.join('/media', 'Author', 'Book ASIN1'), }, downloadHistory: [ { @@ -154,6 +148,7 @@ describe('deleteRequest', () => { audibleAsin: null, plexGuid: 'plex-2', absItemId: null, + filePath: path.join('/media', 'Author', 'Book Two'), }, downloadHistory: [ { @@ -219,6 +214,7 @@ describe('deleteRequest', () => { audibleAsin: 'ASIN3', plexGuid: 'plex-3', absItemId: null, + filePath: path.join('/media', 'Author Name', 'Book Three ASIN3'), }, downloadHistory: [ { @@ -291,6 +287,7 @@ describe('deleteRequest', () => { audibleAsin: null, plexGuid: 'plex-4', absItemId: null, + filePath: path.join('/media', 'Author', 'Book Four'), }, downloadHistory: [ { @@ -349,6 +346,7 @@ describe('deleteRequest', () => { audibleAsin: null, plexGuid: null, absItemId: 'abs-5', + filePath: path.join('/media', 'Author', 'Book Five'), }, downloadHistory: [ { @@ -401,6 +399,7 @@ describe('deleteRequest', () => { audibleAsin: 'ASIN6', plexGuid: null, absItemId: 'abs-item-123', + filePath: path.join('/media', 'Author Six', 'Book Six ASIN6'), }, downloadHistory: [], }); @@ -447,6 +446,7 @@ describe('deleteRequest', () => { audibleAsin: null, plexGuid: null, absItemId: 'abs-item-456', + filePath: path.join('/media', 'Author Seven', 'Book Seven'), }, downloadHistory: [], }); diff --git a/tests/services/request-delete.stored-path.test.ts b/tests/services/request-delete.stored-path.test.ts new file mode 100644 index 0000000..f995539 --- /dev/null +++ b/tests/services/request-delete.stored-path.test.ts @@ -0,0 +1,130 @@ +/** + * Component: Request Delete Stored Path Regression Tests + * Documentation: documentation/admin-features/request-deletion.md + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'fs/promises'; +import * as os from 'os'; +import * as path from 'path'; +import { createPrismaMock } from '../helpers/prisma'; + +const prismaMock = createPrismaMock(); +const configServiceMock = { + get: vi.fn(), + getBackendMode: vi.fn(), +}; + +vi.mock('@/lib/db', () => ({ + prisma: prismaMock, +})); + +vi.mock('@/lib/services/config.service', () => ({ + getConfigService: () => configServiceMock, +})); + +vi.mock('@/lib/utils/logger', () => ({ + RMABLogger: { + create: () => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), + }, +})); + +describe('deleteRequest stored media path', () => { + let tempRoot: string; + + beforeEach(async () => { + vi.clearAllMocks(); + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'rmab-request-delete-')); + configServiceMock.get.mockRejectedValue( + new Error('Media deletion must not read current path configuration') + ); + configServiceMock.getBackendMode.mockResolvedValue('plex'); + prismaMock.request.findMany.mockResolvedValue([]); + prismaMock.request.updateMany.mockResolvedValue({ count: 0 }); + prismaMock.request.update.mockResolvedValue({}); + prismaMock.audiobook.update.mockResolvedValue({}); + prismaMock.plexLibrary.findMany.mockResolvedValue([]); + }); + + afterEach(async () => { + await fs.rm(tempRoot, { recursive: true, force: true }); + }); + + it('deletes the stored directory and preserves the path rendered by the current template', async () => { + const storedPath = path.join(tempRoot, 'Arthur Conan Doyle', 'The Valley of Fear'); + const currentTemplatePath = path.join( + tempRoot, + 'Arthur Conan Doyle', + '1914 - The Valley of Fear' + ); + const unrelatedFile = path.join(currentTemplatePath, 'my own rip.m4b'); + + await fs.mkdir(storedPath, { recursive: true }); + await fs.mkdir(currentTemplatePath, { recursive: true }); + await fs.writeFile(path.join(storedPath, 'book.m4b'), 'organized by RMAB'); + await fs.writeFile(unrelatedFile, 'user-owned file'); + + prismaMock.request.findFirst.mockResolvedValue({ + id: 'req-stored-path', + type: 'audiobook', + audiobook: { + id: 'ab-stored-path', + title: 'The Valley of Fear', + author: 'Arthur Conan Doyle', + narrator: null, + audibleAsin: null, + plexGuid: null, + absItemId: null, + filePath: storedPath, + fileFormat: 'm4b', + }, + downloadHistory: [], + }); + + const { deleteRequest } = await import('@/lib/services/request-delete.service'); + const result = await deleteRequest('req-stored-path', 'admin-1'); + + expect(result.success).toBe(true); + expect(result.filesDeleted).toBe(true); + await expect(fs.access(storedPath)).rejects.toThrow(); + await expect(fs.readFile(unrelatedFile, 'utf8')).resolves.toBe('user-owned file'); + expect(configServiceMock.get).not.toHaveBeenCalled(); + }); + + it('skips file deletion when a legacy row has no stored path', async () => { + const unrelatedPath = path.join(tempRoot, 'Arthur Conan Doyle', 'The Valley of Fear'); + const unrelatedFile = path.join(unrelatedPath, 'my own rip.m4b'); + + await fs.mkdir(unrelatedPath, { recursive: true }); + await fs.writeFile(unrelatedFile, 'user-owned file'); + + prismaMock.request.findFirst.mockResolvedValue({ + id: 'req-no-stored-path', + type: 'audiobook', + audiobook: { + id: 'ab-no-stored-path', + title: 'The Valley of Fear', + author: 'Arthur Conan Doyle', + narrator: null, + audibleAsin: null, + plexGuid: null, + absItemId: null, + filePath: null, + fileFormat: null, + }, + downloadHistory: [], + }); + + const { deleteRequest } = await import('@/lib/services/request-delete.service'); + const result = await deleteRequest('req-no-stored-path', 'admin-1'); + + expect(result.success).toBe(true); + expect(result.filesDeleted).toBe(false); + await expect(fs.readFile(unrelatedFile, 'utf8')).resolves.toBe('user-owned file'); + expect(configServiceMock.get).not.toHaveBeenCalled(); + }); +});