From 568614265f9ab697b9ab3b3918adba13925155a9 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:13:36 +0000 Subject: [PATCH] fix(books): list series stacks before standalones when sorting by series With series collapsed, the series sort used the same key as title, so stacks and standalone books interleaved alphabetically. Order stacks by series name first, then standalones by title, in both directions. The jump rail now only letters series, matching the flat view. Fixes #1456 --- .../book/book-query-builder.service.test.ts | 11 +++++++--- .../book/book-query-builder.service.ts | 5 +++++ server/src/modules/book/book.repository.ts | 7 ++++++- .../test/jump-buckets-invariant.e2e-spec.ts | 21 ++++++++++++++++++- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/server/src/modules/book/book-query-builder.service.test.ts b/server/src/modules/book/book-query-builder.service.test.ts index 5106c32f5..c6d87134e 100644 --- a/server/src/modules/book/book-query-builder.service.test.ts +++ b/server/src/modules/book/book-query-builder.service.test.ts @@ -1901,8 +1901,13 @@ describe('BookQueryBuilder.buildCollapseOrderBy', () => { expect(BookQueryBuilder.buildCollapseOrderBy([{ field: 'title', dir: 'desc' }], 1)).toBe('sort_title DESC NULLS LAST, r.id ASC'); }); - it('generates sort_title for series field', () => { - expect(BookQueryBuilder.buildCollapseOrderBy([{ field: 'series', dir: 'asc' }], 1)).toBe('sort_title ASC NULLS LAST, r.id ASC'); + it('orders series stacks before standalone books, then by title, in both directions', () => { + expect(BookQueryBuilder.buildCollapseOrderBy([{ field: 'series', dir: 'asc' }], 1)).toBe( + 'sort_series ASC NULLS LAST, sort_title ASC NULLS LAST, r.id ASC', + ); + expect(BookQueryBuilder.buildCollapseOrderBy([{ field: 'series', dir: 'desc' }], 1)).toBe( + 'sort_series DESC NULLS LAST, sort_title DESC NULLS LAST, r.id ASC', + ); }); it('generates sort_added_at for addedAt field', () => { @@ -1934,7 +1939,7 @@ describe('BookQueryBuilder.buildCollapseOrderBy', () => { 1, ); expect(result).toContain("split_part(series_index::text, '.', 1)::numeric"); - expect(result).toContain('series_index COLLATE "C" ASC NULLS LAST, sort_title ASC NULLS LAST, r.id ASC'); + expect(result).toContain('series_index COLLATE "C" ASC NULLS LAST, sort_series ASC NULLS LAST, sort_title ASC NULLS LAST, r.id ASC'); }); it('generates user-scoped subquery for readProgress', () => { diff --git a/server/src/modules/book/book-query-builder.service.ts b/server/src/modules/book/book-query-builder.service.ts index 60460e304..66b333164 100644 --- a/server/src/modules/book/book-query-builder.service.ts +++ b/server/src/modules/book/book-query-builder.service.ts @@ -1135,7 +1135,12 @@ export class BookQueryBuilder { } switch (field) { case 'title': + parts.push(`sort_title ${D} NULLS LAST`); + break; + // sort_series is NULL for standalone books, so series stacks lead in both directions + // and the standalones that follow are ordered by title. case 'series': + parts.push(`sort_series ${D} NULLS LAST`); parts.push(`sort_title ${D} NULLS LAST`); break; case 'addedAt': diff --git a/server/src/modules/book/book.repository.ts b/server/src/modules/book/book.repository.ts index 29cf60e24..7f594652e 100644 --- a/server/src/modules/book/book.repository.ts +++ b/server/src/modules/book/book.repository.ts @@ -98,6 +98,7 @@ type CollapsedRawRow = { publisher: string | null; page_count: number | null; sort_title: string | null; + sort_series: string | null; sort_added_at: string | null; book_count: string | null; read_count: string | null; @@ -239,8 +240,10 @@ function collapsedDiscreteSourceParts(field: SortField, userId: number): Collaps switch (field) { case 'title': - case 'series': return direct(sql.raw('r.sort_title')); + // Standalone books have no series letter, matching the flat rail. + case 'series': + return direct(sql.raw('r.sort_series')); case 'author': return direct(sql.raw('r.author_sort_name')); case 'publisher': @@ -1079,6 +1082,7 @@ export class BookRepository { base.metadata_score, base.primary_author_sort_name AS author_sort_name, COALESCE(base.norm_series, lower(base.title)) AS sort_title, + base.norm_series AS sort_series, COALESCE(sa.latest_added_at, base.added_at) AS sort_added_at, COALESCE(sa.first_collection_position, base.collection_position) AS sort_collection_position, sa.book_count, @@ -1311,6 +1315,7 @@ export class BookRepository { ubr.rating, base.primary_author_sort_name AS author_sort_name, COALESCE(base.norm_series, lower(base.title)) AS sort_title, + base.norm_series AS sort_series, COALESCE(sl.latest_added_at, base.added_at) AS sort_added_at ${source.representativeExtraSelect} FROM base_rows base diff --git a/server/test/jump-buckets-invariant.e2e-spec.ts b/server/test/jump-buckets-invariant.e2e-spec.ts index 8a34a2690..332ce9227 100644 --- a/server/test/jump-buckets-invariant.e2e-spec.ts +++ b/server/test/jump-buckets-invariant.e2e-spec.ts @@ -57,7 +57,7 @@ function expectedBucketKey( return String(Math.floor(book.publishedYear / step) * step); } if (field === 'author') return letterOf(book.authorSortName); - if (field === 'series') return letterOf(collapsed ? (book.seriesName ?? book.title) : book.seriesName); + if (field === 'series') return letterOf(book.seriesName); if (field === 'publisher') return letterOf(book.publisher); if (collapsed) return letterOf(book.seriesName ?? book.title); return letterOf(book.title); @@ -412,6 +412,25 @@ describe('Jump buckets invariant (e2e)', { timeout: SCENARIO_TIMEOUT_MS }, () => } }); + for (const dir of dirs) { + it(`lists collapsed series stacks before standalone books by title when sorting by series ${dir}`, async () => { + const [seriesListing, titleListing] = await Promise.all([ + fetchListing([{ field: 'series', dir }], true), + fetchListing([{ field: 'title', dir }], true), + ]); + const rows = seriesListing.items.map((item) => seededById.get((item as BookCard).id)!); + const firstStandalone = rows.findIndex((row) => row.seriesName === null); + + expect(rows.slice(0, firstStandalone).map((row) => row.seriesName)).toEqual( + dir === 'asc' ? ['Dune Saga', 'zeta chronicles'] : ['zeta chronicles', 'Dune Saga'], + ); + expect(rows.slice(firstStandalone).every((row) => row.seriesName === null)).toBe(true); + expect(rows.slice(firstStandalone).map((row) => row.id)).toEqual( + titleListing.items.map((item) => (item as BookCard).id).filter((id) => seededById.get(id)!.seriesName === null), + ); + }); + } + it('preserves every populated year when sparse years fit the bounded capacity', async () => { const response = await fetchBuckets([{ field: 'publishedYear', dir: 'asc' }], false); const expectedYears = [...new Set([...seededById.values()].flatMap((book) => (book.publishedYear === null ? [] : [String(book.publishedYear)])))];