From 0b89e5a4a1ef3070540a989162aac982c8559db4 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Tue, 27 Oct 2026 03:55:23 +0000 Subject: [PATCH] fix(series): count duplicate formats as one volume Preserve raw book counts for compatibility while reporting logical ownership and reading progress for duplicate series positions. Closes #1871 --- .../series/views/SeriesDetailView.spec.ts | 31 +++++++- .../series/views/SeriesDetailView.vue | 8 +- packages/types/src/series.ts | 4 + .../modules/series/series.repository.test.ts | 16 +++- .../src/modules/series/series.repository.ts | 27 ++++--- .../src/modules/series/series.service.test.ts | 31 ++++++++ server/src/modules/series/series.service.ts | 8 +- .../series/utils/series-ladder.utils.test.ts | 47 +++++++++++- .../series/utils/series-ladder.utils.ts | 75 ++++++++++++++----- 9 files changed, 210 insertions(+), 37 deletions(-) diff --git a/client/src/features/series/views/SeriesDetailView.spec.ts b/client/src/features/series/views/SeriesDetailView.spec.ts index aa4f26eeb..25f92012e 100644 --- a/client/src/features/series/views/SeriesDetailView.spec.ts +++ b/client/src/features/series/views/SeriesDetailView.spec.ts @@ -3,6 +3,7 @@ import { computed, defineComponent, nextTick, reactive, ref, type PropType } fro import { flushPromises, mount } from '@vue/test-utils' import type { BookCard, SeriesBooksPage, SeriesDetail } from '@bookorbit/types' import SeriesDetailView from './SeriesDetailView.vue' +import SeriesCompletionBar from '../components/SeriesCompletionBar.vue' const GROUP_BY_MEDIA_STORAGE_KEY = 'bookorbit:series-detail:group-by-media' @@ -319,7 +320,7 @@ describe('SeriesDetailView', () => { expect(wrapper.find('[data-testid="series-media-group-books"]').exists()).toBe(true) }) - it('shows the ownership bar only once a provider has supplied a series total', async () => { + it('falls back to book counts when a server does not return volume counts', async () => { mocks.seriesInfo = ref(makeSeriesInfo({ bookCount: 4, expectedBookCount: 7 })) const wrapper = mountView() @@ -328,9 +329,37 @@ describe('SeriesDetailView', () => { const bar = wrapper.get('[role="progressbar"]') expect(bar.attributes('aria-valuenow')).toBe('4') expect(bar.attributes('aria-valuemax')).toBe('7') + expect(wrapper.text()).toContain('4 volumes') + const completion = wrapper.getComponent(SeriesCompletionBar) + expect(completion.props('readCount')).toBe(0) + expect(completion.props('totalCount')).toBe(4) expect(wrapper.text()).toContain('4 of 7') }) + it('uses logical volume counts for the header, reading progress, and ownership', async () => { + mocks.seriesInfo = ref( + makeSeriesInfo({ + bookCount: 4, + readCount: 2, + ownedVolumeCount: 2, + readVolumeCount: 1, + expectedBookCount: 7, + }), + ) + + const wrapper = mountView() + await nextTick() + + expect(wrapper.text()).toContain('2 volumes') + const completion = wrapper.getComponent(SeriesCompletionBar) + expect(completion.props('readCount')).toBe(1) + expect(completion.props('totalCount')).toBe(2) + const ownership = wrapper.get('[role="progressbar"]') + expect(ownership.attributes('aria-valuenow')).toBe('2') + expect(ownership.attributes('aria-valuemax')).toBe('7') + expect(wrapper.text()).toContain('2 of 7') + }) + it('hides the ownership bar when no provider total is known', async () => { mocks.seriesInfo = ref(makeSeriesInfo({ bookCount: 4, expectedBookCount: null })) diff --git a/client/src/features/series/views/SeriesDetailView.vue b/client/src/features/series/views/SeriesDetailView.vue index 4d94a10da..c522d0b36 100644 --- a/client/src/features/series/views/SeriesDetailView.vue +++ b/client/src/features/series/views/SeriesDetailView.vue @@ -110,6 +110,8 @@ const canEditMetadata = computed(() => hasPermissionInAnyLibrary('library_edit_m const safeLeadDescription = useSafeHtml(() => leadBook.value?.description) const visibleSeriesAuthors = computed(() => (seriesInfo.value?.authors ?? []).slice(0, 5)) const hiddenSeriesAuthorsCount = computed(() => Math.max(0, (seriesInfo.value?.authors.length ?? 0) - visibleSeriesAuthors.value.length)) +const ownedVolumeCount = computed(() => seriesInfo.value?.ownedVolumeCount ?? seriesInfo.value?.bookCount ?? 0) +const readVolumeCount = computed(() => seriesInfo.value?.readVolumeCount ?? seriesInfo.value?.readCount ?? 0) const addToCollectionOpen = ref(false) const addToCollectionBookId = ref(null) const leadFallbackStyle = computed(() => { @@ -538,7 +540,7 @@ defineOptions({ name: 'SeriesDetailView' })

