mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
feat: overhaul series detail view and improve card interactions
This commit is contained in:
@@ -215,6 +215,7 @@ export class CollectionService {
|
||||
const qWhere = q?.trim() ? this.queryBuilder.buildQuickSearch(q.trim()) : undefined;
|
||||
|
||||
if (collapseSeries) {
|
||||
const MAX_COLLAPSE_BOOKS = 5000;
|
||||
const allBookIds = await this.collectionRepo.findAllBookIds(id, libraryIds, qWhere);
|
||||
if (allBookIds.length === 0) {
|
||||
this.logger.log(
|
||||
@@ -223,6 +224,25 @@ export class CollectionService {
|
||||
return { items: [], total: 0, page, size };
|
||||
}
|
||||
|
||||
if (allBookIds.length > MAX_COLLAPSE_BOOKS) {
|
||||
this.logger.warn(
|
||||
`[${event}] [end] collectionId=${id} durationMs=${Date.now() - startedAt} bookCount=${allBookIds.length} cap=${MAX_COLLAPSE_BOOKS} - falling back to non-collapsed pagination`,
|
||||
);
|
||||
const bookPage = await this.collectionRepo.findBookIdsPage(id, libraryIds, page, size, qWhere);
|
||||
if (bookPage.bookIds.length === 0) {
|
||||
return { items: [], total: bookPage.total, page, size };
|
||||
}
|
||||
const { rows, authorRows, fileRows, genreRows, progressRows, statusRows } = await this.bookReadService.findCardsByBookIds(
|
||||
bookPage.bookIds,
|
||||
user.id,
|
||||
);
|
||||
const orderMap = new Map(bookPage.bookIds.map((bookId, index) => [bookId, index]));
|
||||
const items = assembleBookCards(rows, authorRows, fileRows, genreRows, progressRows, statusRows).sort(
|
||||
(left, right) => (orderMap.get(left.id) ?? 0) - (orderMap.get(right.id) ?? 0),
|
||||
);
|
||||
return { items, total: bookPage.total, page, size };
|
||||
}
|
||||
|
||||
const { rows, authorRows, fileRows, genreRows, progressRows, statusRows } = await this.bookReadService.findCardsByBookIds(
|
||||
allBookIds,
|
||||
user.id,
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import 'reflect-metadata';
|
||||
|
||||
import { BadRequestException } from '@nestjs/common';
|
||||
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { SeriesController } from './series.controller';
|
||||
|
||||
@@ -45,7 +47,7 @@ describe('SeriesController', () => {
|
||||
expect(result).toBe(expected);
|
||||
});
|
||||
|
||||
it('findBooks delegates to service with raw param', async () => {
|
||||
it('findBooks delegates to service with decoded param', async () => {
|
||||
const { controller, seriesService } = makeController();
|
||||
const user = makeUser();
|
||||
const dto = { page: 0, size: 50 };
|
||||
@@ -64,14 +66,62 @@ describe('SeriesController', () => {
|
||||
expect(result).toBe(expected);
|
||||
});
|
||||
|
||||
it('findBooks passes param as-is without decoding', async () => {
|
||||
const { controller, seriesService } = makeController();
|
||||
const user = makeUser();
|
||||
const dto = { page: 0, size: 50 };
|
||||
seriesService.findBooks.mockResolvedValue({ items: [] });
|
||||
describe('seriesName validation', () => {
|
||||
it('trims whitespace from seriesName', async () => {
|
||||
const { controller, seriesService } = makeController();
|
||||
seriesService.findBooks.mockResolvedValue({ items: [] });
|
||||
|
||||
await controller.findBooks(user, 'Simple Series', dto as any);
|
||||
await controller.findBooks(makeUser(), ' Harry Potter ', {} as any);
|
||||
expect(seriesService.findBooks).toHaveBeenCalledWith(expect.anything(), 'Harry Potter', expect.anything());
|
||||
});
|
||||
|
||||
expect(seriesService.findBooks).toHaveBeenCalledWith(user, 'Simple Series', dto);
|
||||
it('passes already-decoded param as-is', async () => {
|
||||
const { controller, seriesService } = makeController();
|
||||
seriesService.findBooks.mockResolvedValue({ items: [] });
|
||||
|
||||
await controller.findBooks(makeUser(), 'Harry Potter', {} as any);
|
||||
expect(seriesService.findBooks).toHaveBeenCalledWith(expect.anything(), 'Harry Potter', expect.anything());
|
||||
});
|
||||
|
||||
it('handles names with special characters', async () => {
|
||||
const { controller, seriesService } = makeController();
|
||||
seriesService.findBooks.mockResolvedValue({ items: [] });
|
||||
|
||||
await controller.findBooks(makeUser(), 'A & B / C', {} as any);
|
||||
expect(seriesService.findBooks).toHaveBeenCalledWith(expect.anything(), 'A & B / C', expect.anything());
|
||||
});
|
||||
|
||||
it('handles names with percent sign', async () => {
|
||||
const { controller, seriesService } = makeController();
|
||||
seriesService.findBooks.mockResolvedValue({ items: [] });
|
||||
|
||||
await controller.findBooks(makeUser(), '100% Complete', {} as any);
|
||||
expect(seriesService.findBooks).toHaveBeenCalledWith(expect.anything(), '100% Complete', expect.anything());
|
||||
});
|
||||
|
||||
it('throws BadRequestException for empty seriesName', () => {
|
||||
const { controller } = makeController();
|
||||
expect(() => controller.findBooks(makeUser(), '', {} as any)).toThrow(BadRequestException);
|
||||
});
|
||||
|
||||
it('throws BadRequestException for whitespace-only seriesName', () => {
|
||||
const { controller } = makeController();
|
||||
expect(() => controller.findBooks(makeUser(), ' ', {} as any)).toThrow(BadRequestException);
|
||||
});
|
||||
|
||||
it('throws BadRequestException for seriesName exceeding max length', () => {
|
||||
const { controller } = makeController();
|
||||
const longName = 'A'.repeat(501);
|
||||
expect(() => controller.findBooks(makeUser(), longName, {} as any)).toThrow(BadRequestException);
|
||||
});
|
||||
|
||||
it('accepts seriesName at max length boundary', async () => {
|
||||
const { controller, seriesService } = makeController();
|
||||
seriesService.findBooks.mockResolvedValue({ items: [] });
|
||||
const name = 'A'.repeat(500);
|
||||
|
||||
await controller.findBooks(makeUser(), name, {} as any);
|
||||
expect(seriesService.findBooks).toHaveBeenCalledWith(expect.anything(), name, expect.anything());
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Controller, Get, Param, Query } from '@nestjs/common';
|
||||
import { BadRequestException, Controller, Get, Param, Query } from '@nestjs/common';
|
||||
|
||||
import { CurrentUser } from '../../common/decorators/current-user.decorator';
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
@@ -6,6 +6,8 @@ import { ListSeriesBooksDto } from './dto/list-series-books.dto';
|
||||
import { ListSeriesDto } from './dto/list-series.dto';
|
||||
import { SeriesService } from './series.service';
|
||||
|
||||
const MAX_SERIES_NAME_LENGTH = 500;
|
||||
|
||||
@Controller('series')
|
||||
export class SeriesController {
|
||||
constructor(private readonly seriesService: SeriesService) {}
|
||||
@@ -16,7 +18,14 @@ export class SeriesController {
|
||||
}
|
||||
|
||||
@Get(':seriesName/books')
|
||||
findBooks(@CurrentUser() user: RequestUser, @Param('seriesName') seriesName: string, @Query() dto: ListSeriesBooksDto) {
|
||||
findBooks(@CurrentUser() user: RequestUser, @Param('seriesName') rawSeriesName: string, @Query() dto: ListSeriesBooksDto) {
|
||||
const seriesName = rawSeriesName.trim();
|
||||
if (!seriesName) {
|
||||
throw new BadRequestException('seriesName must not be empty');
|
||||
}
|
||||
if (seriesName.length > MAX_SERIES_NAME_LENGTH) {
|
||||
throw new BadRequestException(`seriesName must not exceed ${MAX_SERIES_NAME_LENGTH} characters`);
|
||||
}
|
||||
return this.seriesService.findBooks(user, seriesName, dto);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -39,6 +39,10 @@ export class SeriesRepository {
|
||||
return inArray(books.libraryId, libraryIds);
|
||||
}
|
||||
|
||||
private escapeLikePattern(s: string): string {
|
||||
return s.replace(/[\\%_]/g, '\\$&');
|
||||
}
|
||||
|
||||
async findPage(params: {
|
||||
q?: string;
|
||||
page: number;
|
||||
@@ -56,15 +60,16 @@ export class SeriesRepository {
|
||||
const conditions: SQL[] = [isNotNull(bookMetadata.seriesName), sql`btrim(${bookMetadata.seriesName}) != ''`, libraryFilter];
|
||||
|
||||
if (params.q) {
|
||||
conditions.push(ilike(bookMetadata.seriesName, `%${params.q}%`));
|
||||
conditions.push(ilike(bookMetadata.seriesName, `%${this.escapeLikePattern(params.q)}%`));
|
||||
}
|
||||
|
||||
if (params.author) {
|
||||
const authorPattern = `%${this.escapeLikePattern(params.author)}%`;
|
||||
conditions.push(
|
||||
sql`${books.id} IN (
|
||||
SELECT ${bookAuthors.bookId} FROM ${bookAuthors}
|
||||
INNER JOIN ${authors} ON ${authors.id} = ${bookAuthors.authorId}
|
||||
WHERE ${ilike(authors.name, `%${params.author}%`)}
|
||||
WHERE ${ilike(authors.name, authorPattern)}
|
||||
)`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -240,39 +240,71 @@ describe('SeriesService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('computeGaps (via findBooks)', () => {
|
||||
describe('findBooks - library filter empty state', () => {
|
||||
it('returns empty state when series exists in another library', async () => {
|
||||
seriesRepo.findDetail
|
||||
.mockResolvedValueOnce(null) // first call with scoped library [2]
|
||||
.mockResolvedValueOnce({ name: 'Dune', bookCount: 5, readCount: 2, authors: ['Frank Herbert'], indices: [1, 2, 3, 4, 5] }); // second call with all libraries [1, 2]
|
||||
seriesRepo.findBookIds.mockResolvedValue({ bookIds: [], total: 0 });
|
||||
|
||||
const result = await service.findBooks(reqUser(), 'Dune', { libraryId: 2 });
|
||||
|
||||
expect(result.items).toEqual([]);
|
||||
expect(result.total).toBe(0);
|
||||
expect(result.seriesInfo.name).toBe('Dune');
|
||||
expect(result.seriesInfo.authors).toEqual(['Frank Herbert']);
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([]);
|
||||
expect(result.seriesInfo.bookCount).toBe(0);
|
||||
});
|
||||
|
||||
it('throws 404 when series does not exist in any library', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue(null);
|
||||
seriesRepo.findBookIds.mockResolvedValue({ bookIds: [], total: 0 });
|
||||
|
||||
await expect(service.findBooks(reqUser(), 'Nonexistent', { libraryId: 1 })).rejects.toBeInstanceOf(NotFoundException);
|
||||
});
|
||||
|
||||
it('throws 404 when no library filter and series not found', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue(null);
|
||||
seriesRepo.findBookIds.mockResolvedValue({ bookIds: [], total: 0 });
|
||||
|
||||
await expect(service.findBooks(reqUser(), 'Nonexistent', {})).rejects.toBeInstanceOf(NotFoundException);
|
||||
});
|
||||
});
|
||||
|
||||
describe('computeGaps edge cases', () => {
|
||||
beforeEach(() => {
|
||||
seriesRepo.findBookIds.mockResolvedValue({ bookIds: [], total: 0 });
|
||||
});
|
||||
|
||||
it('detects integer gaps', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 3, readCount: 0, authors: [], indices: [1, 2, 5] });
|
||||
const result = await service.findBooks(reqUser(), 'S', {});
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([3, 4]);
|
||||
});
|
||||
|
||||
it('ignores decimal indices for gap detection', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 3, readCount: 0, authors: [], indices: [1, 1.5, 3] });
|
||||
const result = await service.findBooks(reqUser(), 'S', {});
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([2]);
|
||||
});
|
||||
|
||||
it('returns empty gaps for single-book series', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 1, readCount: 0, authors: [], indices: [1] });
|
||||
it('returns empty gaps when all indices are non-integer', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 3, readCount: 0, authors: [], indices: [0.5, 1.5, 2.5] });
|
||||
const result = await service.findBooks(reqUser(), 'S', {});
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns empty gaps for continuous series', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 3, readCount: 0, authors: [], indices: [1, 2, 3] });
|
||||
it('returns empty gaps when min index < 1', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 2, readCount: 0, authors: [], indices: [0, 5] });
|
||||
const result = await service.findBooks(reqUser(), 'S', {});
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([]);
|
||||
});
|
||||
|
||||
it('handles near-integer floating point values', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 2, readCount: 0, authors: [], indices: [1.001, 3.002] });
|
||||
it('returns empty gaps when max index > 10000', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 2, readCount: 0, authors: [], indices: [1, 10001] });
|
||||
const result = await service.findBooks(reqUser(), 'S', {});
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([]);
|
||||
});
|
||||
|
||||
it('handles duplicate indices', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 3, readCount: 0, authors: [], indices: [1, 1, 3] });
|
||||
const result = await service.findBooks(reqUser(), 'S', {});
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([2]);
|
||||
});
|
||||
|
||||
it('handles empty indices array', async () => {
|
||||
seriesRepo.findDetail.mockResolvedValue({ name: 'S', bookCount: 0, readCount: 0, authors: [], indices: [] });
|
||||
const result = await service.findBooks(reqUser(), 'S', {});
|
||||
expect(result.seriesInfo.possibleGaps).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -83,6 +83,20 @@ export class SeriesService {
|
||||
]);
|
||||
|
||||
if (!detail) {
|
||||
if (dto.libraryId) {
|
||||
const allLibraryIds = await this.resolveLibraryIds(user);
|
||||
const existsInAnyLibrary = await this.seriesRepo.findDetail({ seriesName, userId: user.id, libraryIds: allLibraryIds });
|
||||
if (existsInAnyLibrary) {
|
||||
const emptyInfo: SeriesDetail = {
|
||||
name: existsInAnyLibrary.name,
|
||||
bookCount: 0,
|
||||
readCount: 0,
|
||||
authors: existsInAnyLibrary.authors,
|
||||
possibleGaps: [],
|
||||
};
|
||||
return { items: [], total: 0, page, size, seriesInfo: emptyInfo };
|
||||
}
|
||||
}
|
||||
throw new NotFoundException('Series not found');
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
import 'reflect-metadata';
|
||||
|
||||
import { validate } from 'class-validator';
|
||||
import { plainToInstance } from 'class-transformer';
|
||||
import { UpdateSeriesCollapsePreferencesDto } from './update-series-collapse-preferences.dto';
|
||||
|
||||
function toDto(plain: Record<string, unknown>): UpdateSeriesCollapsePreferencesDto {
|
||||
return plainToInstance(UpdateSeriesCollapsePreferencesDto, plain);
|
||||
}
|
||||
|
||||
describe('UpdateSeriesCollapsePreferencesDto', () => {
|
||||
it('accepts valid boolean global', async () => {
|
||||
const dto = toDto({ global: true });
|
||||
const errors = await validate(dto);
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('accepts valid boolean libraries', async () => {
|
||||
const dto = toDto({ libraries: { '1': true, '2': false } });
|
||||
const errors = await validate(dto);
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('accepts null values in libraries for deletion', async () => {
|
||||
const dto = toDto({ libraries: { '1': null, '2': true } });
|
||||
const errors = await validate(dto);
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('accepts null values in collections for deletion', async () => {
|
||||
const dto = toDto({ collections: { '5': null } });
|
||||
const errors = await validate(dto);
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('rejects non-boolean/non-null values in libraries', async () => {
|
||||
const dto = toDto({ libraries: { '1': 'yes' } });
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('rejects number values in collections', async () => {
|
||||
const dto = toDto({ collections: { '1': 42 } });
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('rejects array for libraries', async () => {
|
||||
const dto = toDto({ libraries: [true, false] });
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('accepts empty dto (all optional)', async () => {
|
||||
const dto = toDto({});
|
||||
const errors = await validate(dto);
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('rejects non-boolean global', async () => {
|
||||
const dto = toDto({ global: 'true' });
|
||||
const errors = await validate(dto);
|
||||
expect(errors.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
@@ -1,19 +1,19 @@
|
||||
import { IsBoolean, IsOptional, registerDecorator, ValidationOptions } from 'class-validator';
|
||||
|
||||
function IsBooleanRecord(validationOptions?: ValidationOptions) {
|
||||
function IsBooleanOrNullRecord(validationOptions?: ValidationOptions) {
|
||||
return (object: object, propertyName: string) => {
|
||||
registerDecorator({
|
||||
name: 'isBooleanRecord',
|
||||
name: 'isBooleanOrNullRecord',
|
||||
target: object.constructor,
|
||||
propertyName,
|
||||
options: validationOptions,
|
||||
validator: {
|
||||
validate(value: unknown) {
|
||||
if (typeof value !== 'object' || value === null || Array.isArray(value)) return false;
|
||||
return Object.values(value as Record<string, unknown>).every((v) => typeof v === 'boolean');
|
||||
return Object.values(value as Record<string, unknown>).every((v) => typeof v === 'boolean' || v === null);
|
||||
},
|
||||
defaultMessage() {
|
||||
return `${propertyName} must be a record with string keys and boolean values`;
|
||||
return `${propertyName} must be a record with string keys and boolean or null values`;
|
||||
},
|
||||
},
|
||||
});
|
||||
@@ -26,10 +26,10 @@ export class UpdateSeriesCollapsePreferencesDto {
|
||||
global?: boolean;
|
||||
|
||||
@IsOptional()
|
||||
@IsBooleanRecord()
|
||||
libraries?: Record<string, boolean>;
|
||||
@IsBooleanOrNullRecord()
|
||||
libraries?: Record<string, boolean | null>;
|
||||
|
||||
@IsOptional()
|
||||
@IsBooleanRecord()
|
||||
collections?: Record<string, boolean>;
|
||||
@IsBooleanOrNullRecord()
|
||||
collections?: Record<string, boolean | null>;
|
||||
}
|
||||
|
||||
@@ -265,6 +265,15 @@ export class UserService {
|
||||
libraries: { ...currentPrefs.libraries, ...(dto.libraries ?? {}) },
|
||||
collections: { ...currentPrefs.collections, ...(dto.collections ?? {}) },
|
||||
};
|
||||
|
||||
// Remove entries set to null (deletion of overrides)
|
||||
for (const [k, v] of Object.entries(merged.libraries)) {
|
||||
if (v === null) delete merged.libraries[k];
|
||||
}
|
||||
for (const [k, v] of Object.entries(merged.collections)) {
|
||||
if (v === null) delete merged.collections[k];
|
||||
}
|
||||
|
||||
await this.userRepo.update(userId, { settings: { seriesCollapsePreferences: merged } });
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user