fix(smart-scope): include public collections in filters

Honor public collection visibility when evaluating and suggesting collection filter values for viewers.

Closes #2010
This commit is contained in:
neon
2026-10-31 02:18:11 +00:00
parent ee2974459a
commit ba315c7718
5 changed files with 31 additions and 10 deletions
@@ -50,7 +50,7 @@ import {
} from '@bookorbit/types';
import { accentInsensitiveIlike } from '../../common/utils/accent-insensitive-search.utils';
import { bookCustomMetadataValues, bookMetadata, books } from '../../db/schema';
import { bookCustomMetadataValues, bookMetadata, books, collections } from '../../db/schema';
import { BookQueryBuilder } from './book-query-builder.service';
import { BookSortBuilder } from './book-sort-builder.service';
@@ -61,7 +61,7 @@ function makeBuilder() {
const db = {
select: vi.fn().mockReturnValue({ from: sqFrom }),
};
return { builder: new BookQueryBuilder(db as never, new BookSortBuilder()), db };
return { builder: new BookQueryBuilder(db as never, new BookSortBuilder()), db, sqInnerJoin };
}
function wrapRule(rule: Record<string, unknown>) {
@@ -1160,13 +1160,28 @@ describe('tagRuleToSql', () => {
});
describe('collectionRuleToSql', () => {
it('includesAny produces user-scoped EXISTS subquery', () => {
const { builder } = makeBuilder();
it('includesAny matches collections owned by the user or shared publicly', () => {
const { builder, sqInnerJoin } = makeBuilder();
const where = builder.buildWhere(
wrapRule({ type: 'rule', field: 'collection', operator: 'includesAny', value: ['Reading'] }) as never,
USER_CTX,
) as any;
expect(getRuleSql(where)).toMatchObject({ type: 'sql' });
expect(sqInnerJoin).toHaveBeenCalledWith(
collections,
expect.objectContaining({
type: 'and',
clauses: expect.arrayContaining([
expect.objectContaining({
type: 'or',
clauses: [
expect.objectContaining({ type: 'eq', left: collections.userId, right: USER_CTX.userId }),
expect.objectContaining({ type: 'eq', left: collections.isPublic, right: true }),
],
}),
]),
}),
);
});
it('excludesAll with values produces not(EXISTS)', () => {
@@ -1076,7 +1076,10 @@ export class BookQueryBuilder {
const sq = this.db
.select({ one: sql`1` })
.from(collectionBooks)
.innerJoin(collections, and(eq(collectionBooks.collectionId, collections.id), eq(collections.userId, userId)))
.innerJoin(
collections,
and(eq(collectionBooks.collectionId, collections.id), or(eq(collections.userId, userId), eq(collections.isPublic, true))),
)
.where(and(...predicates)!);
return sql`exists (${sq})`;
};
@@ -2,6 +2,7 @@ vi.mock('drizzle-orm', () => ({
and: vi.fn((...clauses: unknown[]) => ({ type: 'and', clauses })),
desc: vi.fn((value: unknown) => ({ type: 'desc', value })),
eq: vi.fn((left: unknown, right: unknown) => ({ type: 'eq', left, right })),
or: vi.fn((...clauses: unknown[]) => ({ type: 'or', clauses })),
inArray: vi.fn((column: unknown, values: unknown[]) => ({ type: 'inArray', column, values })),
isNotNull: vi.fn((value: unknown) => ({ type: 'isNotNull', value })),
sql: vi.fn((parts: TemplateStringsArray, ...values: unknown[]) => ({ type: 'sql', parts, values })),
@@ -17,7 +18,7 @@ vi.mock('../../common/utils/content-filter-sql.utils', () => ({
buildContentFilterClauses: vi.fn(() => [{ type: 'contentFilter' }]),
}));
import { and, desc, eq, inArray, isNotNull } from 'drizzle-orm';
import { and, desc, eq, inArray, isNotNull, or } from 'drizzle-orm';
import { EMPTY_CONTENT_FILTER_RULES } from '@bookorbit/types';
import { accentInsensitiveExactMatchRank, accentInsensitiveIlike } from '../../common/utils/accent-insensitive-search.utils';
@@ -172,12 +173,14 @@ describe('CatalogRepository', () => {
expect(buildContentFilterClauses).not.toHaveBeenCalled();
});
it('keeps collection lookup user-scoped and wildcard-safe', async () => {
it('searches owned and public collections with wildcard-safe matching', async () => {
const { repository, selectChain } = makeRepository([{ name: 'Favorites' }]);
await expect(repository.searchCollections(42, ' sci_fi% ')).resolves.toEqual([{ name: 'Favorites' }]);
expect(eq).toHaveBeenCalledWith(collections.userId, 42);
expect(eq).toHaveBeenCalledWith(collections.isPublic, true);
expect(or).toHaveBeenCalledWith(expect.anything(), expect.anything());
expect(accentInsensitiveIlike).toHaveBeenCalledWith(collections.name, '%sci\\_fi\\%%');
expect(and).toHaveBeenCalled();
expect(selectChain.from).toHaveBeenCalledWith(collections);
@@ -1,5 +1,5 @@
import { Inject, Injectable } from '@nestjs/common';
import { and, desc, eq, inArray, isNotNull, sql } from 'drizzle-orm';
import { and, desc, eq, inArray, isNotNull, or, sql } from 'drizzle-orm';
import type { SQL } from 'drizzle-orm';
import type { NodePgDatabase } from 'drizzle-orm/node-postgres';
import type { PgColumn, PgTable } from 'drizzle-orm/pg-core';
@@ -107,7 +107,7 @@ export class CatalogRepository {
return this.db
.select({ name: collections.name })
.from(collections)
.where(and(eq(collections.userId, userId), accentInsensitiveIlike(collections.name, pattern)))
.where(and(or(eq(collections.userId, userId), eq(collections.isPublic, true)), accentInsensitiveIlike(collections.name, pattern)))
.orderBy(collections.name)
.limit(COLLECTION_SEARCH_LIMIT);
}
@@ -132,7 +132,7 @@ describe('CatalogService', () => {
expect(catalogRepository.listLanguages).not.toHaveBeenCalled();
});
it('keeps collection searches scoped directly by owner id', async () => {
it('delegates readable collection searches by viewer id', async () => {
const { service, catalogRepository, libraryService } = makeService();
catalogRepository.searchCollections.mockResolvedValue([{ name: 'Favorites' }]);