From ba315c77182f0f28030140e342d93294cc512637 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Sat, 31 Oct 2026 02:18:11 +0000 Subject: [PATCH] fix(smart-scope): include public collections in filters Honor public collection visibility when evaluating and suggesting collection filter values for viewers. Closes #2010 --- .../book/book-query-builder.service.test.ts | 23 +++++++++++++++---- .../book/book-query-builder.service.ts | 5 +++- .../catalog/catalog.repository.test.ts | 7 ++++-- .../src/modules/catalog/catalog.repository.ts | 4 ++-- .../modules/catalog/catalog.service.test.ts | 2 +- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/server/src/modules/book/book-query-builder.service.test.ts b/server/src/modules/book/book-query-builder.service.test.ts index debaf87cf..27e0978ad 100644 --- a/server/src/modules/book/book-query-builder.service.test.ts +++ b/server/src/modules/book/book-query-builder.service.test.ts @@ -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) { @@ -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)', () => { diff --git a/server/src/modules/book/book-query-builder.service.ts b/server/src/modules/book/book-query-builder.service.ts index bf4547550..08bbc10b1 100644 --- a/server/src/modules/book/book-query-builder.service.ts +++ b/server/src/modules/book/book-query-builder.service.ts @@ -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})`; }; diff --git a/server/src/modules/catalog/catalog.repository.test.ts b/server/src/modules/catalog/catalog.repository.test.ts index 854dcbfd8..6c8557c1d 100644 --- a/server/src/modules/catalog/catalog.repository.test.ts +++ b/server/src/modules/catalog/catalog.repository.test.ts @@ -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); diff --git a/server/src/modules/catalog/catalog.repository.ts b/server/src/modules/catalog/catalog.repository.ts index 4fe1ec08a..edab3fd76 100644 --- a/server/src/modules/catalog/catalog.repository.ts +++ b/server/src/modules/catalog/catalog.repository.ts @@ -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); } diff --git a/server/src/modules/catalog/catalog.service.test.ts b/server/src/modules/catalog/catalog.service.test.ts index 12746bc64..41a0d3044 100644 --- a/server/src/modules/catalog/catalog.service.test.ts +++ b/server/src/modules/catalog/catalog.service.test.ts @@ -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' }]);