mirror of
https://github.com/kikootwo/ReadMeABook.git
synced 2026-09-01 05:08:38 +00:00
fix: delete requests using stored media path (#276)
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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: [],
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user