From 60a303ac1f45e94a2c7ee8469cc7fc15b2535acb Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Sun, 1 Nov 2026 20:13:34 -0700 Subject: [PATCH] fix(metadata): omit unknown page counts from file writes Normalize stored zero values at the shared write payload boundary so every format omits an unknown page count. Align collection repository tests with the query shape left after podcast removal. Closes #2040 --- .../collection/collection.repository.test.ts | 22 +++++-------------- .../file-write/file-write.repository.test.ts | 12 ++++++++++ .../file-write/file-write.repository.ts | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/server/src/modules/collection/collection.repository.test.ts b/server/src/modules/collection/collection.repository.test.ts index f48500dde..996c31fc5 100644 --- a/server/src/modules/collection/collection.repository.test.ts +++ b/server/src/modules/collection/collection.repository.test.ts @@ -104,8 +104,7 @@ describe('CollectionRepository', () => { const orderBy = vi.fn().mockResolvedValue([{ id: 1, memberCount: 2 }]); const groupBy = vi.fn().mockReturnValue({ orderBy }); const where = vi.fn().mockReturnValue({ groupBy }); - const thirdLeftJoin = vi.fn().mockReturnValue({ where }); - const secondLeftJoin = vi.fn().mockReturnValue({ leftJoin: thirdLeftJoin }); + 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); @@ -116,15 +115,13 @@ describe('CollectionRepository', () => { 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' })); - expect(thirdLeftJoin).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ op: 'eq' })); }); 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 thirdLeftJoin = vi.fn().mockReturnValue({ where }); - const secondLeftJoin = vi.fn().mockReturnValue({ leftJoin: thirdLeftJoin }); + 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); @@ -134,8 +131,8 @@ describe('CollectionRepository', () => { expect(rows).toEqual([{ id: 1, bookCount: 1 }]); expect(where).toHaveBeenCalledWith( expect.objectContaining({ - op: 'and', - clauses: expect.arrayContaining([expect.objectContaining({ op: 'or' }), expect.objectContaining({ op: 'eq', right: 'books' })]), + op: 'or', + clauses: expect.arrayContaining([expect.objectContaining({ op: 'eq', right: 5 }), expect.objectContaining({ op: 'eq', right: true })]), }), ); expect(secondLeftJoin).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ op: 'and' })); @@ -145,8 +142,7 @@ describe('CollectionRepository', () => { const limit = vi.fn().mockResolvedValue([{ id: 10, bookCount: 1 }]); const groupBy = vi.fn().mockReturnValue({ limit }); const where = vi.fn().mockReturnValue({ groupBy }); - const thirdLeftJoin = vi.fn().mockReturnValue({ where }); - const secondLeftJoin = vi.fn().mockReturnValue({ leftJoin: thirdLeftJoin }); + 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); @@ -156,13 +152,7 @@ describe('CollectionRepository', () => { expect(where).toHaveBeenCalledWith( expect.objectContaining({ op: 'and', - clauses: expect.arrayContaining([ - expect.objectContaining({ - op: 'and', - clauses: expect.arrayContaining([expect.objectContaining({ op: 'or' })]), - }), - expect.objectContaining({ op: 'eq', right: 'books' }), - ]), + clauses: expect.arrayContaining([expect.objectContaining({ op: 'eq', right: 10 }), expect.objectContaining({ op: 'or' })]), }), ); }); diff --git a/server/src/modules/file-write/file-write.repository.test.ts b/server/src/modules/file-write/file-write.repository.test.ts index 35dc18b66..980cf31d7 100644 --- a/server/src/modules/file-write/file-write.repository.test.ts +++ b/server/src/modules/file-write/file-write.repository.test.ts @@ -136,6 +136,18 @@ describe('FileWriteRepository', () => { await expect(repo.loadPayload(11)).resolves.toBeNull(); }); + it('loadPayload normalizes an unknown zero page count to null', async () => { + const metadataChain = chain([{ title: 'Unknown length', pageCount: 0 }]); + const emptyRelatedRows = chain([]); + const db = { + select: vi.fn().mockReturnValue(emptyRelatedRows).mockReturnValueOnce(metadataChain), + }; + + const repo = new FileWriteRepository(db as never); + + await expect(repo.loadPayload(12)).resolves.toMatchObject({ pageCount: null }); + }); + it('loadPayload maps metadata, authors, genres, and tags', async () => { const meta = { title: 'Dune', diff --git a/server/src/modules/file-write/file-write.repository.ts b/server/src/modules/file-write/file-write.repository.ts index 58277e873..da61ca84c 100644 --- a/server/src/modules/file-write/file-write.repository.ts +++ b/server/src/modules/file-write/file-write.repository.ts @@ -276,7 +276,7 @@ export class FileWriteRepository { publishedDate: meta.publishedDate, publishedYear: meta.publishedYear, language: meta.language, - pageCount: meta.pageCount, + pageCount: typeof meta.pageCount === 'number' && meta.pageCount > 0 ? meta.pageCount : null, seriesName: meta.seriesName, seriesIndex: meta.seriesIndex, isbn10: meta.isbn10,