mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(library): preserve folder creation order
Keep unrelated folder selections in the order users add them, and make API reads deterministic so web and iOS defaults use the same first folder. Fixes #1412
This commit is contained in:
@@ -11,6 +11,10 @@ describe('folder path selection', () => {
|
||||
expect(consolidateFolderPaths(['/books/fiction', '/audio', '/books', '/books/fantasy'])).toEqual(['/audio', '/books'])
|
||||
})
|
||||
|
||||
it('preserves the order of unrelated folders', () => {
|
||||
expect(consolidateFolderPaths(['/library/zz', '/library/alpha-long', '/library/m'])).toEqual(['/library/zz', '/library/alpha-long', '/library/m'])
|
||||
})
|
||||
|
||||
it('finds the selected path that already covers a nested folder', () => {
|
||||
expect(coveringFolderPath('/books/fiction/scifi', ['/books', '/books/fiction'])).toBe('/books/fiction')
|
||||
})
|
||||
|
||||
@@ -15,8 +15,8 @@ export function coveringFolderPath(path: string, candidates: string[]): string |
|
||||
}
|
||||
|
||||
export function consolidateFolderPaths(paths: string[]): string[] {
|
||||
const unique = [...new Set(paths.map(normalizeFolderPath))].sort((a, b) => a.length - b.length || a.localeCompare(b))
|
||||
return unique.filter((path, index) => !unique.slice(0, index).some((candidate) => isPathInside(path, candidate)))
|
||||
const unique = [...new Set(paths.map(normalizeFolderPath))]
|
||||
return unique.filter((path) => !unique.some((candidate) => candidate !== path && isPathInside(path, candidate)))
|
||||
}
|
||||
|
||||
function isPathInside(path: string, parent: string): boolean {
|
||||
|
||||
@@ -7,7 +7,7 @@ vi.mock('drizzle-orm', () => ({
|
||||
sql: vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({ op: 'sql', text: strings.join(''), values })),
|
||||
}));
|
||||
|
||||
import { books, libraries } from '../../db/schema';
|
||||
import { books, libraries, libraryFolders } from '../../db/schema';
|
||||
import { LIBRARY_BOOK_STATUS_PRESENT } from './library.constants';
|
||||
import { LibraryRepository } from './library.repository';
|
||||
|
||||
@@ -113,6 +113,43 @@ describe('LibraryRepository', () => {
|
||||
expect(orderBy).toHaveBeenCalledWith(libraries.id);
|
||||
});
|
||||
|
||||
it('orders folders by creation time with an id tie-breaker', async () => {
|
||||
const byLibraryOrderBy = vi.fn().mockResolvedValue([]);
|
||||
const byLibraryWhere = vi.fn().mockReturnValue({ orderBy: byLibraryOrderBy });
|
||||
const byLibraryFrom = vi.fn().mockReturnValue({ where: byLibraryWhere });
|
||||
db.select.mockReturnValueOnce({ from: byLibraryFrom });
|
||||
|
||||
await repo.findFoldersByLibrary(7);
|
||||
|
||||
expect(byLibraryOrderBy).toHaveBeenCalledWith(libraryFolders.createdAt, libraryFolders.id);
|
||||
|
||||
const allOrderBy = vi.fn().mockResolvedValue([]);
|
||||
const allFrom = vi.fn().mockReturnValue({ orderBy: allOrderBy });
|
||||
db.select.mockReturnValueOnce({ from: allFrom });
|
||||
|
||||
await repo.findAllFolders();
|
||||
|
||||
expect(allOrderBy).toHaveBeenCalledWith(libraryFolders.libraryId, libraryFolders.createdAt, libraryFolders.id);
|
||||
|
||||
const scopedOrderBy = vi.fn().mockResolvedValue([]);
|
||||
const scopedWhere = vi.fn().mockReturnValue({ orderBy: scopedOrderBy });
|
||||
const scopedFrom = vi.fn().mockReturnValue({ where: scopedWhere });
|
||||
db.select.mockReturnValueOnce({ from: scopedFrom });
|
||||
|
||||
await repo.findFoldersByLibraryIds([7, 8]);
|
||||
|
||||
expect(scopedOrderBy).toHaveBeenCalledWith(libraryFolders.libraryId, libraryFolders.createdAt, libraryFolders.id);
|
||||
|
||||
const pathsOrderBy = vi.fn().mockResolvedValue([]);
|
||||
const pathsInnerJoin = vi.fn().mockReturnValue({ orderBy: pathsOrderBy });
|
||||
const pathsFrom = vi.fn().mockReturnValue({ innerJoin: pathsInnerJoin });
|
||||
db.select.mockReturnValueOnce({ from: pathsFrom });
|
||||
|
||||
await repo.findAllFolderPaths();
|
||||
|
||||
expect(pathsOrderBy).toHaveBeenCalledWith(libraries.displayOrder, libraries.name, libraryFolders.createdAt, libraryFolders.id);
|
||||
});
|
||||
|
||||
it('getStats aggregates counts, sizes, and format map', async () => {
|
||||
const countChain = {
|
||||
from: vi.fn().mockReturnValue({
|
||||
|
||||
@@ -89,23 +89,28 @@ export class LibraryRepository {
|
||||
}
|
||||
|
||||
findFoldersByLibrary(libraryId: number) {
|
||||
return this.db.select().from(libraryFolders).where(eq(libraryFolders.libraryId, libraryId));
|
||||
return this.db.select().from(libraryFolders).where(eq(libraryFolders.libraryId, libraryId)).orderBy(libraryFolders.createdAt, libraryFolders.id);
|
||||
}
|
||||
|
||||
findAllFolders() {
|
||||
return this.db.select().from(libraryFolders);
|
||||
return this.db.select().from(libraryFolders).orderBy(libraryFolders.libraryId, libraryFolders.createdAt, libraryFolders.id);
|
||||
}
|
||||
|
||||
findFoldersByLibraryIds(libraryIds: number[]) {
|
||||
if (libraryIds.length === 0) return Promise.resolve([]);
|
||||
return this.db.select().from(libraryFolders).where(inArray(libraryFolders.libraryId, libraryIds));
|
||||
return this.db
|
||||
.select()
|
||||
.from(libraryFolders)
|
||||
.where(inArray(libraryFolders.libraryId, libraryIds))
|
||||
.orderBy(libraryFolders.libraryId, libraryFolders.createdAt, libraryFolders.id);
|
||||
}
|
||||
|
||||
findAllFolderPaths() {
|
||||
return this.db
|
||||
.select({ libraryId: libraryFolders.libraryId, path: libraryFolders.path, libraryName: libraries.name })
|
||||
.from(libraryFolders)
|
||||
.innerJoin(libraries, eq(libraries.id, libraryFolders.libraryId));
|
||||
.innerJoin(libraries, eq(libraries.id, libraryFolders.libraryId))
|
||||
.orderBy(libraries.displayOrder, libraries.name, libraryFolders.createdAt, libraryFolders.id);
|
||||
}
|
||||
|
||||
insert(data: typeof libraries.$inferInsert) {
|
||||
@@ -120,7 +125,7 @@ export class LibraryRepository {
|
||||
.returning();
|
||||
}
|
||||
|
||||
insertFolder(data: typeof libraryFolders.$inferInsert) {
|
||||
insertFolders(data: (typeof libraryFolders.$inferInsert)[]) {
|
||||
return this.db.insert(libraryFolders).values(data).returning();
|
||||
}
|
||||
|
||||
|
||||
@@ -42,7 +42,7 @@ describe('LibraryService', () => {
|
||||
findFoldersByLibrary: vi.fn(),
|
||||
findByName: vi.fn(),
|
||||
insert: vi.fn(),
|
||||
insertFolder: vi.fn(),
|
||||
insertFolders: vi.fn(),
|
||||
update: vi.fn(),
|
||||
deleteFolder: vi.fn(),
|
||||
findBookIdsByLibrary: vi.fn(),
|
||||
@@ -169,7 +169,10 @@ describe('LibraryService', () => {
|
||||
it('create applies defaults, inserts folders, and starts an async scan', async () => {
|
||||
libraryRepo.findByName.mockResolvedValue([]);
|
||||
libraryRepo.insert.mockResolvedValue([{ id: 5, name: 'Sci-Fi', icon: 'BookOpen' }]);
|
||||
libraryRepo.insertFolder.mockResolvedValueOnce([{ id: 11, path: '/a' }]).mockResolvedValueOnce([{ id: 12, path: '/b' }]);
|
||||
libraryRepo.insertFolders.mockResolvedValue([
|
||||
{ id: 11, path: '/a' },
|
||||
{ id: 12, path: '/b' },
|
||||
]);
|
||||
|
||||
const result = await service.create({ name: 'Sci-Fi', icon: 'BookOpen', folders: ['/a', '/b'] } as any);
|
||||
|
||||
@@ -187,6 +190,10 @@ describe('LibraryService', () => {
|
||||
);
|
||||
expect(scannerService.startScanAsync).toHaveBeenCalledWith(5);
|
||||
expect(fileWatcherService.startWatcher).not.toHaveBeenCalled();
|
||||
expect(libraryRepo.insertFolders).toHaveBeenCalledWith([
|
||||
{ libraryId: 5, path: '/a' },
|
||||
{ libraryId: 5, path: '/b' },
|
||||
]);
|
||||
expect(result.folders).toEqual([
|
||||
{ id: 11, path: '/a' },
|
||||
{ id: 12, path: '/b' },
|
||||
@@ -196,7 +203,7 @@ describe('LibraryService', () => {
|
||||
it('create passes file write defaults to insert', async () => {
|
||||
libraryRepo.findByName.mockResolvedValue([]);
|
||||
libraryRepo.insert.mockResolvedValue([{ id: 5, name: 'Sci-Fi', icon: 'BookOpen' }]);
|
||||
libraryRepo.insertFolder.mockResolvedValueOnce([{ id: 11, path: '/a' }]);
|
||||
libraryRepo.insertFolders.mockResolvedValue([{ id: 11, path: '/a' }]);
|
||||
|
||||
await service.create({ name: 'Sci-Fi', icon: 'BookOpen', folders: ['/a'] } as any);
|
||||
|
||||
@@ -220,7 +227,10 @@ describe('LibraryService', () => {
|
||||
it('create starts watcher immediately when watch is enabled', async () => {
|
||||
libraryRepo.findByName.mockResolvedValue([]);
|
||||
libraryRepo.insert.mockResolvedValue([{ id: 6, name: 'Watched', icon: 'BookOpen', watch: true }]);
|
||||
libraryRepo.insertFolder.mockResolvedValueOnce([{ id: 21, path: '/watch-a' }]).mockResolvedValueOnce([{ id: 22, path: '/watch-b' }]);
|
||||
libraryRepo.insertFolders.mockResolvedValue([
|
||||
{ id: 21, path: '/watch-a' },
|
||||
{ id: 22, path: '/watch-b' },
|
||||
]);
|
||||
|
||||
await service.create({ name: 'Watched', icon: 'BookOpen', folders: ['/watch-a', '/watch-b'], watch: true } as any);
|
||||
|
||||
@@ -231,7 +241,7 @@ describe('LibraryService', () => {
|
||||
it('create registers the configured scan schedule', async () => {
|
||||
libraryRepo.findByName.mockResolvedValue([]);
|
||||
libraryRepo.insert.mockResolvedValue([{ id: 6, name: 'Scheduled', icon: 'BookOpen', watch: false }]);
|
||||
libraryRepo.insertFolder.mockResolvedValueOnce([{ id: 21, path: '/scheduled' }]);
|
||||
libraryRepo.insertFolders.mockResolvedValue([{ id: 21, path: '/scheduled' }]);
|
||||
|
||||
await service.create({
|
||||
name: 'Scheduled',
|
||||
@@ -256,7 +266,7 @@ describe('LibraryService', () => {
|
||||
await expect(service.create({ name: 'Sci-Fi', icon: 'BookOpen', folders: ['/outside'] } as any)).rejects.toBeInstanceOf(ForbiddenException);
|
||||
|
||||
expect(libraryRepo.insert).not.toHaveBeenCalled();
|
||||
expect(libraryRepo.insertFolder).not.toHaveBeenCalled();
|
||||
expect(libraryRepo.insertFolders).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('create rejects missing icons', async () => {
|
||||
@@ -282,7 +292,7 @@ describe('LibraryService', () => {
|
||||
await service.update(3, { folders: ['/keep', '/add'] } as any);
|
||||
|
||||
expect(libraryRepo.deleteFolder).toHaveBeenCalledWith(2);
|
||||
expect(libraryRepo.insertFolder).toHaveBeenCalledWith({ libraryId: 3, path: '/add' });
|
||||
expect(libraryRepo.insertFolders).toHaveBeenCalledWith([{ libraryId: 3, path: '/add' }]);
|
||||
expect(fileWatcherService.startWatcher).not.toHaveBeenCalled();
|
||||
expect(fileWatcherService.stopWatcher).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -294,7 +304,7 @@ describe('LibraryService', () => {
|
||||
await expect(service.update(3, { folders: ['/outside'] } as any)).rejects.toBeInstanceOf(ForbiddenException);
|
||||
|
||||
expect(libraryRepo.update).not.toHaveBeenCalled();
|
||||
expect(libraryRepo.insertFolder).not.toHaveBeenCalled();
|
||||
expect(libraryRepo.insertFolders).not.toHaveBeenCalled();
|
||||
expect(libraryRepo.deleteFolder).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -161,19 +161,19 @@ export class LibraryService {
|
||||
fileRenameEnabled: dto.fileRenameEnabled ?? false,
|
||||
});
|
||||
|
||||
const folders = await Promise.all(folderPaths.map((path) => this.libraryRepo.insertFolder({ libraryId: library.id, path })));
|
||||
const folders = await this.libraryRepo.insertFolders(folderPaths.map((path) => ({ libraryId: library.id, path })));
|
||||
|
||||
if (library.watch) {
|
||||
await this.fileWatcherService.startWatcher(
|
||||
library.id,
|
||||
folders.map(([f]) => f.path),
|
||||
folders.map((folder) => folder.path),
|
||||
);
|
||||
}
|
||||
|
||||
this.scanScheduler.syncSchedule(library.id, dto.autoScanCronExpression ?? null);
|
||||
this.scannerService.startScanAsync(library.id);
|
||||
|
||||
return { ...normalizeLibraryOrganizationMode(library), folders: folders.map(([f]) => f) };
|
||||
return { ...normalizeLibraryOrganizationMode(library), folders };
|
||||
}
|
||||
|
||||
async update(id: number, dto: UpdateLibraryDto) {
|
||||
@@ -213,7 +213,9 @@ export class LibraryService {
|
||||
await Promise.all(toRemove.map((f) => this.libraryRepo.deleteFolder(f.id)));
|
||||
|
||||
const toAdd = folderPaths.filter((p) => !existingByPath.has(p));
|
||||
await Promise.all(toAdd.map((path) => this.libraryRepo.insertFolder({ libraryId: id, path })));
|
||||
if (toAdd.length > 0) {
|
||||
await this.libraryRepo.insertFolders(toAdd.map((path) => ({ libraryId: id, path })));
|
||||
}
|
||||
}
|
||||
|
||||
const folders = await this.libraryRepo.findFoldersByLibrary(id);
|
||||
|
||||
Reference in New Issue
Block a user