From 73ac0bb72232938043cfd8e5e54809ce73fe7c5a Mon Sep 17 00:00:00 2001 From: SergeiAK <17836309+SergeiAK@users.noreply.github.com> Date: Thu, 10 Sep 2026 21:39:14 +0300 Subject: [PATCH] feat(authors): collapse series on author pages (#1271) Reduce large author listings to one card per series while preserving book counts, scoped visibility, and row-based pagination. Compute both totals in the collapsed query so deep pages remain consistent without a second author-wide scan. Closes #301 Co-authored-by: neon <18559658+neonsolstice@users.noreply.github.com> --- client/src/features/author/api/author.ts | 6 +- .../author/composables/useAuthorBooks.test.ts | 122 +++++++++- .../author/composables/useAuthorBooks.ts | 34 ++- .../author/views/AuthorDetailView.spec.ts | 61 +++++ .../author/views/AuthorDetailView.vue | 77 +++++- .../useSeriesCollapsePreference.test.ts | 33 ++- .../useSeriesCollapsePreference.ts | 21 +- packages/types/src/author.ts | 10 +- packages/types/src/series-collapse.ts | 12 +- scripts/e2e/suite-registry.mjs | 21 ++ .../src/modules/authors/authors.repository.ts | 26 ++- .../modules/authors/authors.service.test.ts | 113 ++++++++- server/src/modules/authors/authors.service.ts | 58 ++++- .../dto/authors-dto-validation.test.ts | 13 ++ .../authors/dto/list-author-books.dto.ts | 10 +- server/src/modules/book/book-read.service.ts | 4 + .../src/modules/book/book.repository.test.ts | 58 +++++ server/src/modules/book/book.repository.ts | 30 ++- .../book/series-collapse-prefs.test.ts | 31 +++ .../update-series-collapse-preferences.dto.ts | 5 + server/src/modules/user/user.service.test.ts | 47 +++- server/src/modules/user/user.service.ts | 1 + server/test/author-books-collapse.e2e-spec.ts | 219 ++++++++++++++++++ 23 files changed, 953 insertions(+), 59 deletions(-) create mode 100644 server/test/author-books-collapse.e2e-spec.ts diff --git a/client/src/features/author/api/author.ts b/client/src/features/author/api/author.ts index 1cb8444f4..32e9f1202 100644 --- a/client/src/features/author/api/author.ts +++ b/client/src/features/author/api/author.ts @@ -1,10 +1,10 @@ import { api } from '@/lib/api' import type { AuthorsPage, + AuthorBooksPage, AuthorDetail, AuthorMetadataCandidate, AuthorMetadataProviderKey, - BooksPage, JumpBucketsResponse, MergeAuthorsResult, } from '@bookorbit/types' @@ -34,6 +34,7 @@ type ListAuthorBooksParams = { sort: AuthorBookSort order: SortDirection libraryId?: number | null + collapseSeries?: boolean } export type UpdateAuthorPayload = { @@ -141,13 +142,14 @@ export async function fetchAuthor(id: number): Promise { return res.json() } -export async function fetchAuthorBooks(authorId: number, params: ListAuthorBooksParams): Promise { +export async function fetchAuthorBooks(authorId: number, params: ListAuthorBooksParams): Promise { const query = toQuery({ page: params.page, size: params.size, sort: params.sort, order: params.order, libraryId: params.libraryId ?? undefined, + collapseSeries: params.collapseSeries ? 'true' : undefined, }) const res = await api(`/api/v1/authors/${authorId}/books${query}`) diff --git a/client/src/features/author/composables/useAuthorBooks.test.ts b/client/src/features/author/composables/useAuthorBooks.test.ts index 364251c0e..b534200a3 100644 --- a/client/src/features/author/composables/useAuthorBooks.test.ts +++ b/client/src/features/author/composables/useAuthorBooks.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { ref } from 'vue' -import type { BookCard, BooksPage } from '@bookorbit/types' +import type { AuthorBooksPage, BookCard } from '@bookorbit/types' vi.mock('../api/author', () => ({ fetchAuthorBooks: vi.fn(), @@ -15,8 +15,8 @@ function makeBook(id: number): BookCard { return { id, title: `Book ${id}` } as BookCard } -function makePage(items: BookCard[], total = items.length): BooksPage { - return { items, total, page: 0, size: 50 } +function makePage(items: BookCard[], total = items.length, bookTotal = total): AuthorBooksPage { + return { items, total, bookTotal, page: 0, size: 50 } } describe('useAuthorBooks', () => { @@ -56,19 +56,19 @@ describe('useAuthorBooks', () => { expect(mockFetchAuthorBooks).not.toHaveBeenCalled() }) - it('skips fetch when already loading', async () => { + it('does not fire a second request while one is in flight', async () => { const authorId = ref(1) - let resolveFirst!: (v: BooksPage) => void + let resolveFirst!: (v: AuthorBooksPage) => void mockFetchAuthorBooks.mockImplementation( () => - new Promise((resolve) => { + new Promise((resolve) => { resolveFirst = resolve }), ) const { load } = useAuthorBooks(authorId) const firstLoad = load(true) - const secondLoad = load(true) + const secondLoad = load(false) resolveFirst(makePage([makeBook(1)])) await Promise.all([firstLoad, secondLoad]) @@ -76,6 +76,64 @@ describe('useAuthorBooks', () => { expect(mockFetchAuthorBooks).toHaveBeenCalledTimes(1) }) + it('reloads with the new flag when collapseSeries changes during an unresolved load', async () => { + const authorId = ref(1) + let resolveFirst!: (v: AuthorBooksPage) => void + mockFetchAuthorBooks + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve + }), + ) + .mockResolvedValueOnce(makePage([makeBook(50)], 1, 4)) + + const { load, items, total, bookTotal, collapseSeries } = useAuthorBooks(authorId) + const flatLoad = load(true) + + // The toggle flips while the flat request is still out, so the reset it asks for is the one + // that has to win. + collapseSeries.value = true + void load(true) + + resolveFirst(makePage([makeBook(1), makeBook(2), makeBook(3), makeBook(4)], 4, 4)) + await flatLoad + await vi.waitFor(() => expect(mockFetchAuthorBooks).toHaveBeenCalledTimes(2)) + + expect(mockFetchAuthorBooks).toHaveBeenNthCalledWith(1, 1, expect.objectContaining({ collapseSeries: false })) + expect(mockFetchAuthorBooks).toHaveBeenNthCalledWith(2, 1, expect.objectContaining({ collapseSeries: true, page: 0 })) + expect(items.value).toHaveLength(1) + expect(items.value[0]!.id).toBe(50) + expect(total.value).toBe(1) + expect(bookTotal.value).toBe(4) + }) + + it('keeps the error of a stale request off the page when a reset is queued behind it', async () => { + const authorId = ref(1) + let rejectFirst!: (e: unknown) => void + mockFetchAuthorBooks + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectFirst = reject + }), + ) + .mockResolvedValueOnce(makePage([makeBook(7)], 1)) + + const { load, error, items, collapseSeries } = useAuthorBooks(authorId) + const flatLoad = load(true) + + collapseSeries.value = true + void load(true) + + rejectFirst(new Error('stale request failed')) + await flatLoad + await vi.waitFor(() => expect(items.value).toHaveLength(1)) + + expect(error.value).toBeNull() + expect(items.value[0]!.id).toBe(7) + }) + it('skips non-reset load when no more items to load', async () => { const authorId = ref(1) mockFetchAuthorBooks.mockResolvedValue(makePage([makeBook(1), makeBook(2)], 2)) @@ -103,6 +161,46 @@ describe('useAuthorBooks', () => { expect(hasMore.value).toBe(true) }) + it('sends collapseSeries only once it is turned on', async () => { + const authorId = ref(1) + mockFetchAuthorBooks.mockResolvedValue(makePage([makeBook(1)])) + + const { load, collapseSeries } = useAuthorBooks(authorId) + await load(true) + expect(mockFetchAuthorBooks).toHaveBeenNthCalledWith(1, 1, expect.objectContaining({ collapseSeries: false })) + + collapseSeries.value = true + await load(true) + + expect(mockFetchAuthorBooks).toHaveBeenNthCalledWith(2, 1, expect.objectContaining({ collapseSeries: true })) + }) + + it('paginates on rows while keeping the book count separately', async () => { + const authorId = ref(1) + // 30 collapsed rows standing for 200 books: the row count is what says there is more to load, + // and the book count must not be the thing driving it. + mockFetchAuthorBooks.mockResolvedValueOnce(makePage([makeBook(1), makeBook(2)], 30, 200)) + + const { load, items, total, bookTotal, hasMore } = useAuthorBooks(authorId) + await load(true) + + expect(items.value).toHaveLength(2) + expect(total.value).toBe(30) + expect(bookTotal.value).toBe(200) + expect(hasMore.value).toBe(true) + }) + + it('stops loading once every row has arrived, however many books they stand for', async () => { + const authorId = ref(1) + mockFetchAuthorBooks.mockResolvedValueOnce(makePage([makeBook(1), makeBook(2)], 2, 200)) + + const { load, hasMore, bookTotal } = useAuthorBooks(authorId) + await load(true) + + expect(hasMore.value).toBe(false) + expect(bookTotal.value).toBe(200) + }) + it('resets items and page on reset=true', async () => { const authorId = ref(1) mockFetchAuthorBooks.mockResolvedValueOnce(makePage([makeBook(1)], 3)).mockResolvedValueOnce(makePage([makeBook(99)], 1)) @@ -199,14 +297,15 @@ describe('useAuthorBooks', () => { expect(loading.value).toBe(false) }) - it('passes sort, order, and libraryId to fetchAuthorBooks', async () => { + it('passes sort, order, libraryId, and collapseSeries to fetchAuthorBooks', async () => { const authorId = ref(5) mockFetchAuthorBooks.mockResolvedValue(makePage([])) - const { load, sort, order, libraryId } = useAuthorBooks(authorId) + const { load, sort, order, libraryId, collapseSeries } = useAuthorBooks(authorId) sort.value = 'title' order.value = 'asc' libraryId.value = 7 + collapseSeries.value = true await load(true) @@ -216,6 +315,7 @@ describe('useAuthorBooks', () => { sort: 'title', order: 'asc', libraryId: 7, + collapseSeries: true, }) }) @@ -270,10 +370,10 @@ describe('useAuthorBooks', () => { await load(true) expect(items.value).toHaveLength(1) - let resolve!: (v: BooksPage) => void + let resolve!: (v: AuthorBooksPage) => void mockFetchAuthorBooks.mockImplementation( () => - new Promise((r) => { + new Promise((r) => { resolve = r }), ) diff --git a/client/src/features/author/composables/useAuthorBooks.ts b/client/src/features/author/composables/useAuthorBooks.ts index fd0b37f32..d9332be8f 100644 --- a/client/src/features/author/composables/useAuthorBooks.ts +++ b/client/src/features/author/composables/useAuthorBooks.ts @@ -8,20 +8,33 @@ const PAGE_SIZE = 50 export function useAuthorBooks(authorId: Ref) { const items = ref([]) + /** Rows, so it keeps matching `items` once a series arrives as a single collapsed card. */ const total = ref(0) + /** Books behind those rows. Equal to `total` while the list is flat. */ + const bookTotal = ref(0) const loading = ref(false) const error = ref(null) const sort = ref('addedAt') const order = ref('desc') const libraryId = ref(null) + const collapseSeries = ref(false) const page = ref(0) const hasMore = computed(() => items.value.length < total.value) + /** + * Set when a reset arrives while a request is in flight: the query changed under it, so the + * pending response is stale and the list has to be rebuilt once that request settles. + */ + let resetQueued = false + async function load(reset = false): Promise { if (!authorId.value || Number.isNaN(authorId.value)) return - if (loading.value) return + if (loading.value) { + if (reset) resetQueued = true + return + } if (!reset && !hasMore.value) return loading.value = true @@ -39,27 +52,38 @@ export function useAuthorBooks(authorId: Ref) { sort: sort.value, order: order.value, libraryId: libraryId.value, + collapseSeries: collapseSeries.value, }) - items.value = reset ? data.items : [...items.value, ...data.items] - total.value = data.total - page.value += 1 + // Dropped on purpose when a reset is queued: these rows answer the previous query. + if (!resetQueued) { + items.value = reset ? data.items : [...items.value, ...data.items] + total.value = data.total + bookTotal.value = data.bookTotal + page.value += 1 + } } catch (err) { - error.value = err instanceof Error ? err.message : 'Failed to load books' + if (!resetQueued) error.value = err instanceof Error ? err.message : 'Failed to load books' } finally { loading.value = false + if (resetQueued) { + resetQueued = false + void load(true) + } } } return { items, total, + bookTotal, loading, error, hasMore, sort, order, libraryId, + collapseSeries, load, } } diff --git a/client/src/features/author/views/AuthorDetailView.spec.ts b/client/src/features/author/views/AuthorDetailView.spec.ts index 86270a345..0a5a605cb 100644 --- a/client/src/features/author/views/AuthorDetailView.spec.ts +++ b/client/src/features/author/views/AuthorDetailView.spec.ts @@ -29,12 +29,16 @@ const mocks = vi.hoisted(() => ({ authorNotFound: null as unknown as Ref, books: null as unknown as Ref, total: null as unknown as Ref, + bookTotal: null as unknown as Ref, loadingBooks: null as unknown as Ref, booksError: null as unknown as Ref, hasMore: null as unknown as Ref, sort: null as unknown as Ref<'addedAt' | 'title' | 'publishedYear'>, order: null as unknown as Ref<'asc' | 'desc'>, libraryId: null as unknown as Ref, + collapseSeries: null as unknown as Ref, + collapsePreference: false, + setCollapsePreference: vi.fn<(ctx: unknown, value: boolean | null) => Promise>(), })) vi.mock('vue-router', () => ({ @@ -102,16 +106,26 @@ vi.mock('../composables/useAuthorBooks', () => ({ useAuthorBooks: () => ({ items: mocks.books, total: mocks.total, + bookTotal: mocks.bookTotal, loading: mocks.loadingBooks, error: mocks.booksError, hasMore: mocks.hasMore, sort: mocks.sort, order: mocks.order, libraryId: mocks.libraryId, + collapseSeries: mocks.collapseSeries, load: mocks.loadBooks, }), })) +vi.mock('@/features/book/composables/useSeriesCollapsePreference', () => ({ + useSeriesCollapsePreference: () => ({ + getEffectivePreference: () => mocks.collapsePreference, + setPreference: mocks.setCollapsePreference, + prefs: ref(undefined), + }), +})) + vi.mock('../composables/useAuthorMetadataPreview', () => ({ useAuthorMetadataPreview: () => ({ preview: ref(null), @@ -228,12 +242,16 @@ describe('AuthorDetailView', () => { mocks.authorNotFound = ref(false) mocks.books = ref([makeBook(101), makeBook(102)]) mocks.total = ref(2) + mocks.bookTotal = ref(2) mocks.loadingBooks = ref(false) mocks.booksError = ref(null) mocks.hasMore = ref(false) mocks.sort = ref('addedAt') mocks.order = ref('desc') mocks.libraryId = ref(null) + mocks.collapseSeries = ref(false) + mocks.collapsePreference = false + mocks.setCollapsePreference.mockResolvedValue() }) afterEach(() => { @@ -275,6 +293,49 @@ describe('AuthorDetailView', () => { await flushPromises() } + describe('series collapse toggle', () => { + it('starts from the saved author pages preference', async () => { + mocks.collapsePreference = true + + const wrapper = await mountView() + + expect(mocks.collapseSeries.value).toBe(true) + expect(wrapper.get('[data-testid="author-collapse-series-toggle"]').attributes('aria-pressed')).toBe('true') + }) + + it('turns collapsing on and remembers it for every author page', async () => { + const wrapper = await mountView() + + await wrapper.get('[data-testid="author-collapse-series-toggle"]').trigger('click') + await flushPromises() + + expect(mocks.collapseSeries.value).toBe(true) + expect(mocks.setCollapsePreference).toHaveBeenCalledWith({ authorPages: true }, true) + }) + + it('turns collapsing back off', async () => { + mocks.collapsePreference = true + + const wrapper = await mountView() + await wrapper.get('[data-testid="author-collapse-series-toggle"]').trigger('click') + await flushPromises() + + expect(mocks.collapseSeries.value).toBe(false) + expect(mocks.setCollapsePreference).toHaveBeenCalledWith({ authorPages: true }, false) + }) + + it('counts books rather than rows in the all-loaded label', async () => { + // 2 collapsed rows standing for 200 books: the reader should be told about the books. + mocks.total = ref(2) + mocks.bookTotal = ref(200) + mocks.hasMore = ref(false) + + const wrapper = await mountView() + + expect(wrapper.text()).toContain(i18n.global.t('author.detail.books.allLoaded', { total: '200' })) + }) + }) + it('keeps unsaved edit fields when an author image is uploaded', async () => { vi.mocked(uploadAuthorImage).mockResolvedValue(makeAuthor({ imageUrl: '/api/v1/authors/7/image' })) diff --git a/client/src/features/author/views/AuthorDetailView.vue b/client/src/features/author/views/AuthorDetailView.vue index 5f5bade44..b81299117 100644 --- a/client/src/features/author/views/AuthorDetailView.vue +++ b/client/src/features/author/views/AuthorDetailView.vue @@ -4,7 +4,7 @@ import { computed, onMounted, onUnmounted, ref, watch } from 'vue' import { useI18n } from 'vue-i18n' import { formatNumber } from '@/i18n/formatters' import { useRoute, useRouter } from 'vue-router' -import { ArrowUpDown, ChevronDown, ChevronLeft, ImageMinus, LayoutGrid, List, Upload } from '@lucide/vue' +import { ArrowUpDown, ChevronDown, ChevronLeft, ImageMinus, Layers, LayoutGrid, List, Upload } from '@lucide/vue' import { toast } from 'vue-sonner' import type { AuthorDetail, AuthorSummary, BookCard } from '@bookorbit/types' @@ -12,6 +12,7 @@ import VirtualBookGrid from '@/features/book/components/VirtualBookGrid.vue' import BookListRow from '@/features/book/components/BookListRow.vue' import DeleteBookDialog from '@/features/book/components/DeleteBookDialog.vue' import { useScrollRestoreOnActivate } from '@/features/book/composables/useScrollRestoreOnActivate' +import { useSeriesCollapsePreference } from '@/features/book/composables/useSeriesCollapsePreference' import { useDisplaySettings } from '@/composables/useDisplaySettings' import { useLibraries } from '@/features/library/composables/useLibraries' import { usePermissions } from '@/features/auth/composables/usePermissions' @@ -49,7 +50,30 @@ const authorBooksViewMode = ref<'grid' | 'list'>('grid') const authorId = computed(() => Number(route.params.id)) const { author, loading: loadingAuthor, error: authorError, notFound: authorNotFound, load: loadAuthor } = useAuthorDetail(authorId) -const { items: books, total, loading: loadingBooks, error: booksError, hasMore, sort, order, libraryId, load: loadBooks } = useAuthorBooks(authorId) +const { + items: books, + total, + bookTotal, + loading: loadingBooks, + error: booksError, + hasMore, + sort, + order, + libraryId, + collapseSeries, + load: loadBooks, +} = useAuthorBooks(authorId) + +// One flag for every author page, so the state carries from one author to the next rather than +// being re-chosen per author; the toggle below writes it back. Tracked rather than read once, +// because on a cold load of an author URL the signed-in user - and so the preference - can +// arrive after this component is set up. +const { getEffectivePreference, setPreference } = useSeriesCollapsePreference() +watch( + () => getEffectivePreference({ authorPages: true }), + (value) => (collapseSeries.value = value), + { immediate: true }, +) const authorName = computed(() => author.value?.name ?? '') const pageTitle = computed(() => { if (author.value?.name) return t('author.detail.pageTitleNamed', { name: author.value.name }) @@ -129,7 +153,10 @@ const { books.value = books.value.filter((b) => b.id !== id) if (books.value.length === previousLength) return + // Only a plain book card carries a delete action - a collapsed series row opens the series + // instead - so the card that just went is worth exactly one row and one book. total.value = Math.max(0, total.value - 1) + bookTotal.value = Math.max(0, bookTotal.value - 1) if (author.value) { author.value = { ...author.value, @@ -209,6 +236,14 @@ function handleBookUpdate(updated: BookCard) { if (idx !== -1) books.value = books.value.map((b, i) => (i === idx ? updated : b)) } +const collapseToggleLabel = computed(() => (collapseSeries.value ? t('views.bookView.expandSeries') : t('views.bookView.collapseSeries'))) + +async function handleToggleCollapse() { + const next = !collapseSeries.value + collapseSeries.value = next + await setPreference({ authorPages: true }, next) +} + function loadIfSentinelVisible() { if (loadingBooks.value || !hasMore.value || !sentinel.value) return if (sentinel.value.getBoundingClientRect().top < window.innerHeight + 250) { @@ -448,7 +483,7 @@ watch(authorId, () => { void Promise.all([loadAuthor(), loadBooks(true)]).then(() => loadMetadataPreview()) }) -watch([sort, order, libraryId], () => { +watch([sort, order, libraryId, collapseSeries], () => { void loadBooks(true) }) @@ -618,7 +653,7 @@ defineOptions({ name: 'AuthorDetailView' })

{{ t('author.detail.books.heading') }}

-
+
+ +
diff --git a/client/src/features/book/composables/useSeriesCollapsePreference.test.ts b/client/src/features/book/composables/useSeriesCollapsePreference.test.ts index 74766766f..359078aff 100644 --- a/client/src/features/book/composables/useSeriesCollapsePreference.test.ts +++ b/client/src/features/book/composables/useSeriesCollapsePreference.test.ts @@ -238,7 +238,38 @@ describe('useSeriesCollapsePreference', () => { await setPreference('global', true) - expect(prefs.value).toEqual({ global: true, libraries: {}, collections: {}, smartScopes: {} }) + expect(prefs.value).toEqual({ global: true, libraries: {}, collections: {}, smartScopes: {}, authorPages: false }) + }) + + it('writes the author pages flag without touching the other scopes', async () => { + const userRef = ref(makeUser({ seriesCollapsePreferences: { global: true, libraries: { '2': true }, collections: {} } })) + mockUseAuth.mockReturnValue({ user: userRef } as ReturnType) + + const { setPreference, prefs } = useSeriesCollapsePreference() + + await setPreference({ authorPages: true }, true) + + expect(prefs.value!.authorPages).toBe(true) + expect(prefs.value!.global).toBe(true) + expect(prefs.value!.libraries).toEqual({ '2': true }) + expect(mockApi).toHaveBeenCalledWith( + '/api/v1/users/me/series-collapse-preferences', + expect.objectContaining({ + method: 'PATCH', + body: JSON.stringify({ authorPages: true }), + }), + ) + }) + + it('turns the author pages flag back off', async () => { + const userRef = ref(makeUser({ seriesCollapsePreferences: { global: false, libraries: {}, collections: {}, authorPages: true } })) + mockUseAuth.mockReturnValue({ user: userRef } as ReturnType) + + const { setPreference, prefs } = useSeriesCollapsePreference() + + await setPreference({ authorPages: true }, false) + + expect(prefs.value!.authorPages).toBe(false) }) it('preserves existing library overrides when setting global', async () => { diff --git a/client/src/features/book/composables/useSeriesCollapsePreference.ts b/client/src/features/book/composables/useSeriesCollapsePreference.ts index 882c0991b..6777ca4f4 100644 --- a/client/src/features/book/composables/useSeriesCollapsePreference.ts +++ b/client/src/features/book/composables/useSeriesCollapsePreference.ts @@ -4,6 +4,12 @@ import { resolveCollapsePreference } from '@bookorbit/types' import { useAuth } from '@/features/auth/composables/useAuth' import { api } from '@/lib/api' +/** + * A place whose collapse state is remembered. `authorPages` is a single flag rather than an id, + * so every author page shares it - see the type in `@bookorbit/types` for why. + */ +export type CollapseScope = { libraryId?: number; collectionId?: number; smartScopeId?: number; authorPages?: boolean } + export function useSeriesCollapsePreference() { const { user } = useAuth() @@ -11,28 +17,24 @@ export function useSeriesCollapsePreference() { return user.value?.settings.seriesCollapsePreferences }) - function getEffectivePreference(ctx: { libraryId?: number; collectionId?: number; smartScopeId?: number }): boolean { + function getEffectivePreference(ctx: CollapseScope): boolean { return resolveCollapsePreference(prefs.value, ctx) } let pendingUpdate: Promise = Promise.resolve() - async function setPreference( - ctx: { libraryId?: number; collectionId?: number; smartScopeId?: number } | 'global', - value: boolean | null, - ): Promise { + async function setPreference(ctx: CollapseScope | 'global', value: boolean | null): Promise { const op = () => doSetPreference(ctx, value) pendingUpdate = pendingUpdate.then(op, op) return pendingUpdate } - async function doSetPreference( - ctx: { libraryId?: number; collectionId?: number; smartScopeId?: number } | 'global', - value: boolean | null, - ): Promise { + async function doSetPreference(ctx: CollapseScope | 'global', value: boolean | null): Promise { let body: Record if (ctx === 'global') { body = { global: value === null ? false : value } + } else if (ctx.authorPages) { + body = { authorPages: value === null ? false : value } } else if (ctx.smartScopeId !== undefined) { body = { smartScopes: { [String(ctx.smartScopeId)]: value } } } else if (ctx.collectionId !== undefined) { @@ -64,6 +66,7 @@ export function useSeriesCollapsePreference() { libraries: nextLibraries as Record, collections: nextCollections as Record, smartScopes: nextSmartScopes as Record, + authorPages: body.authorPages !== undefined ? (body.authorPages as boolean) : (current.authorPages ?? false), } const previous = user.value?.settings ? { ...user.value.settings } : undefined diff --git a/packages/types/src/author.ts b/packages/types/src/author.ts index 0407a65f8..b47135512 100644 --- a/packages/types/src/author.ts +++ b/packages/types/src/author.ts @@ -43,7 +43,15 @@ export type AuthorsPage = { size: number; }; -export type AuthorBooksPage = BooksPage; +/** + * `total` counts rows in the page's own unit, so it is the number of cards once + * `collapseSeries` folds a series into one card - that is what paginating and "is there more" + * have to be driven by. `bookTotal` stays the number of books behind those rows, for the + * places that count books rather than cards. The two are equal while the list is flat. + */ +export type AuthorBooksPage = BooksPage & { + bookTotal: number; +}; export type MergeAuthorsResult = { target: AuthorDetail; diff --git a/packages/types/src/series-collapse.ts b/packages/types/src/series-collapse.ts index 565bc3f41..8c0f4bdc4 100644 --- a/packages/types/src/series-collapse.ts +++ b/packages/types/src/series-collapse.ts @@ -3,6 +3,13 @@ export type SeriesCollapsePreferences = { libraries: Record; collections: Record; smartScopes?: Record; + /** + * Author pages share one flag instead of a per-author bucket: the bucket would grow with every + * author browsed, and "collapsed for King but not for Pratchett" is not a distinction anyone + * asks for. Absent means off, so an upgrade leaves author pages flat even for a user whose + * {@link SeriesCollapsePreferences.global} is on. + */ + authorPages?: boolean; }; export type CollapsedSeriesInfo = { @@ -18,9 +25,12 @@ export type CollapsedSeriesInfo = { export function resolveCollapsePreference( prefs: SeriesCollapsePreferences | undefined, - ctx: { libraryId?: number; collectionId?: number; smartScopeId?: number }, + ctx: { libraryId?: number; collectionId?: number; smartScopeId?: number; authorPages?: boolean }, ): boolean { if (!prefs) return false; + // Author pages resolve to their own flag and stop there - they deliberately do not inherit the + // global default, and the library filter on an author page is a filter, not a scope to override. + if (ctx.authorPages) return prefs.authorPages ?? false; if (ctx.smartScopeId !== undefined) { const override = prefs.smartScopes?.[String(ctx.smartScopeId)]; if (override !== undefined) return override; diff --git a/scripts/e2e/suite-registry.mjs b/scripts/e2e/suite-registry.mjs index 18fe318e5..830e4da22 100644 --- a/scripts/e2e/suite-registry.mjs +++ b/scripts/e2e/suite-registry.mjs @@ -351,6 +351,27 @@ export const E2E_SUITES = Object.freeze({ ...SHARED_DB_AND_HELPER_PATHS, ], }, + "author-books-collapse": { + id: "author-books-collapse", + name: "Author Books Collapse", + timeout: 60, + lane: "full", + description: "Author page series collapsing, and the row and book counts it reports", + vitestTarget: "test/author-books-collapse.e2e-spec.ts", + junitOutput: `${TEST_RESULTS_DIR}/author-books-collapse-e2e-junit.xml`, + prepareDedicatedDatabase: true, + useDedicatedDatabase: true, + changedPaths: [ + "server/src/modules/authors/**", + "server/src/modules/book/book.repository.ts", + "server/src/modules/book/book-read.service.ts", + "server/test/author-books-collapse.e2e-spec.ts", + "server/test/e2e/authorization-matrix/**", + ".github/workflows/e2e-runner.yml", + "packages/types/src/author.ts", + ...SHARED_DB_AND_HELPER_PATHS, + ], + }, "series-next-book": { id: "series-next-book", name: "Series Next Book", diff --git a/server/src/modules/authors/authors.repository.ts b/server/src/modules/authors/authors.repository.ts index 019dff8bb..1630123d0 100644 --- a/server/src/modules/authors/authors.repository.ts +++ b/server/src/modules/authors/authors.repository.ts @@ -1,5 +1,5 @@ import { Inject, Injectable } from '@nestjs/common'; -import { SQL, and, asc, desc, eq, inArray, isNull, max, or, sql } from 'drizzle-orm'; +import { SQL, and, asc, desc, eq, exists, inArray, isNull, max, or, sql } from 'drizzle-orm'; import { NodePgDatabase } from 'drizzle-orm/node-postgres'; import type { ContentFilterRules } from '@bookorbit/types'; @@ -290,6 +290,30 @@ export class AuthorsRepository { return { bookIds: rows.map((row: AuthorBookIdRow) => row.id), total: Number(total), page: params.page, size: params.size }; } + /** + * The same "books of this author the user may see" predicate as {@link findBookIdsPage}, shaped + * for the collapsed book query. That query folds series in SQL over a CTE selecting from books + * alone, so authorship has to arrive as a correlated EXISTS rather than the join used above. + * + * Keyed on the author id, not the author name: merged authors and two writers sharing a name + * stay distinct here, which a name-based filter would silently collapse together. + */ + buildBooksWhere(params: { authorId: number; libraryIds: number[]; contentFilters?: ContentFilterRules }): SQL { + // Never `undefined` for the empty case: an absent predicate reads as "no restriction" to the + // query that consumes this, which is the opposite of what no accessible library means. + if (params.libraryIds.length === 0) return sql`false`; + + const byThisAuthor = exists( + this.db + .select({ one: sql`1` }) + .from(bookAuthors) + .where(and(sql`${bookAuthors.bookId} = ${books.id}`, eq(bookAuthors.authorId, params.authorId))), + ); + + const filterClauses = params.contentFilters ? buildContentFilterClauses(params.contentFilters, this.db) : []; + return and(byThisAuthor, inArray(books.libraryId, params.libraryIds), ...filterClauses)!; + } + async updateAuthorById( authorId: number, values: Partial<{ diff --git a/server/src/modules/authors/authors.service.test.ts b/server/src/modules/authors/authors.service.test.ts index 1d3878eb4..41e37a735 100644 --- a/server/src/modules/authors/authors.service.test.ts +++ b/server/src/modules/authors/authors.service.test.ts @@ -42,6 +42,7 @@ describe('AuthorsService', () => { findPage: vi.fn(), findById: vi.fn(), findBookIdsPage: vi.fn(), + buildBooksWhere: vi.fn(), updateAuthorById: vi.fn(), findVisibleAuthorIds: vi.fn(), countDistinctBooks: vi.fn(), @@ -57,6 +58,8 @@ describe('AuthorsService', () => { const bookReadService = { findCardsByBookIds: vi.fn(), + findCardsCollapsed: vi.fn(), + countWhere: vi.fn(), }; const libraryService = { @@ -354,7 +357,7 @@ describe('AuthorsService', () => { const page = await service.findBooks(reqUser(), 99, {}); - expect(page).toEqual({ items: [], total: 0, page: 0, size: 50 }); + expect(page).toEqual({ items: [], total: 0, bookTotal: 0, page: 0, size: 50 }); expect(authorsRepo.findById).not.toHaveBeenCalled(); }); @@ -374,10 +377,116 @@ describe('AuthorsService', () => { const result = await service.findBooks(reqUser(), 10, {}); - expect(result).toEqual({ items: [], total: 0, page: 0, size: 50 }); + expect(result).toEqual({ items: [], total: 0, bookTotal: 0, page: 0, size: 50 }); expect(bookReadService.findCardsByBookIds).not.toHaveBeenCalled(); }); + describe('findBooks with collapseSeries', () => { + const AUTHOR_WHERE = { fake: 'where' } as any; + + function collapsedRow(id: number, seriesName: string, bookCount: number) { + return { + ...makeBookRow(id, `${seriesName} #1`), + seriesId: id * 100, + seriesName, + bookCount, + readCount: 0, + coverBookIds: [id], + coverUpdatedAtByBookId: null, + seriesLatestAddedAt: new Date('2026-08-01T00:00:00.000Z'), + firstVolumeBookId: id, + latestVolumeBookId: id, + firstUnreadBookId: id, + updatedAt: new Date('2026-08-01T00:00:00.000Z'), + }; + } + + function emptyCollapsedResult(rows: ReturnType[], total: number, bookTotal = total) { + return { + rows, + authorRows: [], + fileRows: [], + genreRows: [], + tagRows: [], + progressRows: [], + statusRows: [], + narratorRows: [], + seriesMembershipRows: [], + total, + bookTotal, + }; + } + + beforeEach(() => { + authorsRepo.findById.mockResolvedValue({ id: 10, name: 'Author', bookCount: 200 }); + authorsRepo.buildBooksWhere.mockReturnValue(AUTHOR_WHERE); + }); + + it('routes to the collapsed query instead of the flat one', async () => { + bookReadService.findCardsCollapsed.mockResolvedValue(emptyCollapsedResult([collapsedRow(1, 'Discworld', 41)], 30, 200)); + + const result = await service.findBooks(reqUser(), 10, { collapseSeries: true }); + + expect(authorsRepo.findBookIdsPage).not.toHaveBeenCalled(); + expect(bookReadService.findCardsCollapsed).toHaveBeenCalledWith( + expect.objectContaining({ where: AUTHOR_WHERE, limit: 50, offset: 0, userId: 7 }), + ); + expect(result.items[0]!.collapsedSeries).toEqual(expect.objectContaining({ bookCount: 41 })); + }); + + it('reports rows as total and books as bookTotal', async () => { + bookReadService.findCardsCollapsed.mockResolvedValue(emptyCollapsedResult([collapsedRow(1, 'Discworld', 41)], 30, 200)); + + const result = await service.findBooks(reqUser(), 10, { collapseSeries: true }); + + expect(result.total).toBe(30); + expect(result.bookTotal).toBe(200); + }); + + it('uses counts returned with the collapsed rows without a second query', async () => { + bookReadService.findCardsCollapsed.mockResolvedValue(emptyCollapsedResult([], 30, 200)); + + const result = await service.findBooks(reqUser(), 10, { collapseSeries: true, page: 20 }); + + expect(result).toEqual({ items: [], total: 30, bookTotal: 200, page: 20, size: 50 }); + expect(bookReadService.countWhere).not.toHaveBeenCalled(); + }); + + it('maps the author sort onto the collapsed sort spec', async () => { + bookReadService.findCardsCollapsed.mockResolvedValue(emptyCollapsedResult([], 0)); + + await service.findBooks(reqUser(), 10, { collapseSeries: true, sort: 'title', order: 'asc', page: 2, size: 20 }); + + expect(bookReadService.findCardsCollapsed).toHaveBeenCalledWith( + expect.objectContaining({ sort: [{ field: 'title', dir: 'asc' }], limit: 20, offset: 40 }), + ); + }); + + it('scopes the collapsed query by author id, libraries, and content filters', async () => { + bookReadService.findCardsCollapsed.mockResolvedValue(emptyCollapsedResult([], 0)); + libraryService.findAll.mockResolvedValue([{ id: 3 }]); + + await service.findBooks({ ...reqUser(), contentFilters: EMPTY_CONTENT_FILTER_RULES }, 10, { collapseSeries: true }); + + expect(authorsRepo.buildBooksWhere).toHaveBeenCalledWith({ + authorId: 10, + libraryIds: [3], + contentFilters: EMPTY_CONTENT_FILTER_RULES, + }); + }); + + it('leaves the flat path untouched when the flag is absent', async () => { + authorsRepo.findBookIdsPage.mockResolvedValue({ bookIds: [], total: 7, page: 0, size: 50 }); + + const result = await service.findBooks(reqUser(), 10, {}); + + expect(bookReadService.findCardsCollapsed).not.toHaveBeenCalled(); + // Nothing is folded, so the two counts are the same number. + expect(result.total).toBe(7); + expect(result.bookTotal).toBe(7); + }); + }); + it('findBooks passes sort, order, size, and libraryId to findBookIdsPage', async () => { authorsRepo.findById.mockResolvedValue({ id: 10, name: 'Author', bookCount: 3 }); authorsRepo.findBookIdsPage.mockResolvedValue({ bookIds: [], total: 0, page: 0, size: 10 }); diff --git a/server/src/modules/authors/authors.service.ts b/server/src/modules/authors/authors.service.ts index f24d04b0f..d33cd874b 100644 --- a/server/src/modules/authors/authors.service.ts +++ b/server/src/modules/authors/authors.service.ts @@ -2,16 +2,16 @@ import { BadRequestException, ForbiddenException, Injectable, Logger, NotFoundEx import { Observable } from 'rxjs'; import type { + AuthorBooksPage, AuthorDetail, AuthorMetadataCandidate, AuthorMetadataProviderInfo, AuthorSummary, AuthorsPage, JumpBucketsResponse, - BooksPage, MergeAuthorsResult, } from '@bookorbit/types'; -import { assembleBookCards } from '../book/utils/assemble-book-cards'; +import { assembleBookCards, assembleCollapsedBookCards } from '../book/utils/assemble-book-cards'; import { MAX_OFFSET_ROWS, isOffsetWithinLimit } from '../../common/constants/pagination.constants'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; import { normalizeMetadataText } from '../../common/utils/metadata-text-normalize.utils'; @@ -134,16 +134,20 @@ export class AuthorsService { return this.withAuthorImageUrl(this.mapAuthorDetail(row), 'full'); } - async findBooks(user: RequestUser, authorId: number, dto: ListAuthorBooksDto): Promise { + async findBooks(user: RequestUser, authorId: number, dto: ListAuthorBooksDto): Promise { this.assertPaginationWindow(dto.page ?? 0, dto.size ?? 50); const libraryIds = await this.resolveLibraryIds(user, dto.libraryId); if (libraryIds.length === 0) { - return { items: [], total: 0, page: dto.page ?? 0, size: dto.size ?? 50 }; + return { items: [], total: 0, bookTotal: 0, page: dto.page ?? 0, size: dto.size ?? 50 }; } const author = await this.authorsRepo.findById(authorId, libraryIds, user.isSuperuser ? undefined : user.contentFilters); if (!author) throw new NotFoundException('Author not found'); + if (dto.collapseSeries === true) { + return this.findBooksCollapsed(user, authorId, dto, libraryIds); + } + const page = await this.authorsRepo.findBookIdsPage({ authorId, page: dto.page ?? 0, @@ -155,7 +159,7 @@ export class AuthorsService { }); if (page.bookIds.length === 0) { - return { items: [], total: page.total, page: page.page, size: page.size }; + return { items: [], total: page.total, bookTotal: page.total, page: page.page, size: page.size }; } const orderMap = new Map(page.bookIds.map((id, index) => [id, index])); @@ -172,7 +176,49 @@ export class AuthorsService { cardData.seriesMembershipRows, ).sort((a, b) => (orderMap.get(a.id) ?? 0) - (orderMap.get(b.id) ?? 0)); - return { items, total: page.total, page: page.page, size: page.size }; + return { items, total: page.total, bookTotal: page.total, page: page.page, size: page.size }; + } + + /** + * The collapsed half of {@link findBooks}: the same author scope, run through the book module's + * collapsed query so a series arrives as one card carrying its `collapsedSeries` payload. The + * per-series counts come out of that query scoped to the rows it saw, so they say "books by this + * author in the series" - which is the number an author page should show. + * + * `total` is the row count the collapsed query reports, so paging still walks the list it is + * actually rendering; `bookTotal` counts the books behind those rows for the callers that speak + * in books. Counting both under one visibility rule keeps them from disagreeing. + */ + private async findBooksCollapsed(user: RequestUser, authorId: number, dto: ListAuthorBooksDto, libraryIds: number[]): Promise { + const page = dto.page ?? 0; + const size = dto.size ?? 50; + const where = this.authorsRepo.buildBooksWhere({ + authorId, + libraryIds, + contentFilters: user.isSuperuser ? undefined : user.contentFilters, + }); + + const collapsed = await this.bookReadService.findCardsCollapsed({ + where, + sort: [{ field: dto.sort ?? 'addedAt', dir: dto.order ?? 'desc' }], + limit: size, + offset: page * size, + userId: user.id, + }); + + const items = assembleCollapsedBookCards( + collapsed.rows, + collapsed.authorRows, + collapsed.fileRows, + collapsed.genreRows, + collapsed.progressRows, + collapsed.statusRows, + collapsed.narratorRows, + collapsed.tagRows, + collapsed.seriesMembershipRows, + ); + + return { items, total: collapsed.total, bookTotal: collapsed.bookTotal, page, size }; } listMetadataProviders(): AuthorMetadataProviderInfo[] { diff --git a/server/src/modules/authors/dto/authors-dto-validation.test.ts b/server/src/modules/authors/dto/authors-dto-validation.test.ts index 0eb8ff04d..51085f985 100644 --- a/server/src/modules/authors/dto/authors-dto-validation.test.ts +++ b/server/src/modules/authors/dto/authors-dto-validation.test.ts @@ -65,6 +65,19 @@ describe('Authors DTO validation', () => { expect((await errorsFor(ListAuthorBooksDto, { size: '101' })).length).toBeGreaterThan(0); }); + it('reads collapseSeries out of the query string', async () => { + const dto = plainToInstance(ListAuthorBooksDto, { collapseSeries: 'true' }); + expect(await validate(dto)).toHaveLength(0); + expect(dto.collapseSeries).toBe(true); + + // Anything that is not the string "true" means flat, so a stray value cannot switch it on. + const off = plainToInstance(ListAuthorBooksDto, { collapseSeries: 'false' }); + expect(await validate(off)).toHaveLength(0); + expect(off.collapseSeries).toBe(false); + + expect(plainToInstance(ListAuthorBooksDto, {}).collapseSeries).toBeUndefined(); + }); + it('normalizes metadata provider query values and rejects unknown providers', async () => { const dto = plainToInstance(ListAuthorMetadataDto, { q: 'Jane Doe', diff --git a/server/src/modules/authors/dto/list-author-books.dto.ts b/server/src/modules/authors/dto/list-author-books.dto.ts index 959fb1c1f..f81ef1517 100644 --- a/server/src/modules/authors/dto/list-author-books.dto.ts +++ b/server/src/modules/authors/dto/list-author-books.dto.ts @@ -1,5 +1,5 @@ -import { Type } from 'class-transformer'; -import { IsIn, IsInt, IsOptional, Max, Min } from 'class-validator'; +import { Transform, Type } from 'class-transformer'; +import { IsBoolean, IsIn, IsInt, IsOptional, Max, Min } from 'class-validator'; import { SORT_DIRECTIONS, type SortDirection } from './list-authors.dto'; @@ -33,4 +33,10 @@ export class ListAuthorBooksDto { @IsInt() @Min(1) libraryId?: number; + + /** Fold each series into a single card, the way the library and collection listings do. */ + @IsOptional() + @Transform(({ value }) => value === 'true' || value === true) + @IsBoolean() + collapseSeries?: boolean; } diff --git a/server/src/modules/book/book-read.service.ts b/server/src/modules/book/book-read.service.ts index 3c6135364..fa704a9a4 100644 --- a/server/src/modules/book/book-read.service.ts +++ b/server/src/modules/book/book-read.service.ts @@ -31,6 +31,10 @@ export class BookReadService { return this.bookRepo.findCardsByBookIds(bookIds, userId); } + findCardsCollapsed(opts: Parameters[0]) { + return this.bookRepo.findCardsCollapsed(opts); + } + countWhere(where: SQL | undefined): Promise { return this.bookRepo.countWhere(where); } diff --git a/server/src/modules/book/book.repository.test.ts b/server/src/modules/book/book.repository.test.ts index a626dca1f..7e5d0b90f 100644 --- a/server/src/modules/book/book.repository.test.ts +++ b/server/src/modules/book/book.repository.test.ts @@ -348,6 +348,64 @@ describe('BookRepository', () => { expect(query.sql).not.toContain('FROM "collection_books"'); }); + it('computes row and book totals before paging collapsed cards', async () => { + const execute = vi.fn().mockResolvedValue({ rows: [] }); + const repo = new BookRepository({ execute } as never); + + await repo.findCardsCollapsed({ + where: undefined, + sort: [{ field: 'title', dir: 'asc' }], + limit: 20, + offset: 40, + userId: 7, + }); + + const query = new PgDialect().sqlToQuery(execute.mock.calls[0]![0]); + expect(query.sql).toContain('COUNT(*) AS total_count'); + expect(query.sql).toContain('COALESCE(SUM(COALESCE(book_count, 1)), 0) AS book_total'); + expect(query.sql).toContain('LEFT JOIN LATERAL'); + expect(query.sql).toMatch(/LIMIT \$\d+ OFFSET \$\d+/); + expect(query.params).toEqual(expect.arrayContaining([20, 40])); + }); + + it('returns totals from the sentinel row when a collapsed page is empty', async () => { + const execute = vi.fn().mockResolvedValue({ + rows: [{ id: null, total_count: '30', book_total: '200' }], + }); + const repo = new BookRepository({ execute } as never); + + const result = await repo.findCardsCollapsed({ + where: undefined, + sort: [{ field: 'title', dir: 'asc' }], + limit: 50, + offset: 1_000, + userId: 7, + }); + + expect(result.rows).toEqual([]); + expect(result.total).toBe(30); + expect(result.bookTotal).toBe(200); + }); + + it('returns zero totals when the collapsed scope has no books', async () => { + const execute = vi.fn().mockResolvedValue({ + rows: [{ id: null, total_count: '0', book_total: '0' }], + }); + const repo = new BookRepository({ execute } as never); + + const result = await repo.findCardsCollapsed({ + where: undefined, + sort: [{ field: 'title', dir: 'asc' }], + limit: 50, + offset: 0, + userId: 7, + }); + + expect(result.rows).toEqual([]); + expect(result.total).toBe(0); + expect(result.bookTotal).toBe(0); + }); + it('rejects an unusable collection id on the collapsed path', async () => { const execute = vi.fn().mockResolvedValue({ rows: [] }); const repo = new BookRepository({ execute } as never); diff --git a/server/src/modules/book/book.repository.ts b/server/src/modules/book/book.repository.ts index 6585c5d11..24bbd9d48 100644 --- a/server/src/modules/book/book.repository.ts +++ b/server/src/modules/book/book.repository.ts @@ -67,7 +67,7 @@ type JsonObj = Record; type BookRepositoryTx = Parameters['transaction']>[0]>[0]; type CollapsedRawRow = { - id: number; + id: number | null; status: string; cover_aspect_ratio: string; primary_file_id: number | null; @@ -101,6 +101,7 @@ type CollapsedRawRow = { latest_volume_book_id: number | null; first_unread_book_id: number | null; total_count: string; + book_total: string; }; type PatternMetadataRow = { bookId: number; @@ -822,6 +823,7 @@ export class BookRepository { expectedBookCount: number | null; }[]; total: number; + bookTotal: number; }> { const { where, sort, limit, offset, userId, defaultCollectionId } = opts; if (defaultCollectionId !== undefined && (!Number.isSafeInteger(defaultCollectionId) || defaultCollectionId <= 0)) { @@ -1041,16 +1043,28 @@ export class BookRepository { ON sfu2.series_id = base.series_id AND sfu2.library_id = base.library_id ORDER BY ${sql.raw(COLLAPSE_REPRESENTATIVE_PICK_SQL)} + ), + totals AS ( + SELECT + COUNT(*) AS total_count, + COALESCE(SUM(COALESCE(book_count, 1)), 0) AS book_total + FROM representatives ) - SELECT r.*, - COUNT(*) OVER () AS total_count - FROM representatives r + SELECT r.*, totals.total_count, totals.book_total + FROM totals + LEFT JOIN LATERAL ( + SELECT r.* + FROM representatives r + ORDER BY ${sql.raw(orderBy)} + LIMIT ${limit} OFFSET ${offset} + ) r ON true ORDER BY ${sql.raw(orderBy)} - LIMIT ${limit} OFFSET ${offset} `); - const rawRows = result.rows as CollapsedRawRow[]; - const total = rawRows.length > 0 ? Number(rawRows[0].total_count) : 0; + const queryRows = result.rows as CollapsedRawRow[]; + const total = Number(queryRows[0]?.total_count ?? 0); + const bookTotal = Number(queryRows[0]?.book_total ?? 0); + const rawRows = queryRows.filter((row): row is CollapsedRawRow & { id: number } => row.id !== null); const mappedRows = rawRows.map((r) => ({ id: r.id, @@ -1090,7 +1104,7 @@ export class BookRepository { const bookRefs = mappedRows.map((row) => ({ id: row.id, primaryFileId: row.primaryFileId ?? null })); const enrichment = await this.enrichBookIds(bookRefs, userId); - return { rows: mappedRows, ...enrichment, total }; + return { rows: mappedRows, ...enrichment, total, bookTotal }; } async findJumpBuckets(opts: { diff --git a/server/src/modules/book/series-collapse-prefs.test.ts b/server/src/modules/book/series-collapse-prefs.test.ts index a5e9ee3a6..06ce010ea 100644 --- a/server/src/modules/book/series-collapse-prefs.test.ts +++ b/server/src/modules/book/series-collapse-prefs.test.ts @@ -66,4 +66,35 @@ describe('resolveCollapsePreference', () => { const prefs: SeriesCollapsePreferences = { global: false, libraries: { '42': true }, collections: {} }; expect(resolveCollapsePreference(prefs, { libraryId: 42 })).toBe(true); }); + + describe('author pages', () => { + it('returns the author pages flag when it is set', () => { + const prefs: SeriesCollapsePreferences = { global: false, libraries: {}, collections: {}, authorPages: true }; + expect(resolveCollapsePreference(prefs, { authorPages: true })).toBe(true); + }); + + it('stays off for prefs saved before the flag existed, whatever global says', () => { + const prefs: SeriesCollapsePreferences = { global: true, libraries: {}, collections: {} }; + expect(resolveCollapsePreference(prefs, { authorPages: true })).toBe(false); + }); + + it('does not inherit global, in either direction', () => { + const globalOn: SeriesCollapsePreferences = { global: true, libraries: {}, collections: {}, authorPages: false }; + expect(resolveCollapsePreference(globalOn, { authorPages: true })).toBe(false); + + const globalOff: SeriesCollapsePreferences = { global: false, libraries: {}, collections: {}, authorPages: true }; + expect(resolveCollapsePreference(globalOff, { authorPages: true })).toBe(true); + }); + + it('ignores library overrides, since the author page filter is not a scope', () => { + const prefs: SeriesCollapsePreferences = { global: false, libraries: { '3': true }, collections: {}, authorPages: false }; + expect(resolveCollapsePreference(prefs, { authorPages: true, libraryId: 3 })).toBe(false); + }); + + it('leaves the other scopes untouched when the flag is on', () => { + const prefs: SeriesCollapsePreferences = { global: false, libraries: { '3': true }, collections: {}, authorPages: true }; + expect(resolveCollapsePreference(prefs, { libraryId: 3 })).toBe(true); + expect(resolveCollapsePreference(prefs, {})).toBe(false); + }); + }); }); diff --git a/server/src/modules/user/dto/update-series-collapse-preferences.dto.ts b/server/src/modules/user/dto/update-series-collapse-preferences.dto.ts index 92dbdb32d..3456e9451 100644 --- a/server/src/modules/user/dto/update-series-collapse-preferences.dto.ts +++ b/server/src/modules/user/dto/update-series-collapse-preferences.dto.ts @@ -36,4 +36,9 @@ export class UpdateSeriesCollapsePreferencesDto { @IsOptional() @IsBooleanOrNullRecord() smartScopes?: Record; + + /** One flag for every author page; there is no per-author bucket to override it. */ + @IsOptional() + @IsBoolean() + authorPages?: boolean; } diff --git a/server/src/modules/user/user.service.test.ts b/server/src/modules/user/user.service.test.ts index edfd0592b..79af10088 100644 --- a/server/src/modules/user/user.service.test.ts +++ b/server/src/modules/user/user.service.test.ts @@ -809,7 +809,7 @@ describe('UserService.updateSeriesCollapsePreferences', () => { expect(userRepo.update).toHaveBeenCalledWith(1, { settings: { - seriesCollapsePreferences: { global: true, libraries: {}, collections: {}, smartScopes: {} }, + seriesCollapsePreferences: { global: true, libraries: {}, collections: {}, smartScopes: {}, authorPages: false }, }, }); }); @@ -826,7 +826,7 @@ describe('UserService.updateSeriesCollapsePreferences', () => { expect(userRepo.update).toHaveBeenCalledWith(1, { settings: { - seriesCollapsePreferences: { global: true, libraries: { '3': true }, collections: { '7': false }, smartScopes: {} }, + seriesCollapsePreferences: { global: true, libraries: { '3': true }, collections: { '7': false }, smartScopes: {}, authorPages: false }, }, }); }); @@ -843,7 +843,7 @@ describe('UserService.updateSeriesCollapsePreferences', () => { expect(userRepo.update).toHaveBeenCalledWith(1, { settings: { - seriesCollapsePreferences: { global: false, libraries: { '1': true, '2': false }, collections: {}, smartScopes: {} }, + seriesCollapsePreferences: { global: false, libraries: { '1': true, '2': false }, collections: {}, smartScopes: {}, authorPages: false }, }, }); }); @@ -860,7 +860,7 @@ describe('UserService.updateSeriesCollapsePreferences', () => { expect(userRepo.update).toHaveBeenCalledWith(1, { settings: { - seriesCollapsePreferences: { global: true, libraries: {}, collections: { '5': false, '9': true }, smartScopes: {} }, + seriesCollapsePreferences: { global: true, libraries: {}, collections: { '5': false, '9': true }, smartScopes: {}, authorPages: false }, }, }); }); @@ -897,8 +897,45 @@ describe('UserService.updateSeriesCollapsePreferences', () => { expect(userRepo.update).toHaveBeenCalledWith(1, { settings: { - seriesCollapsePreferences: { global: false, libraries: {}, collections: {}, smartScopes: { '3': true } }, + seriesCollapsePreferences: { global: false, libraries: {}, collections: {}, smartScopes: { '3': true }, authorPages: false }, }, }); }); + + it('sets the author pages flag on its own', async () => { + userRepo.findByIdWithPermissions.mockResolvedValue({ + id: 1, + settings: { + seriesCollapsePreferences: { global: false, libraries: { '3': true }, collections: {} }, + }, + }); + + await service.updateSeriesCollapsePreferences(1, { authorPages: true }); + + expect(userRepo.update).toHaveBeenCalledWith(1, { + settings: { + seriesCollapsePreferences: { global: false, libraries: { '3': true }, collections: {}, smartScopes: {}, authorPages: true }, + }, + }); + }); + + it('leaves the author pages flag alone when another scope is written', async () => { + userRepo.findByIdWithPermissions.mockResolvedValue({ + id: 1, + settings: { + seriesCollapsePreferences: { global: false, libraries: {}, collections: {}, authorPages: true }, + }, + }); + + await service.updateSeriesCollapsePreferences(1, { global: true }); + + expect(userRepo.update).toHaveBeenCalledWith( + 1, + expect.objectContaining({ + settings: expect.objectContaining({ + seriesCollapsePreferences: expect.objectContaining({ authorPages: true }), + }), + }), + ); + }); }); diff --git a/server/src/modules/user/user.service.ts b/server/src/modules/user/user.service.ts index f14766882..2af29fe0c 100644 --- a/server/src/modules/user/user.service.ts +++ b/server/src/modules/user/user.service.ts @@ -504,6 +504,7 @@ export class UserService { libraries: { ...currentPrefs.libraries, ...(dto.libraries ?? {}) }, collections: { ...currentPrefs.collections, ...(dto.collections ?? {}) }, smartScopes: { ...(currentPrefs.smartScopes ?? {}), ...(dto.smartScopes ?? {}) }, + authorPages: dto.authorPages !== undefined ? dto.authorPages : (currentPrefs.authorPages ?? false), }; // Remove entries set to null (deletion of overrides) diff --git a/server/test/author-books-collapse.e2e-spec.ts b/server/test/author-books-collapse.e2e-spec.ts new file mode 100644 index 000000000..bfff69899 --- /dev/null +++ b/server/test/author-books-collapse.e2e-spec.ts @@ -0,0 +1,219 @@ +import { randomUUID } from 'crypto'; + +import type { BookCard } from '@bookorbit/types'; + +import * as schema from '../src/db/schema'; +import { + authHeader, + closeAuthorizationMatrixE2EContext, + createAuthorizationMatrixE2EContext, + createLibraryWithFolder, + createUserAndLogin, + grantLibraryAccess, + type AuthorizationMatrixE2EContext, + type CreatedLibrary, + type TestUserSession, +} from './e2e/authorization-matrix/authorization-matrix-harness'; + +const SCENARIO_TIMEOUT_MS = 60_000; + +let inoSequence = 960_000; + +type SeededSeries = { id: number; name: string }; + +async function createAuthor(ctx: AuthorizationMatrixE2EContext, name: string): Promise { + const [author] = await ctx.db + .insert(schema.authors) + .values({ name: `${name} ${randomUUID()}` }) + .returning({ id: schema.authors.id }); + return author!.id; +} + +async function createSeries(ctx: AuthorizationMatrixE2EContext, name: string): Promise { + const [series] = await ctx.db + .insert(schema.bookSeries) + .values({ name, normalizedName: `${name.toLowerCase()} ${randomUUID()}` }) + .returning({ id: schema.bookSeries.id }); + return { id: series!.id, name }; +} + +async function seedBook( + ctx: AuthorizationMatrixE2EContext, + library: CreatedLibrary, + options: { title: string; authorIds: number[]; series?: SeededSeries; seriesIndex?: string }, +): Promise { + const slug = `${options.title.toLowerCase().replaceAll(' ', '-')}-${randomUUID()}`; + + const [book] = await ctx.db + .insert(schema.books) + .values({ + libraryId: library.libraryId, + libraryFolderId: library.libraryFolderId, + folderPath: `${library.folderPath}/${slug}`, + status: 'present', + }) + .returning({ id: schema.books.id }); + const bookId = book!.id; + + await ctx.db.insert(schema.bookMetadata).values({ + bookId, + title: options.title, + seriesId: options.series?.id ?? null, + seriesName: options.series?.name ?? null, + seriesIndex: options.seriesIndex ?? null, + }); + + inoSequence += 1; + await ctx.db.insert(schema.bookFiles).values({ + bookId, + libraryFolderId: library.libraryFolderId, + absolutePath: `${library.folderPath}/${slug}/${slug}.epub`, + relPath: `${slug}/${slug}.epub`, + ino: BigInt(inoSequence), + sizeBytes: 2048, + format: 'epub', + role: 'content', + }); + + for (const [displayOrder, authorId] of options.authorIds.entries()) { + await ctx.db.insert(schema.bookAuthors).values({ bookId, authorId, displayOrder }); + } + + if (options.series) { + await ctx.db.insert(schema.bookSeriesMemberships).values({ + bookId, + seriesId: options.series.id, + seriesIndex: options.seriesIndex ?? null, + }); + } + + return bookId; +} + +/** + * Exercises the collapsed author listing against a real database, which is the only place the + * correlated EXISTS that carries authorship into the collapsed query actually runs. + */ +describe('Author books collapsed by series (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () => { + let ctx!: AuthorizationMatrixE2EContext; + let reader!: TestUserSession; + let prolificAuthorId!: number; + let otherAuthorId!: number; + let standaloneBookId!: number; + + async function listBooks(authorId: number, query = '') { + return ctx.app.inject({ + method: 'GET', + url: `/api/v1/authors/${authorId}/books${query}`, + headers: authHeader(reader.accessToken), + }); + } + + beforeAll(async () => { + ctx = await createAuthorizationMatrixE2EContext(); + + const library = await createLibraryWithFolder(ctx, { name: `author-collapse-${randomUUID()}` }); + reader = await createUserAndLogin(ctx); + await grantLibraryAccess(ctx, reader.userId, library.libraryId); + + prolificAuthorId = await createAuthor(ctx, 'Prolific Writer'); + otherAuthorId = await createAuthor(ctx, 'Other Writer'); + + const discworld = await createSeries(ctx, 'Discworld'); + const longEarth = await createSeries(ctx, 'The Long Earth'); + + // Five books over two series plus one standalone: seven rows flat, three collapsed. + await seedBook(ctx, library, { title: 'Guards Guards', authorIds: [prolificAuthorId], series: discworld, seriesIndex: '1' }); + await seedBook(ctx, library, { title: 'Men At Arms', authorIds: [prolificAuthorId], series: discworld, seriesIndex: '2' }); + await seedBook(ctx, library, { title: 'Feet Of Clay', authorIds: [prolificAuthorId], series: discworld, seriesIndex: '3' }); + await seedBook(ctx, library, { title: 'The Long Earth', authorIds: [prolificAuthorId], series: longEarth, seriesIndex: '1' }); + await seedBook(ctx, library, { title: 'The Long War', authorIds: [prolificAuthorId], series: longEarth, seriesIndex: '2' }); + standaloneBookId = await seedBook(ctx, library, { title: 'Nation', authorIds: [prolificAuthorId] }); + + // Same series, a different author: it must not be counted into the prolific author's card. + await seedBook(ctx, library, { title: 'Foreign Volume', authorIds: [otherAuthorId], series: discworld, seriesIndex: '4' }); + }); + + afterAll(async () => { + await closeAuthorizationMatrixE2EContext(ctx); + }); + + it('lists every book when collapsing is off', async () => { + const response = await listBooks(prolificAuthorId); + + expect(response.statusCode).toBe(200); + const body = response.json() as { items: BookCard[]; total: number; bookTotal: number }; + expect(body.items).toHaveLength(6); + expect(body.total).toBe(6); + // Flat, so the two counts say the same thing. + expect(body.bookTotal).toBe(6); + expect(body.items.every((item) => item.collapsedSeries === undefined)).toBe(true); + }); + + it('folds each series into one card and leaves the standalone book alone', async () => { + const response = await listBooks(prolificAuthorId, '?collapseSeries=true'); + + expect(response.statusCode).toBe(200); + const body = response.json() as { items: BookCard[]; total: number; bookTotal: number }; + + expect(body.items).toHaveLength(3); + const collapsed = body.items.filter((item) => item.collapsedSeries); + expect(collapsed).toHaveLength(2); + expect(collapsed.map((item) => item.collapsedSeries!.bookCount).sort()).toEqual([2, 3]); + + const standalone = body.items.find((item) => item.id === standaloneBookId); + expect(standalone).toBeDefined(); + expect(standalone!.collapsedSeries).toBeUndefined(); + }); + + it('counts rows in total and books in bookTotal', async () => { + const response = await listBooks(prolificAuthorId, '?collapseSeries=true'); + + const body = response.json() as { total: number; bookTotal: number }; + expect(body.total).toBe(3); + expect(body.bookTotal).toBe(6); + }); + + it('counts only this author in a series shared with another writer', async () => { + const response = await listBooks(prolificAuthorId, '?collapseSeries=true'); + const body = response.json() as { items: BookCard[] }; + + const discworld = body.items.find((item) => item.seriesName === 'Discworld'); + expect(discworld).toBeDefined(); + // The fourth Discworld volume belongs to the other writer, so it stays out of this count. + expect(discworld!.collapsedSeries!.bookCount).toBe(3); + }); + + it('keeps another author to their own books', async () => { + const response = await listBooks(otherAuthorId, '?collapseSeries=true'); + + const body = response.json() as { items: BookCard[]; total: number; bookTotal: number }; + expect(body.total).toBe(1); + expect(body.bookTotal).toBe(1); + expect(body.items[0]!.collapsedSeries!.bookCount).toBe(1); + }); + + it('pages over rows, so a page holds collapsed cards rather than books', async () => { + const first = await listBooks(prolificAuthorId, '?collapseSeries=true&size=2&page=0'); + const second = await listBooks(prolificAuthorId, '?collapseSeries=true&size=2&page=1'); + + const firstBody = first.json() as { items: BookCard[]; total: number }; + const secondBody = second.json() as { items: BookCard[]; total: number }; + + expect(firstBody.items).toHaveLength(2); + expect(secondBody.items).toHaveLength(1); + expect(firstBody.total).toBe(3); + // Walking the pages must reach the end of the list rather than looping on a book count. + expect(firstBody.items.length + secondBody.items.length).toBe(firstBody.total); + }); + + it('preserves both totals when the requested page is beyond the last row', async () => { + const response = await listBooks(prolificAuthorId, '?collapseSeries=true&size=2&page=20'); + + expect(response.statusCode).toBe(200); + const body = response.json() as { items: BookCard[]; total: number; bookTotal: number }; + expect(body.items).toEqual([]); + expect(body.total).toBe(3); + expect(body.bookTotal).toBe(6); + }); +});