fix: paginate collapsed series by visible rows

This commit is contained in:
Jonathan Finley 2026-03-13 22:46:14 -04:00
parent 168844e7a8
commit 272df76ac2
4 changed files with 229 additions and 52 deletions

View file

@ -638,6 +638,7 @@ class LibraryController {
limit: Number(payload.limit) || 0, limit: Number(payload.limit) || 0,
offset: Number(payload.offset) || 0 offset: Number(payload.offset) || 0
} }
payload.hideSingleBookSeries = !!req.library.settings.hideSingleBookSeries
const { libraryItems, count } = await libraryItemsBookFilters.getCollapsedSeriesWindow(req.library.id, seriesId, req.user, include, payload, collapsedSeriesWindow) const { libraryItems, count } = await libraryItemsBookFilters.getCollapsedSeriesWindow(req.library.id, seriesId, req.user, include, payload, collapsedSeriesWindow)
payload.total = count payload.total = count
payload.results = await libraryHelpers.toCollapsedSeriesPayload(libraryItems, seriesId, req.library.settings.hideSingleBookSeries) payload.results = await libraryHelpers.toCollapsedSeriesPayload(libraryItems, seriesId, req.library.settings.hideSingleBookSeries)

View file

@ -91,8 +91,12 @@ function getSeriesSequenceList(collapsedSeries) {
module.exports = { module.exports = {
async toCollapsedSeriesPayload(libraryItems, seriesId, hideSingleBookSeries = false) { async toCollapsedSeriesPayload(libraryItems, seriesId, hideSingleBookSeries = false) {
const collapsedItems = collapseBookSeries(libraryItems, seriesId, hideSingleBookSeries) const shapedItems = libraryItems.some((libraryItem) => libraryItem.collapsedSeries)
const shapedItems = !(collapsedItems.length === 1 && collapsedItems[0].collapsedSeries) ? collapsedItems : libraryItems ? libraryItems
: (() => {
const collapsedItems = collapseBookSeries(libraryItems, seriesId, hideSingleBookSeries)
return !(collapsedItems.length === 1 && collapsedItems[0].collapsedSeries) ? collapsedItems : libraryItems
})()
return Promise.all( return Promise.all(
shapedItems.map(async (libraryItem) => { shapedItems.map(async (libraryItem) => {

View file

@ -165,12 +165,88 @@ async function loadCollapsedSeriesWindow({ findOptions, countOptions, limit, off
limit: Number(limit) || null, limit: Number(limit) || null,
offset: Number(offset) || 0 offset: Number(offset) || 0
}), }),
Database.bookModel.count(countOptions) countOptions ? Database.bookModel.count(countOptions) : Promise.resolve(null)
]) ])
return { books, count } return { books, count }
} }
function mapCollapsedSeriesBookToLibraryItem(bookExpanded) {
const libraryItem = bookExpanded.libraryItem
const book = bookExpanded
delete book.libraryItem
book.series = book.series || []
delete book.bookSeries
book.authors = book.bookAuthors?.map((ba) => ba.author) || []
delete book.bookAuthors
if (libraryItem.feeds?.length) {
libraryItem.rssFeed = libraryItem.feeds[0]
}
libraryItem.media = book
return libraryItem
}
function buildCollapsedSeriesView(libraryItems, seriesId, hideSingleBookSeries = false) {
const seriesEntriesById = {}
libraryItems.forEach((libraryItem) => {
const filteredSeries = libraryItem.media.series.find((series) => series.id === seriesId)
const subseries = (libraryItem.media.series || []).filter((series) => series.id !== seriesId)
subseries.forEach((series) => {
if (!seriesEntriesById[series.id]) {
seriesEntriesById[series.id] = {
id: series.id,
name: series.name,
nameIgnorePrefix: series.nameIgnorePrefix,
books: []
}
}
seriesEntriesById[series.id].books.push({
id: libraryItem.id,
filterSeriesSequence: filteredSeries?.bookSeries?.sequence
})
})
})
let seriesEntries = Object.values(seriesEntriesById)
if (hideSingleBookSeries) {
seriesEntries = seriesEntries.filter((series) => series.books.length > 1)
}
seriesEntries.forEach((series) => {
seriesEntriesById[series.id] = series
})
const activeSeriesEntries = seriesEntries
const visibleRows = []
libraryItems.forEach((libraryItem) => {
const firstSeriesEntries = activeSeriesEntries.filter((series) => series.books[0]?.id === libraryItem.id)
firstSeriesEntries.forEach((series) => {
visibleRows.push({ type: 'collapsed', libraryItemId: libraryItem.id, seriesId: series.id })
})
const isCollapsedBook = activeSeriesEntries.some((series) => series.books.some((book) => book.id === libraryItem.id))
if (!isCollapsedBook) {
visibleRows.push({ type: 'plain', libraryItemId: libraryItem.id })
}
})
if (visibleRows.length === 1 && visibleRows[0].type === 'collapsed') {
return {
visibleRows: libraryItems.map((libraryItem) => ({ type: 'plain', libraryItemId: libraryItem.id })),
seriesEntriesById: {}
}
}
return { visibleRows, seriesEntriesById }
}
function getNextBrowseCursor(books, limit, sortBy, sortDesc, cursorKeys) { function getNextBrowseCursor(books, limit, sortBy, sortDesc, cursorKeys) {
const rowLimit = Number(limit) || 0 const rowLimit = Number(limit) || 0
if (!rowLimit || books.length <= rowLimit) return null if (!rowLimit || books.length <= rowLimit) return null
@ -973,62 +1049,52 @@ module.exports = {
subQuery: false subQuery: false
} }
const countOptions = { const batchLimit = Math.max(Number(window.limit) || 0, 1)
where: userPermissionBookWhere.bookWhere, const allLibraryItems = []
replacements: userPermissionBookWhere.replacements, let rawOffset = 0
include: [
{ while (true) {
model: Database.libraryItemModel, const { books } = await loadCollapsedSeriesWindow({
required: true, findOptions,
where: { countOptions: null,
libraryId limit: batchLimit,
}, offset: rawOffset
include: libraryItemIncludes })
},
{ if (!books.length) break
model: Database.bookSeriesModel,
required: true, allLibraryItems.push(
where: { ...books
seriesId .map((bookExpanded) => mapCollapsedSeriesBookToLibraryItem(bookExpanded))
} .filter((libraryItem) => user?.checkCanAccessLibraryItem ? user.checkCanAccessLibraryItem(libraryItem) : true)
} )
],
distinct: true, rawOffset += books.length
subQuery: false if (books.length < batchLimit) break
} }
const { books, count } = await loadCollapsedSeriesWindow({ const { visibleRows, seriesEntriesById } = buildCollapsedSeriesView(allLibraryItems, seriesId, payload.hideSingleBookSeries)
findOptions, const visibleOffset = Number(window.offset) || 0
countOptions, const visibleLimit = Number(window.limit) || 0
limit: window.limit, const selectedRows = visibleLimit ? visibleRows.slice(visibleOffset, visibleOffset + visibleLimit) : visibleRows
offset: window.offset const libraryItemsById = new Map(allLibraryItems.map((libraryItem) => [libraryItem.id, libraryItem]))
})
const libraryItems = books const libraryItems = selectedRows
.map((bookExpanded) => { .map((row) => {
const libraryItem = bookExpanded.libraryItem const libraryItem = libraryItemsById.get(row.libraryItemId)
const book = bookExpanded if (!libraryItem) return null
if (row.type === 'collapsed') {
delete book.libraryItem return Object.assign(Object.create(Object.getPrototypeOf(libraryItem)), libraryItem, {
collapsedSeries: seriesEntriesById[row.seriesId]
book.series = book.series || [] })
delete book.bookSeries
book.authors = book.bookAuthors?.map((ba) => ba.author) || []
delete book.bookAuthors
if (libraryItem.feeds?.length) {
libraryItem.rssFeed = libraryItem.feeds[0]
} }
libraryItem.media = book
return libraryItem return libraryItem
}) })
.filter((libraryItem) => user?.checkCanAccessLibraryItem ? user.checkCanAccessLibraryItem(libraryItem) : true) .filter(Boolean)
return { return {
libraryItems, libraryItems,
count count: visibleRows.length
} }
}, },

View file

@ -297,7 +297,7 @@ describe('LibraryController large-library browse contract', () => {
expect(findAllStub.firstCall.args[0].offset || 0).to.equal(0) expect(findAllStub.firstCall.args[0].offset || 0).to.equal(0)
}) })
it('applies the requested collapsed-series follow-up window in SQL query options', async () => { it('uses bounded batch query options for collapsed-series follow-up windows', async () => {
const findByPkStub = sinon.stub(Database.seriesModel, 'findByPk').resolves({ books: [] }) const findByPkStub = sinon.stub(Database.seriesModel, 'findByPk').resolves({ books: [] })
const findAllStub = sinon.stub(Database.bookModel, 'findAll').resolves([]) const findAllStub = sinon.stub(Database.bookModel, 'findAll').resolves([])
const req = { const req = {
@ -319,7 +319,7 @@ describe('LibraryController large-library browse contract', () => {
expect(findByPkStub.called).to.equal(false) expect(findByPkStub.called).to.equal(false)
expect(findAllStub.calledOnce).to.equal(true) expect(findAllStub.calledOnce).to.equal(true)
expect(findAllStub.firstCall.args[0].limit).to.equal(20) expect(findAllStub.firstCall.args[0].limit).to.equal(20)
expect(findAllStub.firstCall.args[0].offset).to.equal(20) expect(findAllStub.firstCall.args[0].offset).to.equal(0)
}) })
it('restores collapsed-series payload fields and preserves rssfeed serialization', async () => { it('restores collapsed-series payload fields and preserves rssfeed serialization', async () => {
@ -384,6 +384,112 @@ describe('LibraryController large-library browse contract', () => {
expect(findAllStub.firstCall.args[0].order[0][0].val || findAllStub.firstCall.args[0].order[0][0]).to.include('sequence') expect(findAllStub.firstCall.args[0].order[0][0].val || findAllStub.firstCall.args[0].order[0][0]).to.include('sequence')
}) })
it('paginates collapsed-series browse by visible rows instead of raw books', async () => {
const makeBook = ({ libraryItemId, filterSequence, subseriesId = null, subseriesName = null }) => ({
libraryItem: {
id: libraryItemId,
mediaType: 'book',
toOldJSONMinified() {
return { id: libraryItemId, media: { metadata: {}, duration: 1 } }
}
},
series: [
{ id: 'series_1', name: 'Main Series', nameIgnorePrefix: 'Main Series', bookSeries: { sequence: filterSequence } },
...(subseriesId ? [{ id: subseriesId, name: subseriesName, nameIgnorePrefix: subseriesName, bookSeries: { sequence: filterSequence } }] : [])
],
bookAuthors: []
})
const findAllStub = sinon.stub(Database.bookModel, 'findAll')
sinon.stub(Database.bookModel, 'count').resolves(3)
findAllStub.onCall(0).resolves([
makeBook({ libraryItemId: 'li_1', filterSequence: '1', subseriesId: 'sub_1', subseriesName: 'Subseries 1' })
])
findAllStub.onCall(1).resolves([
makeBook({ libraryItemId: 'li_2', filterSequence: '2', subseriesId: 'sub_1', subseriesName: 'Subseries 1' })
])
findAllStub.onCall(2).resolves([
makeBook({ libraryItemId: 'li_3', filterSequence: '3' })
])
findAllStub.onCall(3).resolves([])
const req = {
query: {
filter: `series.${Buffer.from('series_1').toString('base64')}`,
collapseseries: '1',
limit: '1',
page: '1',
sort: 'sequence'
},
library: { id: 'lib_1', mediaType: 'book', isVirtual: false, settings: {} },
user: { id: 'user_1', checkCanAccessLibraryItem: () => true }
}
const res = { json: sinon.spy() }
await LibraryController.getLibraryItems(req, res)
const response = res.json.firstCall.args[0]
expect(response.total).to.equal(2)
expect(response.results).to.have.length(1)
expect(response.results[0].id).to.equal('li_3')
expect(response.results[0].collapsedSeries).to.equal(undefined)
})
it('keeps a sub-series collapsed on the first visible page instead of splitting it across raw-book windows', async () => {
const makeBook = ({ libraryItemId, filterSequence, subseriesId = null, subseriesName = null }) => ({
libraryItem: {
id: libraryItemId,
mediaType: 'book',
toOldJSONMinified() {
return { id: libraryItemId, media: { metadata: {}, duration: 1 } }
}
},
series: [
{ id: 'series_1', name: 'Main Series', nameIgnorePrefix: 'Main Series', bookSeries: { sequence: filterSequence } },
...(subseriesId ? [{ id: subseriesId, name: subseriesName, nameIgnorePrefix: subseriesName, bookSeries: { sequence: filterSequence } }] : [])
],
bookAuthors: []
})
const findAllStub = sinon.stub(Database.bookModel, 'findAll')
sinon.stub(Database.bookModel, 'count').resolves(3)
findAllStub.onCall(0).resolves([
makeBook({ libraryItemId: 'li_1', filterSequence: '1', subseriesId: 'sub_1', subseriesName: 'Subseries 1' })
])
findAllStub.onCall(1).resolves([
makeBook({ libraryItemId: 'li_2', filterSequence: '2', subseriesId: 'sub_1', subseriesName: 'Subseries 1' })
])
findAllStub.onCall(2).resolves([
makeBook({ libraryItemId: 'li_3', filterSequence: '3' })
])
findAllStub.onCall(3).resolves([])
const req = {
query: {
filter: `series.${Buffer.from('series_1').toString('base64')}`,
collapseseries: '1',
limit: '1',
sort: 'sequence'
},
library: { id: 'lib_1', mediaType: 'book', isVirtual: false, settings: {} },
user: { id: 'user_1', checkCanAccessLibraryItem: () => true }
}
const res = { json: sinon.spy() }
await LibraryController.getLibraryItems(req, res)
const response = res.json.firstCall.args[0]
expect(response.total).to.equal(2)
expect(response.results).to.have.length(1)
expect(response.results[0].collapsedSeries).to.include({
id: 'sub_1',
name: 'Subseries 1',
numBooks: 2
})
expect(response.results[0].collapsedSeries.libraryItemIds).to.deep.equal(['li_1', 'li_2'])
expect(response.results[0].collapsedSeries.seriesSequenceList).to.equal('1-2')
})
it('adds a deterministic libraryItem.id tie-breaker for keyset title browse with duplicate titles', async () => { it('adds a deterministic libraryItem.id tie-breaker for keyset title browse with duplicate titles', async () => {
global.ServerSettings = { sortingIgnorePrefix: true } global.ServerSettings = { sortingIgnorePrefix: true }