{{ seriesInfo.name }}

- {{ t('series.detail.bookCount', { count: seriesInfo.bookCount }) }} + {{ t('series.card.volumeCount', { count: ownedVolumeCount }) }} {{ t('series.detail.byAuthors', { authors: visibleSeriesAuthors.join(', ') }) }} {{ t('series.detail.moreAuthors', { count: hiddenSeriesAuthorsCount }) }} @@ -558,10 +560,10 @@ defineOptions({ name: 'SeriesDetailView' })
- +
diff --git a/packages/types/src/series.ts b/packages/types/src/series.ts index 9a6a8fa2a..7fb9b3f5d 100644 --- a/packages/types/src/series.ts +++ b/packages/types/src/series.ts @@ -71,6 +71,10 @@ export type SeriesDetail = { name: string; bookCount: number; readCount: number; + /** Logical volumes owned, collapsing book records that share a whole-numbered series position. */ + ownedVolumeCount?: number; + /** Owned logical volumes whose furthest status is read. */ + readVolumeCount?: number; authors: string[]; possibleGaps: number[]; /** Total books a metadata provider reports for the series, or null when no provider has told us. */ diff --git a/server/src/modules/series/series.repository.test.ts b/server/src/modules/series/series.repository.test.ts index 060599a99..cd260db97 100644 --- a/server/src/modules/series/series.repository.test.ts +++ b/server/src/modules/series/series.repository.test.ts @@ -275,8 +275,12 @@ describe('SeriesRepository', () => { it('returns detail row with authors and indices when series is found', async () => { const mainChain = makeChain([{ id: 42, name: 'Dune', bookCount: 6, readCount: 2 }]); - const indicesChain = makeChain([{ idx: 1 }, { idx: 2 }, { idx: 3 }]); - db.select.mockReturnValueOnce(mainChain).mockReturnValueOnce(indicesChain); + const membersChain = makeChain([ + { seriesIndex: '1', status: 'read' }, + { seriesIndex: '2', status: 'reading' }, + { seriesIndex: null, status: null }, + ]); + db.select.mockReturnValueOnce(mainChain).mockReturnValueOnce(membersChain); const authorsMap = new Map([[42, ['Frank Herbert']]]); vi.spyOn(repo as never, 'fetchAuthorsForSeries').mockResolvedValue(authorsMap); @@ -289,7 +293,12 @@ describe('SeriesRepository', () => { expect(result!.bookCount).toBe(6); expect(result!.readCount).toBe(2); expect(result!.authors).toEqual(['Frank Herbert']); - expect(result!.indices).toEqual([1, 2, 3]); + expect(result!.indices).toEqual(['1', '2']); + expect(result!.members).toEqual([ + { seriesIndex: '1', status: 'read' }, + { seriesIndex: '2', status: 'reading' }, + { seriesIndex: null, status: null }, + ]); }); it('returns empty authors when map has no entry for seriesId', async () => { @@ -303,6 +312,7 @@ describe('SeriesRepository', () => { expect(result!.authors).toEqual([]); expect(result!.indices).toEqual([]); + expect(result!.members).toEqual([]); }); }); diff --git a/server/src/modules/series/series.repository.ts b/server/src/modules/series/series.repository.ts index 24fbce8c8..4952d1a63 100644 --- a/server/src/modules/series/series.repository.ts +++ b/server/src/modules/series/series.repository.ts @@ -30,14 +30,17 @@ type SeriesSummaryRow = { libraryNames: string[]; }; -/** One book of a series, as the list needs it to draw the volume ladder. */ -export type SeriesMemberRow = { - bookId: number; +export type SeriesVolumeMember = { seriesIndex: string | null; - title: string | null; status: string | null; }; +/** One book of a series, as the list needs it to draw the volume ladder. */ +export type SeriesMemberRow = SeriesVolumeMember & { + bookId: number; + title: string | null; +}; + export type SeriesFacetRow = { all: number; notStarted: number; @@ -69,6 +72,7 @@ type SeriesDetailRow = { expectedBookCount: number | null; authors: string[]; indices: string[]; + members: SeriesVolumeMember[]; }; @Injectable() @@ -281,18 +285,20 @@ export class SeriesRepository { const row = rows[0]; - const [authorsMap, indicesRows] = await Promise.all([ + const [authorsMap, memberRows] = await Promise.all([ this.fetchAuthorsForSeries([params.seriesId], params.libraryIds, params.contentFilters), this.db - .select({ idx: bookSeriesMemberships.seriesIndex }) + .select({ + seriesIndex: bookSeriesMemberships.seriesIndex, + status: userBookStatus.status, + }) .from(books) .innerJoin(bookSeriesMemberships, eq(bookSeriesMemberships.bookId, books.id)) - .where( - and(eq(bookSeriesMemberships.seriesId, params.seriesId), libraryFilter, ...filterClauses, isNotNull(bookSeriesMemberships.seriesIndex)), - ), + .leftJoin(userBookStatus, and(eq(userBookStatus.bookId, books.id), eq(userBookStatus.userId, params.userId))) + .where(and(eq(bookSeriesMemberships.seriesId, params.seriesId), libraryFilter, ...filterClauses)), ]); - const indices = indicesRows.map((r) => r.idx!); + const indices = memberRows.map((row) => row.seriesIndex).filter((index): index is string => index !== null); return { id: row.id, @@ -302,6 +308,7 @@ export class SeriesRepository { expectedBookCount: row.expectedBookCount ?? null, authors: authorsMap.get(params.seriesId) ?? [], indices, + members: memberRows, }; } diff --git a/server/src/modules/series/series.service.test.ts b/server/src/modules/series/series.service.test.ts index 186dd9db3..0c80fba57 100644 --- a/server/src/modules/series/series.service.test.ts +++ b/server/src/modules/series/series.service.test.ts @@ -339,6 +339,35 @@ describe('SeriesService', () => { expect(result.total).toBe(3); }); + it('reports logical volume counts without changing raw book counts', async () => { + seriesRepo.findDetail.mockResolvedValue({ + id: 42, + name: 'Two formats', + bookCount: 4, + readCount: 2, + authors: [], + indices: ['1', '1', '2', '2'], + expectedBookCount: 3, + members: [ + { seriesIndex: '1', status: null }, + { seriesIndex: '1', status: 'read' }, + { seriesIndex: '2', status: 'reading' }, + { seriesIndex: '2', status: 'read' }, + ], + }); + seriesRepo.findBookIds.mockResolvedValue({ bookIds: [], total: 4 }); + + const result = await service.findBooks(reqUser(), 42, {}); + + expect(result.seriesInfo).toMatchObject({ + bookCount: 4, + readCount: 2, + ownedVolumeCount: 2, + readVolumeCount: 2, + possibleGaps: [3], + }); + }); + it('preserves book order from repository', async () => { seriesRepo.findDetail.mockResolvedValue({ id: 42, name: 'Test', bookCount: 2, readCount: 0, authors: [], indices: ['1', '2'] }); seriesRepo.findBookIds.mockResolvedValue({ bookIds: [20, 10], total: 2 }); @@ -453,6 +482,8 @@ describe('SeriesService', () => { expect(result.seriesInfo.authors).toEqual(['Frank Herbert']); expect(result.seriesInfo.possibleGaps).toEqual([]); expect(result.seriesInfo.bookCount).toBe(0); + expect(result.seriesInfo.ownedVolumeCount).toBe(0); + expect(result.seriesInfo.readVolumeCount).toBe(0); }); it('throws 404 when series does not exist in any library', async () => { diff --git a/server/src/modules/series/series.service.ts b/server/src/modules/series/series.service.ts index 59ef1470f..5ef8eb1fb 100644 --- a/server/src/modules/series/series.service.ts +++ b/server/src/modules/series/series.service.ts @@ -14,7 +14,7 @@ import { ListSeriesBooksDto } from './dto/list-series-books.dto'; import { ListSeriesDto } from './dto/list-series.dto'; import { SeriesRepository } from './series.repository'; import { computeSeriesGaps } from './utils/series-gaps.utils'; -import { buildVolumeLadder } from './utils/series-ladder.utils'; +import { buildVolumeLadder, countSeriesVolumes } from './utils/series-ladder.utils'; @Injectable() export class SeriesService { @@ -138,6 +138,8 @@ export class SeriesService { name: existsInAnyLibrary.name, bookCount: 0, readCount: 0, + ownedVolumeCount: 0, + readVolumeCount: 0, authors: existsInAnyLibrary.authors, possibleGaps: [], expectedBookCount: existsInAnyLibrary.expectedBookCount ?? null, @@ -149,6 +151,9 @@ export class SeriesService { } const possibleGaps = computeSeriesGaps(detail.indices, detail.bookCount, detail.expectedBookCount); + const volumeCounts = detail.members + ? countSeriesVolumes({ members: detail.members, bookCount: detail.bookCount, expectedBookCount: detail.expectedBookCount }) + : { ownedVolumeCount: detail.bookCount, readVolumeCount: detail.readCount }; let items: BooksPage['items'] = []; if (bookPage.bookIds.length > 0) { @@ -186,6 +191,7 @@ export class SeriesService { name: detail.name, bookCount: detail.bookCount, readCount: detail.readCount, + ...volumeCounts, authors: detail.authors, possibleGaps, expectedBookCount: detail.expectedBookCount ?? null, diff --git a/server/src/modules/series/utils/series-ladder.utils.test.ts b/server/src/modules/series/utils/series-ladder.utils.test.ts index bce5134f2..537ae52f6 100644 --- a/server/src/modules/series/utils/series-ladder.utils.test.ts +++ b/server/src/modules/series/utils/series-ladder.utils.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest'; import type { SeriesMemberRow } from '../series.repository'; import { computeSeriesGaps, resolveLadderRange } from './series-gaps.utils'; -import { buildVolumeLadder } from './series-ladder.utils'; +import { buildVolumeLadder, countSeriesVolumes } from './series-ladder.utils'; function member(bookId: number, seriesIndex: string | null, status: string | null = null, title = `Book ${bookId}`): SeriesMemberRow { return { bookId, seriesIndex, title, status }; @@ -122,3 +122,48 @@ describe('buildVolumeLadder', () => { }); }); }); + +describe('countSeriesVolumes', () => { + it('counts ebook and audiobook copies at the same indexes once', () => { + const members = [member(1, '1'), member(2, '1', 'read'), member(3, '2', 'reading'), member(4, '2')]; + + expect(countSeriesVolumes({ members, bookCount: 4, expectedBookCount: 7 })).toEqual({ + ownedVolumeCount: 2, + readVolumeCount: 1, + }); + }); + + it('uses the furthest status of every numbered volume', () => { + const members = [member(1, '1', 'reading'), member(2, '1', 'read'), member(3, '2'), member(4, '2', 'read'), member(5, '3', 'reading')]; + + expect(countSeriesVolumes({ members, bookCount: 5, expectedBookCount: null })).toEqual({ + ownedVolumeCount: 3, + readVolumeCount: 2, + }); + }); + + it('keeps unnumbered and fractional books independent', () => { + const members = [member(1, '1', 'read'), member(2, null, 'read'), member(3, null), member(4, '1.5', 'read'), member(5, '1.5')]; + + expect(countSeriesVolumes({ members, bookCount: 5, expectedBookCount: null })).toEqual({ + ownedVolumeCount: 5, + readVolumeCount: 3, + }); + }); + + it('matches the ladder fallback when a zero index invalidates its numbered range', () => { + const members = [member(1, '0', 'read'), member(2, '1'), member(3, '1', 'read')]; + + expect(countSeriesVolumes({ members, bookCount: 3, expectedBookCount: null })).toEqual({ + ownedVolumeCount: 3, + readVolumeCount: 2, + }); + }); + + it('returns zero counts for an empty series', () => { + expect(countSeriesVolumes({ members: [], bookCount: 0, expectedBookCount: null })).toEqual({ + ownedVolumeCount: 0, + readVolumeCount: 0, + }); + }); +}); diff --git a/server/src/modules/series/utils/series-ladder.utils.ts b/server/src/modules/series/utils/series-ladder.utils.ts index 3f844d8ff..2ebabd07a 100644 --- a/server/src/modules/series/utils/series-ladder.utils.ts +++ b/server/src/modules/series/utils/series-ladder.utils.ts @@ -1,7 +1,7 @@ import type { SeriesIndex, SeriesVolumeSlot, SeriesVolumeStatus } from '@bookorbit/types'; import { SERIES_VOLUME_SLOT_LIMIT } from '@bookorbit/types'; -import type { SeriesMemberRow } from '../series.repository'; +import type { SeriesMemberRow, SeriesVolumeMember } from '../series.repository'; import { computeSeriesGaps, resolveLadderRange } from './series-gaps.utils'; export type SeriesLadder = { @@ -11,6 +11,11 @@ export type SeriesLadder = { next: { bookId: number; index: SeriesIndex | null; title: string | null } | null; }; +export type SeriesVolumeCounts = { + ownedVolumeCount: number; + readVolumeCount: number; +}; + const STATUS_RANK: Record = { read: 3, reading: 2, unread: 1, missing: 0 }; function memberStatus(status: string | null): SeriesVolumeStatus { @@ -25,6 +30,56 @@ function integerIndexOf(value: string | null): number | null { return Number.isSafeInteger(parsed) ? parsed : null; } +function groupSeriesMembers(params: { + members: T[]; + bookCount: number; + expectedBookCount: number | null; +}): { + gaps: number[]; + range: { from: number; to: number } | undefined; + byIndex: Map; + unnumbered: T[]; +} { + const { members, bookCount, expectedBookCount } = params; + const indices = members.map((member) => member.seriesIndex).filter((index): index is string => index !== null); + const gaps = computeSeriesGaps(indices, bookCount, expectedBookCount); + const range = resolveLadderRange(indices, bookCount, expectedBookCount); + const byIndex = new Map(); + const unnumbered: T[] = []; + + for (const member of members) { + const index = integerIndexOf(member.seriesIndex); + if (index === null || !range || index < range.from || index > range.to) { + unnumbered.push(member); + continue; + } + const held = byIndex.get(index); + if (!held || STATUS_RANK[memberStatus(member.status)] > STATUS_RANK[memberStatus(held.status)]) { + byIndex.set(index, member); + } + } + + return { gaps, range, byIndex, unnumbered }; +} + +/** + * Counts logical volumes rather than book records. Numbered copies share one volume and keep the + * furthest reading state, while books without a usable whole-numbered position stay independent. + */ +export function countSeriesVolumes(params: { + members: SeriesVolumeMember[]; + bookCount: number; + expectedBookCount: number | null; +}): SeriesVolumeCounts { + const grouped = groupSeriesMembers(params); + const owned = [...grouped.byIndex.values(), ...grouped.unnumbered]; + + return { + ownedVolumeCount: owned.length, + readVolumeCount: owned.filter((member) => memberStatus(member.status) === 'read').length, + }; +} + /** * The volume ladder a list row draws: one slot per number the series should have, plus the books * that carry no usable number, at the end. @@ -51,23 +106,7 @@ export function buildVolumeLadder(params: { return { volumes: [], truncated: true, gaps: [], next: firstOpenMember(members) }; } - const indices = members.map((m) => m.seriesIndex).filter((idx): idx is string => idx !== null); - const gaps = computeSeriesGaps(indices, bookCount, expectedBookCount); - const range = resolveLadderRange(indices, bookCount, expectedBookCount); - - const byIndex = new Map(); - const unnumbered: SeriesMemberRow[] = []; - for (const member of members) { - const index = integerIndexOf(member.seriesIndex); - if (index === null || !range || index < range.from || index > range.to) { - unnumbered.push(member); - continue; - } - const held = byIndex.get(index); - if (!held || STATUS_RANK[memberStatus(member.status)] > STATUS_RANK[memberStatus(held.status)]) { - byIndex.set(index, member); - } - } + const { gaps, range, byIndex, unnumbered } = groupSeriesMembers({ members, bookCount, expectedBookCount }); const volumes: SeriesVolumeSlot[] = []; let truncated = false;