mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
feat(collections): add public collections with shared read access
Add an isPublic flag to collections allowing users to share collections across the instance. Public collections are readable by all authenticated users while write, edit, reorder, and delete operations remain restricted to the owner.
This commit is contained in:
@@ -0,0 +1,2 @@
|
||||
ALTER TABLE "collections" ADD COLUMN "is_public" boolean DEFAULT false NOT NULL;--> statement-breakpoint
|
||||
CREATE INDEX "collections_public_display_name_idx" ON "collections" USING btree ("is_public","display_order","name");
|
||||
File diff suppressed because it is too large
Load Diff
@@ -568,6 +568,13 @@
|
||||
"when": 1787850415027,
|
||||
"tag": "0080_add_book_metadata_cover_updated_at",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 81,
|
||||
"version": "7",
|
||||
"when": 1787931558680,
|
||||
"tag": "0081_add_public_collections",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ export const collections = pgTable(
|
||||
name: text('name').notNull(),
|
||||
icon: text('icon'),
|
||||
description: text('description'),
|
||||
isPublic: boolean('is_public').notNull().default(false),
|
||||
syncToKobo: boolean('sync_to_kobo').notNull().default(false),
|
||||
displayOrder: integer('display_order').notNull().default(0),
|
||||
createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(),
|
||||
@@ -24,6 +25,7 @@ export const collections = pgTable(
|
||||
(t) => [
|
||||
uniqueIndex('collections_user_name_uidx').on(t.userId, t.name),
|
||||
index('collections_user_display_name_idx').on(t.userId, t.displayOrder, t.name),
|
||||
index('collections_public_display_name_idx').on(t.isPublic, t.displayOrder, t.name),
|
||||
],
|
||||
);
|
||||
|
||||
@@ -47,3 +49,6 @@ export const collectionBooks = pgTable(
|
||||
index('collection_books_collection_position_idx').on(table.collectionId, table.position),
|
||||
],
|
||||
);
|
||||
|
||||
export type Collection = typeof collections.$inferSelect;
|
||||
export type NewCollection = typeof collections.$inferInsert;
|
||||
|
||||
@@ -15,11 +15,14 @@ async function errorsFor<T extends object>(dtoClass: new () => T, value: Record<
|
||||
describe('Collection DTO validation', () => {
|
||||
describe('CreateCollectionDto', () => {
|
||||
it('accepts valid payloads and rejects empty names or missing icons', async () => {
|
||||
expect((await errorsFor(CreateCollectionDto, { name: 'Favorites', icon: '⭐', description: 'Best books', syncToKobo: true })).length).toBe(0);
|
||||
expect(
|
||||
(await errorsFor(CreateCollectionDto, { name: 'Favorites', icon: '⭐', description: 'Best books', isPublic: true, syncToKobo: true })).length,
|
||||
).toBe(0);
|
||||
expect((await errorsFor(CreateCollectionDto, { name: '' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(CreateCollectionDto, {})).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(CreateCollectionDto, { name: 'Favorites' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(CreateCollectionDto, { name: 'Favorites', icon: ' ' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(CreateCollectionDto, { name: 'Favorites', icon: '⭐', isPublic: 'yes' })).length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('enforces field lengths', async () => {
|
||||
@@ -36,6 +39,8 @@ describe('Collection DTO validation', () => {
|
||||
expect((await errorsFor(UpdateCollectionDto, { name: '' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateCollectionDto, { icon: ' ' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateCollectionDto, { icon: null })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateCollectionDto, { isPublic: false })).length).toBe(0);
|
||||
expect((await errorsFor(UpdateCollectionDto, { isPublic: 1 })).length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -3,6 +3,8 @@ vi.mock('drizzle-orm', () => ({
|
||||
count: vi.fn(() => ({ op: 'count' })),
|
||||
eq: vi.fn((left: unknown, right: unknown) => ({ op: 'eq', left, right })),
|
||||
inArray: vi.fn((left: unknown, right: unknown[]) => ({ op: 'inArray', left, right })),
|
||||
ne: vi.fn((left: unknown, right: unknown) => ({ op: 'ne', left, right })),
|
||||
or: vi.fn((...clauses: unknown[]) => ({ op: 'or', clauses })),
|
||||
sql: Object.assign(
|
||||
vi.fn((strings: TemplateStringsArray, ...values: unknown[]) => ({ op: 'sql', text: strings.join(''), values })),
|
||||
{
|
||||
@@ -15,6 +17,7 @@ import { CollectionRepository } from './collection.repository';
|
||||
|
||||
describe('CollectionRepository', () => {
|
||||
const txWhere = vi.fn();
|
||||
const txReturning = vi.fn();
|
||||
const txSet = vi.fn();
|
||||
const txUpdate = vi.fn(() => ({ set: txSet }));
|
||||
const tx = {
|
||||
@@ -35,21 +38,36 @@ describe('CollectionRepository', () => {
|
||||
db.transaction.mockImplementation(async (callback: (transaction: typeof tx) => Promise<void>) => callback(tx));
|
||||
txUpdate.mockImplementation(() => ({ set: txSet }));
|
||||
txSet.mockReturnValue({ where: txWhere });
|
||||
txWhere.mockResolvedValue(undefined);
|
||||
txWhere.mockReturnValue({ returning: txReturning });
|
||||
txReturning.mockResolvedValue([{ id: 1 }]);
|
||||
});
|
||||
|
||||
it('updateDisplayOrders performs all updates in a single transaction and updates timestamps', async () => {
|
||||
await repo.updateDisplayOrders(12, [
|
||||
const updatedCount = await repo.updateDisplayOrders(12, [
|
||||
{ id: 1, displayOrder: 3 },
|
||||
{ id: 2, displayOrder: 4 },
|
||||
]);
|
||||
|
||||
expect(updatedCount).toBe(2);
|
||||
expect(db.transaction).toHaveBeenCalledTimes(1);
|
||||
expect(txUpdate).toHaveBeenCalledTimes(2);
|
||||
expect(txSet).toHaveBeenNthCalledWith(1, expect.objectContaining({ displayOrder: 3, updatedAt: expect.anything() }));
|
||||
expect(txSet).toHaveBeenNthCalledWith(2, expect.objectContaining({ displayOrder: 4, updatedAt: expect.anything() }));
|
||||
});
|
||||
|
||||
it('aborts the transaction when any collection is not owned by the caller', async () => {
|
||||
txReturning.mockResolvedValueOnce([{ id: 1 }]).mockResolvedValueOnce([]);
|
||||
|
||||
const updatedCount = await repo.updateDisplayOrders(12, [
|
||||
{ id: 1, displayOrder: 3 },
|
||||
{ id: 2, displayOrder: 4 },
|
||||
]);
|
||||
|
||||
expect(updatedCount).toBe(0);
|
||||
expect(db.transaction).toHaveBeenCalledTimes(1);
|
||||
expect(txUpdate).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('findBookIdsPage short-circuits when no library ids are accessible', async () => {
|
||||
const result = await repo.findBookIdsPage(20, [], 1, 25);
|
||||
|
||||
@@ -82,18 +100,73 @@ describe('CollectionRepository', () => {
|
||||
expect(result).toEqual({ bookIds: [7, 9], total: 2, page: 1, size: 2 });
|
||||
});
|
||||
|
||||
it('findAllForUserWithMembership builds membership projection for provided book ids', async () => {
|
||||
it('findAllOwnedForUserWithMembership builds a viewer-filtered membership projection for provided book ids', async () => {
|
||||
const orderBy = vi.fn().mockResolvedValue([{ id: 1, memberCount: 2 }]);
|
||||
const groupBy = vi.fn().mockReturnValue({ orderBy });
|
||||
const where = vi.fn().mockReturnValue({ groupBy });
|
||||
const leftJoin = vi.fn().mockReturnValue({ where });
|
||||
const secondLeftJoin = vi.fn().mockReturnValue({ where });
|
||||
const leftJoin = vi.fn().mockReturnValue({ leftJoin: secondLeftJoin });
|
||||
const from = vi.fn().mockReturnValue({ leftJoin });
|
||||
db.select.mockReturnValueOnce({ from } as never);
|
||||
|
||||
const rows = await repo.findAllForUserWithMembership(5, [100, 101]);
|
||||
const visibleBooksWhere = { type: 'visible-books' } as never;
|
||||
const rows = await repo.findAllOwnedForUserWithMembership(5, [100, 101], visibleBooksWhere);
|
||||
|
||||
expect(rows).toEqual([{ id: 1, memberCount: 2 }]);
|
||||
expect(db.select).toHaveBeenCalledWith(expect.objectContaining({ memberCount: expect.anything() }));
|
||||
expect(secondLeftJoin).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ op: 'and' }));
|
||||
});
|
||||
|
||||
it('findAllVisibleForUser keeps public metadata visible while counting only viewer-visible books', async () => {
|
||||
const orderBy = vi.fn().mockResolvedValue([{ id: 1, bookCount: 1 }]);
|
||||
const groupBy = vi.fn().mockReturnValue({ orderBy });
|
||||
const where = vi.fn().mockReturnValue({ groupBy });
|
||||
const secondLeftJoin = vi.fn().mockReturnValue({ where });
|
||||
const firstLeftJoin = vi.fn().mockReturnValue({ leftJoin: secondLeftJoin });
|
||||
const from = vi.fn().mockReturnValue({ leftJoin: firstLeftJoin });
|
||||
db.select.mockReturnValueOnce({ from } as never);
|
||||
|
||||
const rows = await repo.findAllVisibleForUser(5, { type: 'visible-books' } as never);
|
||||
|
||||
expect(rows).toEqual([{ id: 1, bookCount: 1 }]);
|
||||
expect(where).toHaveBeenCalledWith(expect.objectContaining({ op: 'or' }));
|
||||
expect(secondLeftJoin).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ op: 'and' }));
|
||||
});
|
||||
|
||||
it('findByIdForViewer embeds owner-or-public authorization in the query', async () => {
|
||||
const limit = vi.fn().mockResolvedValue([{ id: 10, bookCount: 1 }]);
|
||||
const groupBy = vi.fn().mockReturnValue({ limit });
|
||||
const where = vi.fn().mockReturnValue({ groupBy });
|
||||
const secondLeftJoin = vi.fn().mockReturnValue({ where });
|
||||
const firstLeftJoin = vi.fn().mockReturnValue({ leftJoin: secondLeftJoin });
|
||||
const from = vi.fn().mockReturnValue({ leftJoin: firstLeftJoin });
|
||||
db.select.mockReturnValueOnce({ from } as never);
|
||||
|
||||
await repo.findByIdForViewer(10, 5, false, { type: 'visible-books' } as never);
|
||||
|
||||
expect(where).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
op: 'and',
|
||||
clauses: expect.arrayContaining([expect.objectContaining({ op: 'or' })]),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('buildReadableMembershipWhere carries the same owner-or-public authorization predicate', () => {
|
||||
const limit = vi.fn().mockReturnValue({});
|
||||
const where = vi.fn().mockReturnValue({ limit });
|
||||
const innerJoin = vi.fn().mockReturnValue({ where });
|
||||
const from = vi.fn().mockReturnValue({ innerJoin });
|
||||
db.select.mockReturnValueOnce({ from } as never);
|
||||
|
||||
repo.buildReadableMembershipWhere(10, 5, false);
|
||||
|
||||
expect(where).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
op: 'and',
|
||||
clauses: expect.arrayContaining([expect.objectContaining({ op: 'or' })]),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('addBooks and removeBooks issue membership writes with expected payloads', async () => {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { Inject, Injectable } from '@nestjs/common';
|
||||
import { SQL, and, count, eq, inArray, sql } from 'drizzle-orm';
|
||||
import { SQL, and, count, eq, inArray, ne, or, sql } from 'drizzle-orm';
|
||||
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
|
||||
|
||||
import { DB } from '../../db';
|
||||
@@ -8,56 +8,70 @@ import { bookMetadata, books, collectionBooks, collections } from '../../db/sche
|
||||
|
||||
type Db = NodePgDatabase<typeof schema>;
|
||||
|
||||
class CollectionReorderMismatchError extends Error {}
|
||||
|
||||
const collectionFields = {
|
||||
id: collections.id,
|
||||
userId: collections.userId,
|
||||
name: collections.name,
|
||||
icon: collections.icon,
|
||||
description: collections.description,
|
||||
isPublic: collections.isPublic,
|
||||
syncToKobo: collections.syncToKobo,
|
||||
displayOrder: collections.displayOrder,
|
||||
createdAt: collections.createdAt,
|
||||
updatedAt: collections.updatedAt,
|
||||
bookCount: count(collectionBooks.bookId),
|
||||
};
|
||||
|
||||
const collectionFieldsWithVisibleBookCount = {
|
||||
...collectionFields,
|
||||
bookCount: count(books.id),
|
||||
};
|
||||
|
||||
@Injectable()
|
||||
export class CollectionRepository {
|
||||
constructor(@Inject(DB) private readonly db: Db) {}
|
||||
|
||||
findAllForUser(userId: number) {
|
||||
findAllVisibleForUser(userId: number, visibleBooksWhere: SQL | undefined) {
|
||||
return this.db
|
||||
.select(collectionFields)
|
||||
.select(collectionFieldsWithVisibleBookCount)
|
||||
.from(collections)
|
||||
.leftJoin(collectionBooks, eq(collections.id, collectionBooks.collectionId))
|
||||
.where(eq(collections.userId, userId))
|
||||
.leftJoin(books, and(eq(books.id, collectionBooks.bookId), ne(books.status, 'processing'), visibleBooksWhere))
|
||||
.where(or(eq(collections.userId, userId), eq(collections.isPublic, true)))
|
||||
.groupBy(collections.id, collections.userId)
|
||||
.orderBy(collections.displayOrder, collections.name);
|
||||
}
|
||||
|
||||
findAllForUserWithMembership(userId: number, bookIds: number[]) {
|
||||
findAllOwnedForUserWithMembership(userId: number, bookIds: number[], visibleBooksWhere: SQL | undefined) {
|
||||
const bookIdList = sql.join(
|
||||
bookIds.map((id) => sql`${id}`),
|
||||
sql`, `,
|
||||
);
|
||||
return this.db
|
||||
.select({
|
||||
...collectionFields,
|
||||
memberCount: sql<number>`count(case when ${collectionBooks.bookId} in (${bookIdList}) then 1 end)::int`,
|
||||
...collectionFieldsWithVisibleBookCount,
|
||||
memberCount: sql<number>`count(case when ${books.id} in (${bookIdList}) then 1 end)::int`,
|
||||
})
|
||||
.from(collections)
|
||||
.leftJoin(collectionBooks, eq(collections.id, collectionBooks.collectionId))
|
||||
.leftJoin(books, and(eq(books.id, collectionBooks.bookId), ne(books.status, 'processing'), visibleBooksWhere))
|
||||
.where(eq(collections.userId, userId))
|
||||
.groupBy(collections.id, collections.userId)
|
||||
.orderBy(collections.displayOrder, collections.name);
|
||||
}
|
||||
|
||||
findById(id: number) {
|
||||
return this.db.select(collectionFields).from(collections).where(eq(collections.id, id)).limit(1);
|
||||
}
|
||||
|
||||
findByIdForViewer(id: number, userId: number, isSuperuser: boolean, visibleBooksWhere: SQL | undefined) {
|
||||
return this.db
|
||||
.select(collectionFields)
|
||||
.select(collectionFieldsWithVisibleBookCount)
|
||||
.from(collections)
|
||||
.leftJoin(collectionBooks, eq(collections.id, collectionBooks.collectionId))
|
||||
.where(eq(collections.id, id))
|
||||
.leftJoin(books, and(eq(books.id, collectionBooks.bookId), ne(books.status, 'processing'), visibleBooksWhere))
|
||||
.where(isSuperuser ? eq(collections.id, id) : and(eq(collections.id, id), or(eq(collections.userId, userId), eq(collections.isPublic, true))))
|
||||
.groupBy(collections.id, collections.userId)
|
||||
.limit(1);
|
||||
}
|
||||
@@ -93,11 +107,18 @@ export class CollectionRepository {
|
||||
.returning();
|
||||
}
|
||||
|
||||
buildMembershipWhere(collectionId: number): SQL {
|
||||
buildReadableMembershipWhere(collectionId: number, userId: number, isSuperuser: boolean): SQL {
|
||||
const membership = this.db
|
||||
.select({ one: sql`1` })
|
||||
.from(collectionBooks)
|
||||
.where(and(eq(collectionBooks.collectionId, collectionId), eq(collectionBooks.bookId, books.id)))
|
||||
.innerJoin(collections, eq(collections.id, collectionBooks.collectionId))
|
||||
.where(
|
||||
and(
|
||||
eq(collectionBooks.collectionId, collectionId),
|
||||
eq(collectionBooks.bookId, books.id),
|
||||
...(isSuperuser ? [] : [or(eq(collections.userId, userId), eq(collections.isPublic, true))!]),
|
||||
),
|
||||
)
|
||||
.limit(1);
|
||||
return sql`exists (${membership})`;
|
||||
}
|
||||
@@ -151,14 +172,24 @@ export class CollectionRepository {
|
||||
return rows.map((row) => row.bookId);
|
||||
}
|
||||
|
||||
async updateDisplayOrders(userId: number, order: { id: number; displayOrder: number }[]) {
|
||||
await this.db.transaction(async (tx) => {
|
||||
for (const item of order) {
|
||||
await tx
|
||||
.update(collections)
|
||||
.set({ displayOrder: item.displayOrder, updatedAt: sql`now()` })
|
||||
.where(and(eq(collections.id, item.id), eq(collections.userId, userId)));
|
||||
}
|
||||
});
|
||||
async updateDisplayOrders(userId: number, order: { id: number; displayOrder: number }[]): Promise<number> {
|
||||
try {
|
||||
return await this.db.transaction(async (tx) => {
|
||||
let updatedCount = 0;
|
||||
for (const item of order) {
|
||||
const updatedRows = await tx
|
||||
.update(collections)
|
||||
.set({ displayOrder: item.displayOrder, updatedAt: sql`now()` })
|
||||
.where(and(eq(collections.id, item.id), eq(collections.userId, userId)))
|
||||
.returning({ id: collections.id });
|
||||
if (updatedRows.length !== 1) throw new CollectionReorderMismatchError();
|
||||
updatedCount += 1;
|
||||
}
|
||||
return updatedCount;
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof CollectionReorderMismatchError) return 0;
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,6 +31,7 @@ function makeCollection(overrides?: Record<string, unknown>) {
|
||||
name: 'Favorites',
|
||||
icon: 'FolderOpen',
|
||||
description: null,
|
||||
isPublic: false,
|
||||
syncToKobo: false,
|
||||
displayOrder: 0,
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
@@ -42,9 +43,10 @@ function makeCollection(overrides?: Record<string, unknown>) {
|
||||
|
||||
function makeService() {
|
||||
const collectionRepo = {
|
||||
findAllForUser: vi.fn(),
|
||||
findAllForUserWithMembership: vi.fn(),
|
||||
findAllVisibleForUser: vi.fn(),
|
||||
findAllOwnedForUserWithMembership: vi.fn(),
|
||||
findById: vi.fn(),
|
||||
findByIdForViewer: vi.fn().mockResolvedValue([makeCollection()]),
|
||||
insert: vi.fn(),
|
||||
update: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
@@ -53,12 +55,12 @@ function makeService() {
|
||||
removeBooks: vi.fn(),
|
||||
findBookIdsPage: vi.fn(),
|
||||
findAllBookIds: vi.fn(),
|
||||
buildMembershipWhere: vi.fn(),
|
||||
buildReadableMembershipWhere: vi.fn(),
|
||||
};
|
||||
|
||||
const libraryService = {
|
||||
verifyUserAccess: vi.fn(),
|
||||
findAccessibleLibraryIds: vi.fn(),
|
||||
findAccessibleLibraryIds: vi.fn().mockResolvedValue([100]),
|
||||
};
|
||||
|
||||
const queryBuilder = {
|
||||
@@ -94,35 +96,52 @@ describe('CollectionService', () => {
|
||||
it('returns all collections for user when membership ids are absent', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
const user = makeUser();
|
||||
collectionRepo.findAllForUser.mockResolvedValue([makeCollection()]);
|
||||
collectionRepo.findAllVisibleForUser.mockResolvedValue([makeCollection()]);
|
||||
|
||||
const result = await service.findAll(user);
|
||||
|
||||
expect(collectionRepo.findAllForUser).toHaveBeenCalledWith(user.id);
|
||||
expect(collectionRepo.findAllForUserWithMembership).not.toHaveBeenCalled();
|
||||
expect(result).toEqual([makeCollection()]);
|
||||
expect(collectionRepo.findAllVisibleForUser).toHaveBeenCalledWith(user.id, expect.anything());
|
||||
expect(collectionRepo.findAllOwnedForUserWithMembership).not.toHaveBeenCalled();
|
||||
expect(result).toEqual([{ ...makeCollection(), isOwner: true }]);
|
||||
});
|
||||
|
||||
it('returns public collections as read-only viewer projections with content-filtered counts', async () => {
|
||||
const { service, collectionRepo, queryBuilder } = makeService();
|
||||
const user = makeUser({ id: 8 });
|
||||
const shared = makeCollection({ id: 20, userId: 99, isPublic: true, bookCount: 2 });
|
||||
collectionRepo.findAllVisibleForUser.mockResolvedValue([shared]);
|
||||
|
||||
const result = await service.findAll(user);
|
||||
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledWith(undefined, {
|
||||
accessibleLibraryIds: [100],
|
||||
userId: 8,
|
||||
timeZone: 'UTC',
|
||||
contentFilters: EMPTY_CONTENT_FILTER_RULES,
|
||||
});
|
||||
expect(result).toEqual([{ ...shared, isOwner: false }]);
|
||||
});
|
||||
|
||||
it('returns membership counts when book ids are provided', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
const user = makeUser();
|
||||
collectionRepo.findAllForUserWithMembership.mockResolvedValue([makeCollection({ memberCount: 2 })]);
|
||||
collectionRepo.findAllOwnedForUserWithMembership.mockResolvedValue([makeCollection({ memberCount: 2 })]);
|
||||
|
||||
const result = await service.findAll(user, [4, 5]);
|
||||
|
||||
expect(collectionRepo.findAllForUserWithMembership).toHaveBeenCalledWith(user.id, [4, 5]);
|
||||
expect(collectionRepo.findAllOwnedForUserWithMembership).toHaveBeenCalledWith(user.id, [4, 5], expect.anything());
|
||||
expect(result[0]).toEqual(expect.objectContaining({ memberCount: 2 }));
|
||||
});
|
||||
|
||||
it('falls back to the non-membership query when book ids are empty', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
const user = makeUser();
|
||||
collectionRepo.findAllForUser.mockResolvedValue([makeCollection()]);
|
||||
collectionRepo.findAllVisibleForUser.mockResolvedValue([makeCollection()]);
|
||||
|
||||
await service.findAll(user, []);
|
||||
|
||||
expect(collectionRepo.findAllForUser).toHaveBeenCalledWith(user.id);
|
||||
expect(collectionRepo.findAllForUserWithMembership).not.toHaveBeenCalled();
|
||||
expect(collectionRepo.findAllVisibleForUser).toHaveBeenCalledWith(user.id, expect.anything());
|
||||
expect(collectionRepo.findAllOwnedForUserWithMembership).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('resolves selection payloads before loading membership counts', async () => {
|
||||
@@ -130,24 +149,24 @@ describe('CollectionService', () => {
|
||||
const user = makeUser();
|
||||
const selection = { query: { libraryId: 5, q: 'dune' } };
|
||||
bookService.resolveSelectionToIds.mockResolvedValue([10, 11, 10]);
|
||||
collectionRepo.findAllForUserWithMembership.mockResolvedValue([makeCollection({ memberCount: 2 })]);
|
||||
collectionRepo.findAllOwnedForUserWithMembership.mockResolvedValue([makeCollection({ memberCount: 2 })]);
|
||||
|
||||
const result = await service.findAllWithSelectionMembership(selection, user);
|
||||
|
||||
expect(bookService.resolveSelectionToIds).toHaveBeenCalledWith(selection, user);
|
||||
expect(collectionRepo.findAllForUserWithMembership).toHaveBeenCalledWith(user.id, [10, 11]);
|
||||
expect(collectionRepo.findAllOwnedForUserWithMembership).toHaveBeenCalledWith(user.id, [10, 11], expect.anything());
|
||||
expect(result[0]).toEqual(expect.objectContaining({ memberCount: 2 }));
|
||||
});
|
||||
|
||||
it('uses the plain collection list when a selection resolves to zero books', async () => {
|
||||
const { service, collectionRepo, bookService } = makeService();
|
||||
bookService.resolveSelectionToIds.mockResolvedValue([]);
|
||||
collectionRepo.findAllForUser.mockResolvedValue([makeCollection()]);
|
||||
collectionRepo.findAllVisibleForUser.mockResolvedValue([makeCollection()]);
|
||||
|
||||
await service.findAllWithSelectionMembership({ query: { libraryId: 5 } }, makeUser());
|
||||
|
||||
expect(collectionRepo.findAllForUser).toHaveBeenCalledWith(1);
|
||||
expect(collectionRepo.findAllForUserWithMembership).not.toHaveBeenCalled();
|
||||
expect(collectionRepo.findAllVisibleForUser).toHaveBeenCalledWith(1, expect.anything());
|
||||
expect(collectionRepo.findAllOwnedForUserWithMembership).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -170,19 +189,41 @@ describe('CollectionService', () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
const collection = makeCollection({ userId: 99 });
|
||||
collectionRepo.findById.mockResolvedValue([collection]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([collection]);
|
||||
|
||||
const result = await service.findOne(10, makeUser({ isSuperuser: true }));
|
||||
|
||||
expect(result).toEqual(collection);
|
||||
expect(result).toEqual({ ...collection, isOwner: false });
|
||||
});
|
||||
|
||||
it('allows a non-owner to read a public collection and preserves viewer-filtered counts', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
const shared = makeCollection({ userId: 99, isPublic: true, bookCount: 40 });
|
||||
const viewerProjection = makeCollection({ userId: 99, isPublic: true, bookCount: 3 });
|
||||
collectionRepo.findById.mockResolvedValue([shared]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([viewerProjection]);
|
||||
|
||||
const result = await service.findOne(10, makeUser({ id: 8 }));
|
||||
|
||||
expect(result).toEqual({ ...viewerProjection, isOwner: false });
|
||||
});
|
||||
});
|
||||
|
||||
describe('update', () => {
|
||||
it('rejects updates from a non-owner even when the collection is public', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
collectionRepo.findById.mockResolvedValue([makeCollection({ userId: 99, isPublic: true })]);
|
||||
|
||||
await expect(service.update(10, { name: 'Hijacked' }, makeUser({ id: 8 }))).rejects.toThrow(ForbiddenException);
|
||||
expect(collectionRepo.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns the hydrated collection so derived counts remain available', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
const existing = makeCollection({ bookCount: 3 });
|
||||
const hydrated = makeCollection({ name: 'Updated Favorites', icon: 'FolderHeart', syncToKobo: true, bookCount: 3 });
|
||||
collectionRepo.findById.mockResolvedValueOnce([existing]).mockResolvedValueOnce([hydrated]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([hydrated]);
|
||||
collectionRepo.update.mockResolvedValue([
|
||||
{
|
||||
id: existing.id,
|
||||
@@ -205,7 +246,7 @@ describe('CollectionService', () => {
|
||||
syncToKobo: true,
|
||||
});
|
||||
expect(collectionRepo.findById).toHaveBeenCalledTimes(2);
|
||||
expect(result).toEqual(hydrated);
|
||||
expect(result).toEqual({ ...hydrated, isOwner: true });
|
||||
});
|
||||
|
||||
it('maps unique constraint errors to ConflictException semantics', async () => {
|
||||
@@ -245,7 +286,7 @@ describe('CollectionService', () => {
|
||||
it('creates collection for current user and returns hydrated row', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
collectionRepo.insert.mockResolvedValue([{ id: 25 }]);
|
||||
collectionRepo.findById.mockResolvedValue([makeCollection({ id: 25, name: 'New Collection' })]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([makeCollection({ id: 25, userId: 9, name: 'New Collection' })]);
|
||||
|
||||
const result = await service.create({ name: 'New Collection', icon: '⭐' } as any, makeUser({ id: 9 }));
|
||||
|
||||
@@ -254,12 +295,24 @@ describe('CollectionService', () => {
|
||||
userId: 9,
|
||||
name: 'New Collection',
|
||||
icon: '⭐',
|
||||
isPublic: false,
|
||||
syncToKobo: false,
|
||||
}),
|
||||
);
|
||||
expect(result).toEqual(expect.objectContaining({ id: 25, name: 'New Collection' }));
|
||||
});
|
||||
|
||||
it('persists explicit public visibility on create', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
const created = makeCollection({ id: 26, userId: 9, isPublic: true });
|
||||
collectionRepo.insert.mockResolvedValue([created]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([created]);
|
||||
|
||||
await service.create({ name: 'Shared', icon: 'Globe', isPublic: true } as never, makeUser({ id: 9 }));
|
||||
|
||||
expect(collectionRepo.insert).toHaveBeenCalledWith(expect.objectContaining({ userId: 9, isPublic: true }));
|
||||
});
|
||||
|
||||
it('rejects create when icon is empty after trimming', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
|
||||
@@ -304,6 +357,7 @@ describe('CollectionService', () => {
|
||||
it('verifies collection ownership and resolves the selection before adding books', async () => {
|
||||
const { service, collectionRepo, bookService } = makeService();
|
||||
collectionRepo.findById.mockResolvedValueOnce([makeCollection()]).mockResolvedValueOnce([makeCollection({ bookCount: 2 })]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([makeCollection({ bookCount: 2 })]);
|
||||
bookService.resolveSelectionToIds.mockResolvedValue([7, 8]);
|
||||
collectionRepo.addBooks.mockResolvedValue([]);
|
||||
const selection = { bookIds: [7, 8] };
|
||||
@@ -384,10 +438,11 @@ describe('CollectionService', () => {
|
||||
const { service, collectionRepo, libraryService, queryBuilder, bookService } = makeService();
|
||||
collectionRepo.findById.mockResolvedValue([makeCollection()]);
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([100]);
|
||||
collectionRepo.buildMembershipWhere.mockReturnValue('membership-where');
|
||||
collectionRepo.buildReadableMembershipWhere.mockReturnValue('membership-where');
|
||||
bookService.executeBooksQuery.mockResolvedValue({ items: [], total: 0, page: 0, size: 50 });
|
||||
const user = makeUser();
|
||||
|
||||
const result = await service.getBooks(10, makeUser(), 0, 50);
|
||||
const result = await service.getBooks(10, user, 0, 50);
|
||||
|
||||
expect(result).toEqual({ items: [], total: 0, page: 0, size: 50 });
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledWith(undefined, {
|
||||
@@ -397,7 +452,7 @@ describe('CollectionService', () => {
|
||||
timeZone: 'UTC',
|
||||
contentFilters: EMPTY_CONTENT_FILTER_RULES,
|
||||
});
|
||||
expect(collectionRepo.buildMembershipWhere).toHaveBeenCalledWith(10);
|
||||
expect(collectionRepo.buildReadableMembershipWhere).toHaveBeenCalledWith(10, user.id, user.isSuperuser);
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalledWith(
|
||||
1,
|
||||
expect.anything(),
|
||||
@@ -413,7 +468,7 @@ describe('CollectionService', () => {
|
||||
const { service, collectionRepo, libraryService, queryBuilder, bookService } = makeService();
|
||||
collectionRepo.findById.mockResolvedValue([makeCollection()]);
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([100, 101]);
|
||||
collectionRepo.buildMembershipWhere.mockReturnValue('membership-where');
|
||||
collectionRepo.buildReadableMembershipWhere.mockReturnValue('membership-where');
|
||||
queryBuilder.buildWhere.mockReturnValue('filter-where');
|
||||
bookService.executeBooksQuery.mockResolvedValue({ items: [{ id: 2 }, { id: 1 }], total: 2, page: 0, size: 50 });
|
||||
|
||||
@@ -457,18 +512,40 @@ describe('CollectionService', () => {
|
||||
expect(libraryService.findAccessibleLibraryIds).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('allows a non-owner to query a public collection through the viewer access predicate', async () => {
|
||||
const { service, collectionRepo, libraryService, queryBuilder, bookService } = makeService();
|
||||
collectionRepo.findById.mockResolvedValue([makeCollection({ userId: 99, isPublic: true })]);
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([100]);
|
||||
collectionRepo.buildReadableMembershipWhere.mockReturnValue('membership-where');
|
||||
queryBuilder.buildWhere.mockReturnValue('viewer-where');
|
||||
bookService.executeBooksQuery.mockResolvedValue({ items: [{ id: 7 }], total: 1, page: 0, size: 50 });
|
||||
|
||||
await service.getBooks(10, makeUser({ id: 8 }), 0, 50);
|
||||
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledWith(
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
accessibleLibraryIds: [100],
|
||||
userId: 8,
|
||||
contentFilters: EMPTY_CONTENT_FILTER_RULES,
|
||||
}),
|
||||
);
|
||||
expect(bookService.executeBooksQuery).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('queryJumpBuckets intersects the membership filter like queryBooks', async () => {
|
||||
const { service, collectionRepo, libraryService, queryBuilder, bookService } = makeService();
|
||||
collectionRepo.findById.mockResolvedValue([makeCollection()]);
|
||||
libraryService.findAccessibleLibraryIds.mockResolvedValue([100]);
|
||||
collectionRepo.buildMembershipWhere.mockReturnValue('membership-where');
|
||||
collectionRepo.buildReadableMembershipWhere.mockReturnValue('membership-where');
|
||||
queryBuilder.buildWhere.mockReturnValue('filter-where');
|
||||
bookService.executeJumpBucketsQuery.mockResolvedValue({ buckets: [], total: 0, kind: 'letter', granularity: null });
|
||||
|
||||
const query = { sort: [{ field: 'title', dir: 'asc' as const }], pagination: { page: 0, size: 50 } };
|
||||
await service.queryJumpBuckets(10, makeUser(), query as never);
|
||||
const user = makeUser();
|
||||
await service.queryJumpBuckets(10, user, query as never);
|
||||
|
||||
expect(collectionRepo.buildMembershipWhere).toHaveBeenCalledWith(10);
|
||||
expect(collectionRepo.buildReadableMembershipWhere).toHaveBeenCalledWith(10, user.id, user.isSuperuser);
|
||||
expect(bookService.executeJumpBucketsQuery).toHaveBeenCalledWith(1, expect.anything(), query, 'UTC');
|
||||
});
|
||||
|
||||
@@ -486,7 +563,7 @@ describe('CollectionService', () => {
|
||||
describe('reorder', () => {
|
||||
it('delegates reorder writes to repository with current user id', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
collectionRepo.updateDisplayOrders.mockResolvedValue(undefined);
|
||||
collectionRepo.updateDisplayOrders.mockResolvedValue(2);
|
||||
const user = makeUser({ id: 33 });
|
||||
|
||||
await service.reorder(
|
||||
@@ -511,12 +588,43 @@ describe('CollectionService', () => {
|
||||
|
||||
await expect(service.reorder({ order: [{ id: 1, displayOrder: 0 }] }, makeUser({ id: 1 }))).rejects.toThrow('db unavailable');
|
||||
});
|
||||
|
||||
it('rejects duplicate or non-owned collection IDs instead of silently reordering a subset', async () => {
|
||||
const { service, collectionRepo } = makeService();
|
||||
|
||||
await expect(
|
||||
service.reorder(
|
||||
{
|
||||
order: [
|
||||
{ id: 1, displayOrder: 0 },
|
||||
{ id: 1, displayOrder: 1 },
|
||||
],
|
||||
},
|
||||
makeUser(),
|
||||
),
|
||||
).rejects.toThrow(BadRequestException);
|
||||
expect(collectionRepo.updateDisplayOrders).not.toHaveBeenCalled();
|
||||
|
||||
collectionRepo.updateDisplayOrders.mockResolvedValue(1);
|
||||
await expect(
|
||||
service.reorder(
|
||||
{
|
||||
order: [
|
||||
{ id: 1, displayOrder: 0 },
|
||||
{ id: 99, displayOrder: 1 },
|
||||
],
|
||||
},
|
||||
makeUser(),
|
||||
),
|
||||
).rejects.toThrow(ForbiddenException);
|
||||
});
|
||||
});
|
||||
|
||||
describe('removeBooks', () => {
|
||||
it('updates collection membership and returns hydrated collection', async () => {
|
||||
const { service, collectionRepo, bookService } = makeService();
|
||||
collectionRepo.findById.mockResolvedValueOnce([makeCollection()]).mockResolvedValueOnce([makeCollection({ bookCount: 1 })]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([makeCollection({ bookCount: 1 })]);
|
||||
bookService.resolveSelectionToIds.mockResolvedValue([7]);
|
||||
collectionRepo.removeBooks.mockResolvedValue([{ collectionId: 10, bookId: 7 }]);
|
||||
|
||||
@@ -575,6 +683,7 @@ describe('CollectionService', () => {
|
||||
collectionRepo.findById
|
||||
.mockResolvedValueOnce([makeCollection({ userId: 88 })])
|
||||
.mockResolvedValueOnce([makeCollection({ userId: 88, bookCount: 0 })]);
|
||||
collectionRepo.findByIdForViewer.mockResolvedValue([makeCollection({ userId: 88, bookCount: 0 })]);
|
||||
bookService.resolveSelectionToIds.mockResolvedValue([7]);
|
||||
collectionRepo.removeBooks.mockResolvedValue([{ collectionId: 10, bookId: 7 }]);
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
|
||||
import { resolveTimeZone } from '../../common/utils/timezone.utils';
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { normalizeIconValue } from '../../common/utils/icon-value.utils';
|
||||
import type { Collection as CollectionRow } from '../../db/schema/collections';
|
||||
import { BookService } from '../book/book.service';
|
||||
import { BookQueryBuilder } from '../book/book-query-builder.service';
|
||||
import { LibraryService } from '../library/library.service';
|
||||
@@ -42,19 +43,58 @@ export class CollectionService {
|
||||
private readonly achievementEvents: AchievementEventsService,
|
||||
) {}
|
||||
|
||||
private assertAccess(ownerId: number, user: RequestUser): void {
|
||||
if (ownerId !== user.id && !user.isSuperuser) {
|
||||
private assertReadAccess(collection: CollectionRow, user: RequestUser): void {
|
||||
if (!collection.isPublic && collection.userId !== user.id && !user.isSuperuser) {
|
||||
throw new ForbiddenException(COLLECTION_ACCESS_DENIED_MESSAGE);
|
||||
}
|
||||
}
|
||||
|
||||
private async findCollectionForUserOrThrow(id: number, user: RequestUser) {
|
||||
private assertWriteAccess(collection: CollectionRow, user: RequestUser): void {
|
||||
if (collection.userId !== user.id && !user.isSuperuser) {
|
||||
throw new ForbiddenException('Cannot modify this collection');
|
||||
}
|
||||
}
|
||||
|
||||
private async getCollectionOrThrow(id: number): Promise<CollectionRow> {
|
||||
const [collection] = await this.collectionRepo.findById(id);
|
||||
if (!collection) throw new NotFoundException(COLLECTION_NOT_FOUND_MESSAGE);
|
||||
this.assertAccess(collection.userId, user);
|
||||
return collection;
|
||||
}
|
||||
|
||||
private async getReadableCollectionOrThrow(id: number, user: RequestUser): Promise<CollectionRow> {
|
||||
const collection = await this.getCollectionOrThrow(id);
|
||||
this.assertReadAccess(collection, user);
|
||||
return collection;
|
||||
}
|
||||
|
||||
private async getWritableCollectionOrThrow(id: number, user: RequestUser): Promise<CollectionRow> {
|
||||
const collection = await this.getCollectionOrThrow(id);
|
||||
this.assertWriteAccess(collection, user);
|
||||
return collection;
|
||||
}
|
||||
|
||||
private toResponse<T extends CollectionRow & { bookCount: number }>(collection: T, user: RequestUser) {
|
||||
return { ...collection, isOwner: collection.userId === user.id };
|
||||
}
|
||||
|
||||
private async buildViewerBookWhere(user: RequestUser): Promise<SQL | undefined> {
|
||||
const libraryIds = await this.libraryService.findAccessibleLibraryIds(user);
|
||||
const timeZone = resolveTimeZone((user.settings as { timezone?: unknown } | undefined)?.timezone, 'UTC');
|
||||
return this.queryBuilder.buildWhere(undefined, {
|
||||
accessibleLibraryIds: libraryIds,
|
||||
userId: user.id,
|
||||
timeZone,
|
||||
contentFilters: user.isSuperuser ? undefined : user.contentFilters,
|
||||
});
|
||||
}
|
||||
|
||||
private async hydrateForViewer(collection: CollectionRow, user: RequestUser) {
|
||||
const visibleBooksWhere = await this.buildViewerBookWhere(user);
|
||||
const [hydrated] = await this.collectionRepo.findByIdForViewer(collection.id, user.id, user.isSuperuser, visibleBooksWhere);
|
||||
if (!hydrated) throw new NotFoundException(COLLECTION_NOT_FOUND_MESSAGE);
|
||||
return this.toResponse(hydrated, user);
|
||||
}
|
||||
|
||||
private buildErrorLogFields(error: unknown): { errorClass: string; errorMessage: string } {
|
||||
const errorClass = error instanceof Error ? error.name : 'Error';
|
||||
const errorMessage = sanitizeLogValue(error instanceof Error ? error.message : String(error));
|
||||
@@ -74,11 +114,14 @@ export class CollectionService {
|
||||
return [...new Set(ids)];
|
||||
}
|
||||
|
||||
findAll(user: RequestUser, bookIds?: number[]) {
|
||||
async findAll(user: RequestUser, bookIds?: number[]) {
|
||||
const visibleBooksWhere = await this.buildViewerBookWhere(user);
|
||||
if (bookIds && bookIds.length > 0) {
|
||||
return this.collectionRepo.findAllForUserWithMembership(user.id, bookIds);
|
||||
const collections = await this.collectionRepo.findAllOwnedForUserWithMembership(user.id, bookIds, visibleBooksWhere);
|
||||
return collections.map((collection) => this.toResponse(collection, user));
|
||||
}
|
||||
return this.collectionRepo.findAllForUser(user.id);
|
||||
const collections = await this.collectionRepo.findAllVisibleForUser(user.id, visibleBooksWhere);
|
||||
return collections.map((collection) => this.toResponse(collection, user));
|
||||
}
|
||||
|
||||
async findAllWithSelectionMembership(dto: CollectionBooksDto, user: RequestUser) {
|
||||
@@ -87,7 +130,8 @@ export class CollectionService {
|
||||
}
|
||||
|
||||
async findOne(id: number, user: RequestUser) {
|
||||
return this.findCollectionForUserOrThrow(id, user);
|
||||
const collection = await this.getReadableCollectionOrThrow(id, user);
|
||||
return this.hydrateForViewer(collection, user);
|
||||
}
|
||||
|
||||
async create(dto: CreateCollectionDto, user: RequestUser) {
|
||||
@@ -101,14 +145,14 @@ export class CollectionService {
|
||||
name: dto.name,
|
||||
icon,
|
||||
description: dto.description ?? null,
|
||||
isPublic: dto.isPublic ?? false,
|
||||
syncToKobo: dto.syncToKobo ?? false,
|
||||
});
|
||||
const [collection] = await this.collectionRepo.findById(inserted.id);
|
||||
this.achievementEvents.emit(ACHIEVEMENT_EVENT_COLLECTION_CREATED, {
|
||||
userId: user.id,
|
||||
collectionId: inserted.id,
|
||||
});
|
||||
return collection;
|
||||
return this.hydrateForViewer(inserted, user);
|
||||
} catch (error) {
|
||||
if (isUniqueViolation(error)) {
|
||||
throw new ConflictException('A collection with this name already exists');
|
||||
@@ -118,7 +162,7 @@ export class CollectionService {
|
||||
}
|
||||
|
||||
async update(id: number, dto: UpdateCollectionDto, user: RequestUser) {
|
||||
const existing = await this.findCollectionForUserOrThrow(id, user);
|
||||
const existing = await this.getWritableCollectionOrThrow(id, user);
|
||||
const icon = dto.icon !== undefined ? normalizeIconValue(dto.icon) : normalizeIconValue(existing.icon);
|
||||
if (!icon) {
|
||||
throw new BadRequestException('Icon is required');
|
||||
@@ -129,6 +173,7 @@ export class CollectionService {
|
||||
...(dto.name !== undefined && { name: dto.name }),
|
||||
...(dto.icon !== undefined && { icon }),
|
||||
...(dto.description !== undefined && { description: dto.description }),
|
||||
...(dto.isPublic !== undefined && { isPublic: dto.isPublic }),
|
||||
...(dto.syncToKobo !== undefined && { syncToKobo: dto.syncToKobo }),
|
||||
});
|
||||
} catch (error) {
|
||||
@@ -139,11 +184,12 @@ export class CollectionService {
|
||||
}
|
||||
|
||||
const [updated] = await this.collectionRepo.findById(id);
|
||||
return updated;
|
||||
if (!updated) throw new NotFoundException(COLLECTION_NOT_FOUND_MESSAGE);
|
||||
return this.hydrateForViewer(updated, user);
|
||||
}
|
||||
|
||||
async remove(id: number, user: RequestUser) {
|
||||
const existing = await this.findCollectionForUserOrThrow(id, user);
|
||||
const existing = await this.getWritableCollectionOrThrow(id, user);
|
||||
await this.collectionRepo.delete(id, existing.userId);
|
||||
}
|
||||
|
||||
@@ -152,7 +198,14 @@ export class CollectionService {
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] userId=${user.id} itemCount=${dto.order.length} - reorder collections started`);
|
||||
try {
|
||||
await this.collectionRepo.updateDisplayOrders(user.id, dto.order);
|
||||
const distinctIds = new Set(dto.order.map((item) => item.id));
|
||||
if (distinctIds.size !== dto.order.length) {
|
||||
throw new BadRequestException('Duplicate collection IDs are not allowed in reorder payload');
|
||||
}
|
||||
const updatedCount = await this.collectionRepo.updateDisplayOrders(user.id, dto.order);
|
||||
if (updatedCount !== dto.order.length) {
|
||||
throw new ForbiddenException('Cannot reorder one or more collections');
|
||||
}
|
||||
this.logger.log(
|
||||
`[${event}] [end] userId=${user.id} durationMs=${Date.now() - startedAt} itemCount=${dto.order.length} - reorder collections completed`,
|
||||
);
|
||||
@@ -172,14 +225,13 @@ export class CollectionService {
|
||||
`[${event}] [start] collectionId=${id} userId=${user.id} selectionMode=${this.getSelectionMode(dto)} requestedCount=${this.getRequestedCount(dto)} - add books started`,
|
||||
);
|
||||
try {
|
||||
await this.findCollectionForUserOrThrow(id, user);
|
||||
const collection = await this.getWritableCollectionOrThrow(id, user);
|
||||
const bookIds = await this.resolveSelectionBookIds(dto, user);
|
||||
if (bookIds.length > 0) {
|
||||
await this.collectionRepo.addBooks(id, bookIds);
|
||||
}
|
||||
const [updated] = await this.collectionRepo.findById(id);
|
||||
this.logger.log(`[${event}] [end] collectionId=${id} durationMs=${Date.now() - startedAt} bookCount=${bookIds.length} - add books completed`);
|
||||
return updated;
|
||||
return this.hydrateForViewer(collection, user);
|
||||
} catch (error) {
|
||||
const { errorClass, errorMessage } = this.buildErrorLogFields(error);
|
||||
this.logger.warn(
|
||||
@@ -196,16 +248,15 @@ export class CollectionService {
|
||||
`[${event}] [start] collectionId=${id} userId=${user.id} selectionMode=${this.getSelectionMode(dto)} requestedCount=${this.getRequestedCount(dto)} - remove books started`,
|
||||
);
|
||||
try {
|
||||
await this.findCollectionForUserOrThrow(id, user);
|
||||
const collection = await this.getWritableCollectionOrThrow(id, user);
|
||||
const bookIds = await this.resolveSelectionBookIds(dto, user);
|
||||
if (bookIds.length > 0) {
|
||||
await this.collectionRepo.removeBooks(id, bookIds);
|
||||
}
|
||||
const [updated] = await this.collectionRepo.findById(id);
|
||||
this.logger.log(
|
||||
`[${event}] [end] collectionId=${id} durationMs=${Date.now() - startedAt} bookCount=${bookIds.length} - remove books completed`,
|
||||
);
|
||||
return updated;
|
||||
return this.hydrateForViewer(collection, user);
|
||||
} catch (error) {
|
||||
const { errorClass, errorMessage } = this.buildErrorLogFields(error);
|
||||
this.logger.warn(
|
||||
@@ -254,7 +305,7 @@ export class CollectionService {
|
||||
}
|
||||
|
||||
private async buildBooksWhere(id: number, user: RequestUser, query: BookQuery): Promise<SQL | undefined> {
|
||||
await this.findCollectionForUserOrThrow(id, user);
|
||||
await this.getReadableCollectionOrThrow(id, user);
|
||||
const libraryIds = await this.libraryService.findAccessibleLibraryIds(user);
|
||||
const timeZone = resolveTimeZone((user.settings as { timezone?: unknown } | undefined)?.timezone, 'UTC');
|
||||
const filterWhere = this.queryBuilder.buildWhere(query.filter, {
|
||||
@@ -264,6 +315,6 @@ export class CollectionService {
|
||||
timeZone,
|
||||
contentFilters: user.isSuperuser ? undefined : user.contentFilters,
|
||||
});
|
||||
return and(filterWhere, this.collectionRepo.buildMembershipWhere(id));
|
||||
return and(filterWhere, this.collectionRepo.buildReadableMembershipWhere(id, user.id, user.isSuperuser));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,6 +23,10 @@ export class CreateCollectionDto {
|
||||
@MaxLength(1000)
|
||||
description?: string;
|
||||
|
||||
@IsOptional()
|
||||
@IsBoolean()
|
||||
isPublic?: boolean;
|
||||
|
||||
@IsOptional()
|
||||
@IsBoolean()
|
||||
syncToKobo?: boolean;
|
||||
|
||||
@@ -25,6 +25,10 @@ export class UpdateCollectionDto {
|
||||
@MaxLength(1000)
|
||||
description?: string;
|
||||
|
||||
@ValidateIf((_, value) => value !== undefined)
|
||||
@IsBoolean()
|
||||
isPublic?: boolean;
|
||||
|
||||
@IsOptional()
|
||||
@IsBoolean()
|
||||
syncToKobo?: boolean;
|
||||
|
||||
@@ -127,7 +127,12 @@ describe('SmartScopeService', () => {
|
||||
const result = await service.findAll(user);
|
||||
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledTimes(1);
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledWith(secondSmartScope.filter, { accessibleLibraryIds: [2, 3], userId: 8, timeZone: 'UTC' });
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledWith(secondSmartScope.filter, {
|
||||
accessibleLibraryIds: [2, 3],
|
||||
userId: 8,
|
||||
timeZone: 'UTC',
|
||||
contentFilters: EMPTY_CONTENT_FILTER_RULES,
|
||||
});
|
||||
expect(result).toEqual([
|
||||
{ ...firstSmartScope, isOwner: false, koboSyncEnabled: false, bookCount: 0 },
|
||||
{ ...secondSmartScope, isOwner: false, koboSyncEnabled: false, bookCount: 7 },
|
||||
@@ -162,7 +167,12 @@ describe('SmartScopeService', () => {
|
||||
{ ...healthyScope, isOwner: false, koboSyncEnabled: false, bookCount: 7 },
|
||||
]);
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledTimes(1);
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledWith(healthyScope.filter, { accessibleLibraryIds: [2, 3], userId: 8, timeZone: 'UTC' });
|
||||
expect(queryBuilder.buildWhere).toHaveBeenCalledWith(healthyScope.filter, {
|
||||
accessibleLibraryIds: [2, 3],
|
||||
userId: 8,
|
||||
timeZone: 'UTC',
|
||||
contentFilters: EMPTY_CONTENT_FILTER_RULES,
|
||||
});
|
||||
expect(errorSpy).toHaveBeenCalledTimes(1);
|
||||
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('[smart_scope.count] [fail] scopeId=1 userId=8'));
|
||||
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('durationMs='));
|
||||
|
||||
@@ -93,7 +93,12 @@ export class SmartScopeService {
|
||||
try {
|
||||
const filter = validateGroupRule(smartScope.filter);
|
||||
if (!filter) return { ...this.toResponse(smartScope, user, koboSyncEnabledFor(smartScope)), bookCount: 0 };
|
||||
where = this.queryBuilder.buildWhere(filter, { accessibleLibraryIds, userId: user.id, timeZone });
|
||||
where = this.queryBuilder.buildWhere(filter, {
|
||||
accessibleLibraryIds,
|
||||
userId: user.id,
|
||||
timeZone,
|
||||
contentFilters: user.isSuperuser ? undefined : user.contentFilters,
|
||||
});
|
||||
} catch (err) {
|
||||
if (!(err instanceof BadRequestException)) throw err;
|
||||
const errorClass = err.constructor.name;
|
||||
|
||||
@@ -1113,6 +1113,109 @@ describe('Authorization matrix (e2e)', () => {
|
||||
},
|
||||
});
|
||||
expectError(addInaccessibleBook, 403, 'No access to this library');
|
||||
|
||||
const publicCollectionResponse = await ctx.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/collections',
|
||||
headers: authHeader(personas.ownerUser.accessToken),
|
||||
payload: {
|
||||
name: `authz-public-collection-${randomUUID()}`,
|
||||
icon: 'Globe',
|
||||
isPublic: true,
|
||||
},
|
||||
});
|
||||
expect(publicCollectionResponse.statusCode).toBe(201);
|
||||
const publicCollectionId = (publicCollectionResponse.json() as { id: number }).id;
|
||||
|
||||
const addVisibleOwnerBook = await ctx.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/v1/collections/${publicCollectionId}/books`,
|
||||
headers: authHeader(personas.ownerUser.accessToken),
|
||||
payload: { bookIds: [bookA.bookId] },
|
||||
});
|
||||
expect(addVisibleOwnerBook.statusCode).toBe(201);
|
||||
|
||||
const publicRead = await ctx.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/v1/collections/${publicCollectionId}`,
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
});
|
||||
expect(publicRead.statusCode).toBe(200);
|
||||
expect(publicRead.json()).toEqual(expect.objectContaining({ id: publicCollectionId, isPublic: true, isOwner: false, bookCount: 0 }));
|
||||
|
||||
const publicBooks = await ctx.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/v1/collections/${publicCollectionId}/books?page=0&size=50`,
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
});
|
||||
expect(publicBooks.statusCode).toBe(200);
|
||||
expect(publicBooks.json()).toEqual(expect.objectContaining({ items: [], total: 0 }));
|
||||
|
||||
const publicList = await ctx.app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/collections',
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
});
|
||||
expect(publicList.statusCode).toBe(200);
|
||||
expect(publicList.json()).toEqual(expect.arrayContaining([expect.objectContaining({ id: publicCollectionId, bookCount: 0, isOwner: false })]));
|
||||
expect(publicList.json()).not.toEqual(expect.arrayContaining([expect.objectContaining({ id: collectionId })]));
|
||||
|
||||
const writableTargets = await ctx.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/v1/collections?bookIds=${bookA.bookId}`,
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
});
|
||||
expect(writableTargets.statusCode).toBe(200);
|
||||
expect(writableTargets.json()).not.toEqual(expect.arrayContaining([expect.objectContaining({ id: publicCollectionId })]));
|
||||
|
||||
for (const mutation of [
|
||||
{ method: 'PATCH' as const, url: `/api/v1/collections/${publicCollectionId}`, payload: { name: 'Hijacked' } },
|
||||
{ method: 'POST' as const, url: `/api/v1/collections/${publicCollectionId}/books`, payload: { bookIds: [bookA.bookId] } },
|
||||
{ method: 'DELETE' as const, url: `/api/v1/collections/${publicCollectionId}`, payload: undefined },
|
||||
]) {
|
||||
const response = await ctx.app.inject({
|
||||
method: mutation.method,
|
||||
url: mutation.url,
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
...(mutation.payload ? { payload: mutation.payload } : {}),
|
||||
});
|
||||
expectError(response, 403, 'Cannot modify this collection');
|
||||
}
|
||||
|
||||
const otherUserCollectionResponse = await ctx.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/collections',
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
payload: {
|
||||
name: `authz-owned-collection-${randomUUID()}`,
|
||||
icon: 'Folder',
|
||||
},
|
||||
});
|
||||
expect(otherUserCollectionResponse.statusCode).toBe(201);
|
||||
const otherUserCollection = otherUserCollectionResponse.json() as { id: number; displayOrder: number };
|
||||
|
||||
const foreignReorder = await ctx.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/collections/reorder',
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
payload: {
|
||||
order: [
|
||||
{ id: otherUserCollection.id, displayOrder: 999 },
|
||||
{ id: publicCollectionId, displayOrder: 0 },
|
||||
],
|
||||
},
|
||||
});
|
||||
expectError(foreignReorder, 403, 'Cannot reorder one or more collections');
|
||||
|
||||
const listAfterRejectedReorder = await ctx.app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/collections',
|
||||
headers: authHeader(personas.otherUser.accessToken),
|
||||
});
|
||||
expect(listAfterRejectedReorder.statusCode).toBe(200);
|
||||
expect(listAfterRejectedReorder.json()).toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ id: otherUserCollection.id, displayOrder: otherUserCollection.displayOrder })]),
|
||||
);
|
||||
});
|
||||
|
||||
it('enforces smartScope private read and owner-only write rules', async () => {
|
||||
|
||||
Reference in New Issue
Block a user