mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(smart-scope): enable series collapsing when scope contains saved series rules
Separate transient query filter rules from saved smart scope rules when evaluating series collapsing eligibility so series collapse as expected when requested in smart scopes. Closes #780
This commit is contained in:
neon
@@ -2714,6 +2714,75 @@ describe('BookService', () => {
|
||||
expect(bookRepo.findJumpBucketsCollapsed).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('executeJumpBucketsQuery collapses when a series rule belongs to the containing scope', async () => {
|
||||
const { service, bookRepo } = makeService();
|
||||
const scopeFilter = {
|
||||
type: 'group' as const,
|
||||
join: 'OR' as const,
|
||||
rules: [{ type: 'rule' as const, field: 'series' as const, operator: 'contains' as const, value: 'Batman' }],
|
||||
};
|
||||
bookRepo.findJumpBucketsCollapsed.mockResolvedValue({ buckets: [], total: 0, kind: 'letter', granularity: null });
|
||||
|
||||
await service.executeJumpBucketsQuery(
|
||||
9,
|
||||
'WHERE' as never,
|
||||
{
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
pagination: { page: 0, size: 50 },
|
||||
maxBuckets: 24,
|
||||
collapseSeries: true,
|
||||
filter: scopeFilter,
|
||||
},
|
||||
'UTC',
|
||||
{ seriesSelectionFilter: undefined },
|
||||
);
|
||||
|
||||
expect(bookRepo.findJumpBucketsCollapsed).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ where: 'WHERE', sort: [{ field: 'title', dir: 'asc' }], userId: 9 }),
|
||||
);
|
||||
expect(bookRepo.findJumpBuckets).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('executeJumpBucketsQuery still expands when a temporary series filter is supplied separately', async () => {
|
||||
const { service, queryBuilder, bookRepo } = makeService();
|
||||
const temporaryFilter = {
|
||||
type: 'group' as const,
|
||||
join: 'AND' as const,
|
||||
rules: [{ type: 'rule' as const, field: 'series' as const, operator: 'eq' as const, value: 'Batman' }],
|
||||
};
|
||||
const effectiveFilter = {
|
||||
type: 'group' as const,
|
||||
join: 'AND' as const,
|
||||
rules: [
|
||||
{
|
||||
type: 'group' as const,
|
||||
join: 'AND' as const,
|
||||
rules: [{ type: 'rule' as const, field: 'language' as const, operator: 'eq' as const, value: 'en' }],
|
||||
},
|
||||
temporaryFilter,
|
||||
],
|
||||
};
|
||||
queryBuilder.buildOrderBy.mockReturnValue(['ORDER'] as never);
|
||||
bookRepo.findJumpBuckets.mockResolvedValue({ buckets: [], total: 0, kind: 'letter', granularity: null });
|
||||
|
||||
await service.executeJumpBucketsQuery(
|
||||
9,
|
||||
'WHERE' as never,
|
||||
{
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
pagination: { page: 0, size: 50 },
|
||||
maxBuckets: 24,
|
||||
collapseSeries: true,
|
||||
filter: effectiveFilter,
|
||||
},
|
||||
'UTC',
|
||||
{ seriesSelectionFilter: temporaryFilter },
|
||||
);
|
||||
|
||||
expect(bookRepo.findJumpBuckets).toHaveBeenCalledWith(expect.objectContaining({ where: 'WHERE', orderBy: ['ORDER'], userId: 9 }));
|
||||
expect(bookRepo.findJumpBucketsCollapsed).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('executeJumpBucketsQuery keeps collapseSeries for a series presence filter', async () => {
|
||||
const { service, bookRepo } = makeService();
|
||||
bookRepo.findJumpBucketsCollapsed.mockResolvedValue({ buckets: [], total: 0, kind: 'letter', granularity: null });
|
||||
@@ -4331,6 +4400,85 @@ describe('BookService', () => {
|
||||
expect(bookRepo.findCards).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns collapsed results when series selection rules belong to the containing scope', async () => {
|
||||
const { service, bookRepo, queryBuilder } = makeService();
|
||||
const scopeFilter = {
|
||||
type: 'group' as const,
|
||||
join: 'OR' as const,
|
||||
rules: [
|
||||
{ type: 'rule' as const, field: 'series' as const, operator: 'contains' as const, value: 'Batman' },
|
||||
{ type: 'rule' as const, field: 'series' as const, operator: 'contains' as const, value: 'Detective Comics' },
|
||||
],
|
||||
};
|
||||
bookRepo.findCardsCollapsed.mockResolvedValue(emptyCardQueryResult);
|
||||
|
||||
await service.executeBooksQuery(
|
||||
12,
|
||||
'scope-where' as never,
|
||||
{
|
||||
pagination: { page: 0, size: 50 },
|
||||
sort: [{ field: 'series', dir: 'asc' }],
|
||||
filter: scopeFilter,
|
||||
collapseSeries: true,
|
||||
},
|
||||
{ seriesSelectionFilter: undefined },
|
||||
);
|
||||
|
||||
expect(bookRepo.findCardsCollapsed).toHaveBeenCalledWith({
|
||||
where: 'scope-where',
|
||||
sort: [{ field: 'series', dir: 'asc' }],
|
||||
limit: 50,
|
||||
offset: 0,
|
||||
userId: 12,
|
||||
});
|
||||
expect(bookRepo.findCards).not.toHaveBeenCalled();
|
||||
expect(queryBuilder.buildOrderBy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns expanded results when a temporary series filter is supplied separately', async () => {
|
||||
const { service, bookRepo, queryBuilder } = makeService();
|
||||
const temporaryFilter = {
|
||||
type: 'group' as const,
|
||||
join: 'AND' as const,
|
||||
rules: [{ type: 'rule' as const, field: 'series' as const, operator: 'eq' as const, value: 'Batman' }],
|
||||
};
|
||||
const effectiveFilter = {
|
||||
type: 'group' as const,
|
||||
join: 'AND' as const,
|
||||
rules: [
|
||||
{
|
||||
type: 'group' as const,
|
||||
join: 'AND' as const,
|
||||
rules: [{ type: 'rule' as const, field: 'language' as const, operator: 'eq' as const, value: 'en' }],
|
||||
},
|
||||
temporaryFilter,
|
||||
],
|
||||
};
|
||||
queryBuilder.buildOrderBy.mockReturnValue('order-by');
|
||||
bookRepo.findCards.mockResolvedValue(emptyCardQueryResult);
|
||||
|
||||
await service.executeBooksQuery(
|
||||
12,
|
||||
'scope-where' as never,
|
||||
{
|
||||
pagination: { page: 0, size: 50 },
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
filter: effectiveFilter,
|
||||
collapseSeries: true,
|
||||
},
|
||||
{ seriesSelectionFilter: temporaryFilter },
|
||||
);
|
||||
|
||||
expect(bookRepo.findCards).toHaveBeenCalledWith({
|
||||
where: 'scope-where',
|
||||
orderBy: 'order-by',
|
||||
limit: 50,
|
||||
offset: 0,
|
||||
userId: 12,
|
||||
});
|
||||
expect(bookRepo.findCardsCollapsed).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not call logger.warn when query completes fast', async () => {
|
||||
const { service, bookRepo, queryBuilder } = makeService();
|
||||
const warnSpy = vi.spyOn((service as unknown as { logger: Logger }).logger, 'warn').mockImplementation(() => undefined);
|
||||
|
||||
@@ -49,6 +49,7 @@ import type {
|
||||
BookWriteAndRenameResult,
|
||||
BooksPage,
|
||||
FileRenameResult,
|
||||
GroupRule,
|
||||
JumpBucketsResponse,
|
||||
JumpBucketsQuery,
|
||||
MetadataFetchDiagnostics,
|
||||
@@ -94,6 +95,10 @@ import type { UpdateBookMetadataAndLocksDto } from './dto/update-book-metadata-a
|
||||
import { buildBookDetailSupplementalFields } from './utils/build-book-detail-supplemental-fields';
|
||||
import type { SetStatusDto } from '../user-book-status/dto/set-status.dto';
|
||||
|
||||
type SeriesCollapseQueryOptions = {
|
||||
seriesSelectionFilter: GroupRule | undefined;
|
||||
};
|
||||
|
||||
const METADATA_UPDATE_FAILPOINTS = [
|
||||
'afterScalarUpdate',
|
||||
'afterSeriesMembershipsReplace',
|
||||
@@ -1033,10 +1038,16 @@ export class BookService {
|
||||
return this.executeBooksQuery(user.id, where, query);
|
||||
}
|
||||
|
||||
async executeBooksQuery(userId: number, where: SQL | undefined, query: BookQuery): Promise<BooksPage> {
|
||||
async executeBooksQuery(
|
||||
userId: number,
|
||||
where: SQL | undefined,
|
||||
query: BookQuery,
|
||||
collapseOptions?: SeriesCollapseQueryOptions,
|
||||
): Promise<BooksPage> {
|
||||
const start = Date.now();
|
||||
const { page, size } = query.pagination;
|
||||
const shouldCollapse = query.collapseSeries === true && !BookQueryBuilder.hasSeriesSelectionFilter(query.filter);
|
||||
const seriesSelectionFilter = collapseOptions ? collapseOptions.seriesSelectionFilter : query.filter;
|
||||
const shouldCollapse = query.collapseSeries === true && !BookQueryBuilder.hasSeriesSelectionFilter(seriesSelectionFilter);
|
||||
|
||||
if (shouldCollapse) {
|
||||
const { rows, authorRows, fileRows, genreRows, tagRows, progressRows, statusRows, narratorRows, seriesMembershipRows, total } =
|
||||
@@ -1124,12 +1135,19 @@ export class BookService {
|
||||
return this.executeJumpBucketsQuery(user.id, where, query, timeZone);
|
||||
}
|
||||
|
||||
async executeJumpBucketsQuery(userId: number, where: SQL | undefined, query: JumpBucketsQuery, timeZone = 'UTC'): Promise<JumpBucketsResponse> {
|
||||
async executeJumpBucketsQuery(
|
||||
userId: number,
|
||||
where: SQL | undefined,
|
||||
query: JumpBucketsQuery,
|
||||
timeZone = 'UTC',
|
||||
collapseOptions?: SeriesCollapseQueryOptions,
|
||||
): Promise<JumpBucketsResponse> {
|
||||
const event = 'book.jump_buckets';
|
||||
const strategy = jumpRailStrategyForSort(query.sort);
|
||||
const kind = strategy?.kind ?? null;
|
||||
const primary = query.sort[0] ?? { field: 'title' as const, dir: 'asc' as const };
|
||||
const shouldCollapse = query.collapseSeries === true && !BookQueryBuilder.hasSeriesSelectionFilter(query.filter);
|
||||
const seriesSelectionFilter = collapseOptions ? collapseOptions.seriesSelectionFilter : query.filter;
|
||||
const shouldCollapse = query.collapseSeries === true && !BookQueryBuilder.hasSeriesSelectionFilter(seriesSelectionFilter);
|
||||
if (!strategy) throw new BadRequestException('jump buckets are not available for this sort');
|
||||
|
||||
const start = Date.now();
|
||||
|
||||
@@ -324,11 +324,16 @@ describe('SmartScopeService', () => {
|
||||
timeZone: 'UTC',
|
||||
contentFilters: EMPTY_CONTENT_FILTER_RULES,
|
||||
});
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(12, 'where', {
|
||||
filter: smartScope.filter,
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
pagination: { page: 1, size: 25 },
|
||||
});
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(
|
||||
12,
|
||||
'where',
|
||||
{
|
||||
filter: smartScope.filter,
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
pagination: { page: 1, size: 25 },
|
||||
},
|
||||
{ seriesSelectionFilter: undefined },
|
||||
);
|
||||
expect(result).toEqual({ items: [], total: 0, page: 1, size: 25 });
|
||||
});
|
||||
|
||||
@@ -365,17 +370,22 @@ describe('SmartScopeService', () => {
|
||||
},
|
||||
{ accessibleLibraryIds: [9], userId: 12, q: 'needle', timeZone: 'UTC', contentFilters: EMPTY_CONTENT_FILTER_RULES },
|
||||
);
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(12, 'combined-where', {
|
||||
filter: {
|
||||
type: 'group',
|
||||
join: 'AND',
|
||||
rules: [smartScope.filter, requestFilter],
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(
|
||||
12,
|
||||
'combined-where',
|
||||
{
|
||||
filter: {
|
||||
type: 'group',
|
||||
join: 'AND',
|
||||
rules: [smartScope.filter, requestFilter],
|
||||
},
|
||||
sort: [{ field: 'author', dir: 'desc' }],
|
||||
pagination: { page: 0, size: 50 },
|
||||
q: 'needle',
|
||||
collapseSeries: true,
|
||||
},
|
||||
sort: [{ field: 'author', dir: 'desc' }],
|
||||
pagination: { page: 0, size: 50 },
|
||||
q: 'needle',
|
||||
collapseSeries: true,
|
||||
});
|
||||
{ seriesSelectionFilter: requestFilter },
|
||||
);
|
||||
});
|
||||
|
||||
describe('queryJumpBuckets', () => {
|
||||
@@ -420,10 +430,48 @@ describe('SmartScopeService', () => {
|
||||
'where',
|
||||
expect.objectContaining({ sort: [{ field: 'author', dir: 'desc' }] }),
|
||||
'UTC',
|
||||
{ seriesSelectionFilter: undefined },
|
||||
);
|
||||
expect(result).toEqual({ buckets: [{ key: 'A', label: 'A', index: 0 }], total: 3, kind: 'letter', granularity: null });
|
||||
});
|
||||
|
||||
it('keeps a saved series rule out of collapse eligibility while retaining it in the jump-bucket query', async () => {
|
||||
const { service, smartScopeRepo, libraryService, queryBuilder, bookService } = makeService();
|
||||
const smartScope = makeSmartScope({
|
||||
filter: {
|
||||
type: 'group',
|
||||
join: 'OR',
|
||||
rules: [{ type: 'rule', field: 'series', operator: 'contains', value: 'Batman' }],
|
||||
},
|
||||
});
|
||||
smartScopeRepo.findById.mockResolvedValue([smartScope]);
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([9]);
|
||||
queryBuilder.buildWhere.mockReturnValue('series-scope-where');
|
||||
bookService.executeJumpBucketsQuery.mockResolvedValue({
|
||||
buckets: [],
|
||||
total: 0,
|
||||
kind: 'letter',
|
||||
granularity: null,
|
||||
});
|
||||
|
||||
await service.queryJumpBuckets(5, makeUser(), {
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
pagination: { page: 0, size: 50 },
|
||||
collapseSeries: true,
|
||||
});
|
||||
|
||||
expect(bookService.executeJumpBucketsQuery).toHaveBeenCalledWith(
|
||||
12,
|
||||
'series-scope-where',
|
||||
expect.objectContaining({
|
||||
filter: smartScope.filter,
|
||||
collapseSeries: true,
|
||||
}),
|
||||
'UTC',
|
||||
{ seriesSelectionFilter: undefined },
|
||||
);
|
||||
});
|
||||
|
||||
it('denies access to private scopes of other users', async () => {
|
||||
const { service, smartScopeRepo, bookService } = makeService();
|
||||
smartScopeRepo.findById.mockResolvedValue([makeSmartScope({ id: 5, userId: 99, isPublic: false, filter: null })]);
|
||||
@@ -488,14 +536,53 @@ describe('SmartScopeService', () => {
|
||||
},
|
||||
{ accessibleLibraryIds: [9], userId: 12, q: 'needle', timeZone: 'UTC', contentFilters: EMPTY_CONTENT_FILTER_RULES },
|
||||
);
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(12, 'combined-where', {
|
||||
...query,
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(
|
||||
12,
|
||||
'combined-where',
|
||||
{
|
||||
...query,
|
||||
filter: {
|
||||
type: 'group',
|
||||
join: 'AND',
|
||||
rules: [smartScope.filter, requestFilter],
|
||||
},
|
||||
},
|
||||
{ seriesSelectionFilter: requestFilter },
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps a saved series rule out of collapse eligibility while retaining it in the books query', async () => {
|
||||
const { service, smartScopeRepo, libraryService, queryBuilder, bookService } = makeService();
|
||||
const smartScope = makeSmartScope({
|
||||
filter: {
|
||||
type: 'group',
|
||||
join: 'AND',
|
||||
rules: [smartScope.filter, requestFilter],
|
||||
join: 'OR',
|
||||
rules: [
|
||||
{ type: 'rule', field: 'series', operator: 'contains', value: 'Batman' },
|
||||
{ type: 'rule', field: 'series', operator: 'contains', value: 'Detective Comics' },
|
||||
],
|
||||
},
|
||||
});
|
||||
smartScopeRepo.findById.mockResolvedValue([smartScope]);
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([9]);
|
||||
queryBuilder.buildWhere.mockReturnValue('series-scope-where');
|
||||
bookService.executeBooksQuery.mockResolvedValue({ items: [], total: 0, page: 0, size: 50 });
|
||||
|
||||
await service.queryBooks(5, makeUser(), {
|
||||
sort: [{ field: 'series', dir: 'asc' }],
|
||||
pagination: { page: 0, size: 50 },
|
||||
collapseSeries: true,
|
||||
});
|
||||
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(
|
||||
12,
|
||||
'series-scope-where',
|
||||
expect.objectContaining({
|
||||
filter: smartScope.filter,
|
||||
collapseSeries: true,
|
||||
}),
|
||||
{ seriesSelectionFilter: undefined },
|
||||
);
|
||||
});
|
||||
|
||||
it('uses the smartScope defaultSort when the query has no sort', async () => {
|
||||
@@ -517,11 +604,16 @@ describe('SmartScopeService', () => {
|
||||
|
||||
await service.queryBooks(5, makeUser({ id: 12 }), query);
|
||||
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(12, 'scope-where', {
|
||||
...query,
|
||||
filter: smartScope.filter,
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
});
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(
|
||||
12,
|
||||
'scope-where',
|
||||
{
|
||||
...query,
|
||||
filter: smartScope.filter,
|
||||
sort: [{ field: 'title', dir: 'asc' }],
|
||||
},
|
||||
{ seriesSelectionFilter: undefined },
|
||||
);
|
||||
});
|
||||
|
||||
it('logs a warning when queryBooks takes at least 500ms', async () => {
|
||||
|
||||
@@ -165,7 +165,9 @@ export class SmartScopeService {
|
||||
const { where, effectiveQuery } = prepared;
|
||||
|
||||
try {
|
||||
const result = await this.bookService.executeBooksQuery(user.id, where, effectiveQuery);
|
||||
const result = await this.bookService.executeBooksQuery(user.id, where, effectiveQuery, {
|
||||
seriesSelectionFilter: query.filter,
|
||||
});
|
||||
const durationMs = Date.now() - start;
|
||||
if (durationMs >= 500) {
|
||||
this.logger.warn(
|
||||
@@ -190,7 +192,9 @@ export class SmartScopeService {
|
||||
// Eligibility is validated by the book service against effectiveQuery.sort,
|
||||
// i.e. after the scope's defaultSort has been resolved.
|
||||
const timeZone = resolveTimeZone((user.settings as { timezone?: unknown } | undefined)?.timezone, 'UTC');
|
||||
return this.bookService.executeJumpBucketsQuery(user.id, prepared.where, prepared.effectiveQuery, timeZone);
|
||||
return this.bookService.executeJumpBucketsQuery(user.id, prepared.where, prepared.effectiveQuery, timeZone, {
|
||||
seriesSelectionFilter: query.filter,
|
||||
});
|
||||
}
|
||||
|
||||
private async prepareBooksQuery<T extends BookQuery>(
|
||||
|
||||
Reference in New Issue
Block a user