feat: read lists books can be sorted by release date

read list are ordered by default, which is manual ordering
if manual ordering is disabled, books will be sorted by release date

Closes: #846
This commit is contained in:
Gauthier Roebroeck 2023-02-22 17:10:01 +08:00
parent 6583334970
commit e3bf9065a1
17 changed files with 281 additions and 30 deletions

View file

@ -150,7 +150,7 @@ export default Vue.extend({
} as ReadListCreationDto
try {
const created = await this.$komgaReadLists.postReadList(toCreate)
await this.$komgaReadLists.postReadList(toCreate)
this.dialogClose()
} catch (e) {
this.$eventHub.$emit(ERROR, {message: e.message} as ErrorEvent)

View file

@ -46,6 +46,17 @@
</v-col>
</v-row>
<v-row>
<v-col>
<div class="text-body-2">{{ $t('dialog.edit_readlist.label_ordering') }}</div>
<v-checkbox
v-model="form.ordered"
:label="$t('dialog.edit_readlist.field_manual_ordering')"
hide-details
/>
</v-col>
</v-row>
</v-container>
</v-card>
</v-tab-item>
@ -118,6 +129,7 @@ export default Vue.extend({
form: {
name: '',
summary: '',
ordered: true,
},
poster: {
selectedThumbnail: '',
@ -170,6 +182,7 @@ export default Vue.extend({
this.tab = 0
this.form.name = readList.name
this.form.summary = readList.summary
this.form.ordered = readList.ordered
this.poster.selectedThumbnail = ''
this.poster.deleteQueue = []
@ -216,6 +229,7 @@ export default Vue.extend({
const update = {
name: this.form.name,
summary: this.form.summary,
ordered: this.form.ordered,
} as ReadListUpdateDto
await this.$komgaReadLists.patchReadList(this.readList.id, update)

View file

@ -174,7 +174,8 @@
},
"browse_readlist": {
"edit_elements": "Edit elements",
"edit_readlist": "Edit read list"
"edit_readlist": "Edit read list",
"manual_ordering": "manual ordering"
},
"browse_series": {
"earliest_year_from_release_dates": "This is the earliest year from the release dates from all books in the series",
@ -434,7 +435,9 @@
"button_confirm": "Save changes",
"dialog_title": "Edit read list",
"field_name": "Name",
"field_manual_ordering": "Manual ordering",
"field_summary": "Summary",
"label_ordering": "By default, books in a read list are ordered manually. You can disable manual ordering to sort books by release date.",
"tab_general": "General",
"tab_poster": "Poster"
},

View file

@ -2,6 +2,7 @@ interface ReadListDto {
id: string,
name: string,
summary: string,
ordered: boolean,
filtered: boolean,
bookIds: string[],
createdDate: string,
@ -11,12 +12,14 @@ interface ReadListDto {
interface ReadListCreationDto {
name: string,
summary?: string,
ordered?: boolean,
bookIds: string[]
}
interface ReadListUpdateDto {
name?: string,
summary?: string,
ordered?: boolean,
bookIds?: string[]
}

View file

@ -11,6 +11,9 @@
<v-chip label class="mx-4">
<span style="font-size: 1.1rem">{{ readList.bookIds.length }}</span>
</v-chip>
<span v-if="readList.ordered"
class="font-italic text-overline"
>({{ $t('browse_readlist.manual_ordering') }})</span>
</v-toolbar-title>
<v-spacer/>
@ -131,10 +134,10 @@
<item-browser
:items.sync="books"
:item-context="[ItemContext.SHOW_SERIES]"
:item-context="itemContext"
:selected.sync="selectedBooks"
:edit-function="isAdmin ? editSingleBook : undefined"
:draggable="editElements"
:draggable="editElements && readList.ordered"
:deletable="editElements"
/>
@ -275,6 +278,10 @@ export default Vue.extend({
next()
},
computed: {
itemContext(): ItemContext[] {
if(this.readList?.ordered === false) return [ItemContext.SHOW_SERIES, ItemContext.RELEASE_DATE]
return [ItemContext.SHOW_SERIES]
},
paginationVisible(): number {
switch (this.$vuetify.breakpoint.name) {
case 'xs':

View file

@ -0,0 +1,2 @@
alter table READLIST
add column ORDERED boolean NOT NULL DEFAULT 1;

View file

@ -8,6 +8,10 @@ import java.util.SortedMap
data class ReadList(
val name: String,
val summary: String = "",
/**
* Indicates whether the read list is ordered manually
*/
val ordered: Boolean = true,
val bookIds: SortedMap<Int, String> = sortedMapOf(),

View file

@ -6,6 +6,9 @@ import java.time.LocalDateTime
data class SeriesCollection(
val name: String,
/**
* Indicates whether the collection is ordered manually
*/
val ordered: Boolean = false,
val seriesIds: List<String> = emptyList(),

View file

@ -2,6 +2,7 @@ package org.gotson.komga.infrastructure.jooq
import org.gotson.komga.domain.model.BookSearchWithReadProgress
import org.gotson.komga.domain.model.ContentRestrictions
import org.gotson.komga.domain.model.ReadList
import org.gotson.komga.domain.model.ReadStatus
import org.gotson.komga.infrastructure.datasource.SqliteUdfDataSource
import org.gotson.komga.infrastructure.search.LuceneEntity
@ -185,20 +186,20 @@ class BookDtoDao(
findSiblingSeries(bookId, userId, next = true)
override fun findPreviousInReadListOrNull(
readListId: String,
readList: ReadList,
bookId: String,
userId: String,
filterOnLibraryIds: Collection<String>?,
): BookDto? =
findSiblingReadList(readListId, bookId, userId, filterOnLibraryIds, next = false)
findSiblingReadList(readList, bookId, userId, filterOnLibraryIds, next = false)
override fun findNextInReadListOrNull(
readListId: String,
readList: ReadList,
bookId: String,
userId: String,
filterOnLibraryIds: Collection<String>?,
): BookDto? =
findSiblingReadList(readListId, bookId, userId, filterOnLibraryIds, next = true)
findSiblingReadList(readList, bookId, userId, filterOnLibraryIds, next = true)
override fun findAllOnDeck(userId: String, filterOnLibraryIds: Collection<String>?, pageable: Pageable, restrictions: ContentRestrictions): Page<BookDto> {
val seriesIds = dsl.select(s.ID)
@ -283,28 +284,52 @@ class BookDtoDao(
}
private fun findSiblingReadList(
readListId: String,
readList: ReadList,
bookId: String,
userId: String,
filterOnLibraryIds: Collection<String>?,
next: Boolean,
): BookDto? {
val numberSort = dsl.select(rlb.NUMBER)
.from(b)
.leftJoin(rlb).on(b.ID.eq(rlb.BOOK_ID))
.where(b.ID.eq(bookId))
.and(rlb.READLIST_ID.eq(readListId))
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.fetchOne(0, Int::class.java)
if (readList.ordered) {
val numberSort = dsl.select(rlb.NUMBER)
.from(b)
.leftJoin(rlb).on(b.ID.eq(rlb.BOOK_ID))
.where(b.ID.eq(bookId))
.and(rlb.READLIST_ID.eq(readList.id))
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.fetchOne(rlb.NUMBER)
return selectBase(userId, true)
.where(rlb.READLIST_ID.eq(readListId))
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.orderBy(rlb.NUMBER.let { if (next) it.asc() else it.desc() })
.seek(numberSort)
.limit(1)
.fetchAndMap()
.firstOrNull()
return selectBase(userId, true)
.where(rlb.READLIST_ID.eq(readList.id))
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.orderBy(rlb.NUMBER.let { if (next) it.asc() else it.desc() })
.seek(numberSort)
.limit(1)
.fetchAndMap()
.firstOrNull()
} else {
// it is too complex to perform a seek by release date as it could be null and could also have multiple occurrences of the same value
// instead we pull the whole list of ids, and perform the seek on the list
val bookIds = dsl.select(b.ID)
.from(b)
.leftJoin(rlb).on(b.ID.eq(rlb.BOOK_ID))
.leftJoin(d).on(b.ID.eq(d.BOOK_ID))
.where(rlb.READLIST_ID.eq(readList.id))
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.orderBy(d.RELEASE_DATE)
.fetch(b.ID)
val bookIndex = bookIds.indexOfFirst { it == bookId }
if (bookIndex == -1) return null
val siblingId = bookIds.getOrNull(bookIndex + if (next) 1 else -1) ?: return null
return selectBase(userId)
.where(b.ID.eq(siblingId))
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.limit(1)
.fetchAndMap()
.firstOrNull()
}
}
private fun selectBase(userId: String, selectReadListNumber: Boolean = false) =

View file

@ -157,6 +157,7 @@ class ReadListDao(
.set(rl.ID, readList.id)
.set(rl.NAME, readList.name)
.set(rl.SUMMARY, readList.summary)
.set(rl.ORDERED, readList.ordered)
.set(rl.BOOK_COUNT, readList.bookIds.size)
.execute()
@ -178,6 +179,7 @@ class ReadListDao(
dsl.update(rl)
.set(rl.NAME, readList.name)
.set(rl.SUMMARY, readList.summary)
.set(rl.ORDERED, readList.ordered)
.set(rl.BOOK_COUNT, readList.bookIds.size)
.set(rl.LAST_MODIFIED_DATE, LocalDateTime.now(ZoneId.of("Z")))
.where(rl.ID.eq(readList.id))
@ -233,6 +235,7 @@ class ReadListDao(
ReadList(
name = name,
summary = summary,
ordered = ordered,
bookIds = bookIds,
id = id,
createdDate = createdDate.toCurrentTimeZone(),

View file

@ -621,7 +621,10 @@ class OpdsController(
@Parameter(hidden = true) page: Pageable,
): OpdsFeed =
readListRepository.findByIdOrNull(id, principal.user.getAuthorizedLibraryIds(null))?.let { readList ->
val pageable = PageRequest.of(page.pageNumber, page.pageSize, Sort.by(Sort.Order.asc("readList.number")))
val sort =
if (readList.ordered) Sort.by(Sort.Order.asc("readList.number"))
else Sort.by(Sort.Order.asc("metadata.releaseDate"))
val pageable = PageRequest.of(page.pageNumber, page.pageSize, sort)
val bookSearch = BookSearchWithReadProgress(
mediaStatus = setOf(Media.Status.READY),

View file

@ -2,6 +2,7 @@ package org.gotson.komga.interfaces.api.persistence
import org.gotson.komga.domain.model.BookSearchWithReadProgress
import org.gotson.komga.domain.model.ContentRestrictions
import org.gotson.komga.domain.model.ReadList
import org.gotson.komga.interfaces.api.rest.dto.BookDto
import org.springframework.data.domain.Page
import org.springframework.data.domain.Pageable
@ -26,14 +27,14 @@ interface BookDtoRepository {
fun findNextInSeriesOrNull(bookId: String, userId: String): BookDto?
fun findPreviousInReadListOrNull(
readListId: String,
readList: ReadList,
bookId: String,
userId: String,
filterOnLibraryIds: Collection<String>?,
): BookDto?
fun findNextInReadListOrNull(
readListId: String,
readList: ReadList,
bookId: String,
userId: String,
filterOnLibraryIds: Collection<String>?,

View file

@ -232,6 +232,7 @@ class ReadListController(
ReadList(
name = readList.name,
summary = readList.summary,
ordered = readList.ordered,
bookIds = readList.bookIds.toIndexedMap(),
),
).toDto()
@ -258,6 +259,7 @@ class ReadListController(
val updated = existing.copy(
name = readList.name ?: existing.name,
summary = readList.summary ?: existing.summary,
ordered = readList.ordered ?: existing.ordered,
bookIds = readList.bookIds?.toIndexedMap() ?: existing.bookIds,
)
try {
@ -295,7 +297,9 @@ class ReadListController(
@Parameter(hidden = true) page: Pageable,
): Page<BookDto> =
readListRepository.findByIdOrNull(id, principal.user.getAuthorizedLibraryIds(null))?.let { readList ->
val sort = Sort.by(Sort.Order.asc("readList.number"))
val sort =
if (readList.ordered) Sort.by(Sort.Order.asc("readList.number"))
else Sort.by(Sort.Order.asc("metadata.releaseDate"))
val pageRequest =
if (unpaged) UnpagedSorted(sort)
@ -332,7 +336,7 @@ class ReadListController(
): BookDto =
readListRepository.findByIdOrNull(id, principal.user.getAuthorizedLibraryIds(null))?.let {
bookDtoRepository.findPreviousInReadListOrNull(
id,
it,
bookId,
principal.user.id,
principal.user.getAuthorizedLibraryIds(null),
@ -348,7 +352,7 @@ class ReadListController(
): BookDto =
readListRepository.findByIdOrNull(id, principal.user.getAuthorizedLibraryIds(null))?.let {
bookDtoRepository.findNextInReadListOrNull(
id,
it,
bookId,
principal.user.id,
principal.user.getAuthorizedLibraryIds(null),

View file

@ -7,6 +7,7 @@ import javax.validation.constraints.NotEmpty
data class ReadListCreationDto(
@get:NotBlank val name: String,
val summary: String = "",
val ordered: Boolean = true,
@get:NotEmpty @get:UniqueElements
val bookIds: List<String>,
)

View file

@ -9,6 +9,7 @@ data class ReadListDto(
val id: String,
val name: String,
val summary: String,
val ordered: Boolean,
val bookIds: List<String>,
@ -25,6 +26,7 @@ fun ReadList.toDto() =
id = id,
name = name,
summary = summary,
ordered = ordered,
bookIds = bookIds.values.toList(),
createdDate = createdDate.toUTC(),
lastModifiedDate = lastModifiedDate.toUTC(),

View file

@ -9,4 +9,5 @@ data class ReadListUpdateDto(
val summary: String?,
@get:NullOrNotEmpty @get:UniqueElements
val bookIds: List<String>?,
val ordered: Boolean?,
)

View file

@ -6,6 +6,7 @@ import org.gotson.komga.domain.model.ReadList
import org.gotson.komga.domain.model.makeBook
import org.gotson.komga.domain.model.makeLibrary
import org.gotson.komga.domain.model.makeSeries
import org.gotson.komga.domain.persistence.BookMetadataRepository
import org.gotson.komga.domain.persistence.LibraryRepository
import org.gotson.komga.domain.persistence.ReadListRepository
import org.gotson.komga.domain.persistence.SeriesMetadataRepository
@ -14,9 +15,11 @@ import org.gotson.komga.domain.service.ReadListLifecycle
import org.gotson.komga.domain.service.SeriesLifecycle
import org.gotson.komga.language.toIndexedMap
import org.hamcrest.Matchers
import org.hamcrest.Matchers.contains
import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeAll
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
@ -33,6 +36,7 @@ import org.springframework.test.web.servlet.post
import java.net.URLEncoder
import java.nio.charset.StandardCharsets
import java.nio.file.Files
import java.time.LocalDate
@ExtendWith(SpringExtension::class)
@SpringBootTest
@ -45,6 +49,7 @@ class ReadListControllerTest(
@Autowired private val libraryRepository: LibraryRepository,
@Autowired private val seriesLifecycle: SeriesLifecycle,
@Autowired private val seriesMetadataRepository: SeriesMetadataRepository,
@Autowired private val bookMetadataRepository: BookMetadataRepository,
) {
private val library1 = makeLibrary("Library1", id = "1")
@ -1011,4 +1016,174 @@ class ReadListControllerTest(
header { string("Content-Disposition", Matchers.containsString(URLEncoder.encode(name, StandardCharsets.UTF_8.name()))) }
}
}
@Nested
inner class Unordered {
private val library = makeLibrary("Library")
private val series = makeSeries("Series", library.id)
private lateinit var books: List<Book>
private lateinit var rlAllDiffDates: ReadList
private lateinit var rlAllNullDates: ReadList
private lateinit var rlAllBooks: ReadList
@BeforeAll
fun setup() {
libraryRepository.insert(library)
seriesLifecycle.createSeries(series)
books = (1..5).map { makeBook("Book_$it", libraryId = library.id, seriesId = series.id) }
seriesLifecycle.addBooks(series, books)
bookMetadataRepository.findById(books[0].id).let { bookMetadataRepository.update(it.copy(releaseDate = LocalDate.of(2020, 1, 1))) }
bookMetadataRepository.findById(books[1].id).let { bookMetadataRepository.update(it.copy(releaseDate = LocalDate.of(2020, 1, 1))) }
bookMetadataRepository.findById(books[2].id).let { bookMetadataRepository.update(it.copy(releaseDate = LocalDate.of(2021, 1, 1))) }
}
@BeforeEach
fun makeReadLists() {
rlAllDiffDates = readListLifecycle.addReadList(
ReadList(
name = "All different dates",
ordered = false,
bookIds = listOf(2, 1).map { books[it].id }.toIndexedMap(),
),
)
rlAllNullDates = readListLifecycle.addReadList(
ReadList(
name = "All null dates",
ordered = false,
bookIds = books.drop(3).map { it.id }.toIndexedMap(),
),
)
rlAllBooks = readListLifecycle.addReadList(
ReadList(
name = "All books",
ordered = false,
bookIds = books.map { it.id }.toIndexedMap(),
),
)
}
@Test
@WithMockCustomUser
fun `given unordered read lists when getting books then books are sorted by release date`() {
mockMvc.get("/api/v1/readlists/${rlAllDiffDates.id}/books")
.andExpect {
status { isOk() }
jsonPath("$.content.[*].['id']") { contains(listOf(1, 2).map { books[it].id }) }
}
mockMvc.get("/api/v1/readlists/${rlAllNullDates.id}/books")
.andExpect {
status { isOk() }
jsonPath("$.content.[*].['id']") { contains(listOf(3, 4).map { books[it].id }) }
}
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books")
.andExpect {
status { isOk() }
jsonPath("$.content.[*].['id']") { contains(listOf(3, 4, 0, 1, 2).map { books[it].id }) }
}
}
@Test
@WithMockCustomUser
fun `given unordered read lists when getting book siblings then it is returned according to release date sort or not found`() {
// rlAllDiffDates: 1, 2
// first book: id=1
mockMvc.get("/api/v1/readlists/${rlAllDiffDates.id}/books/${books[1].id}/previous")
.andExpect { status { isNotFound() } }
mockMvc.get("/api/v1/readlists/${rlAllDiffDates.id}/books/${books[1].id}/next")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[2].id) }
}
// second book: id=2
mockMvc.get("/api/v1/readlists/${rlAllDiffDates.id}/books/${books[2].id}/previous")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[1].id) }
}
mockMvc.get("/api/v1/readlists/${rlAllDiffDates.id}/books/${books[2].id}/next")
.andExpect { status { isNotFound() } }
// rlAllNullDates: 3, 4
// first book: id=3
mockMvc.get("/api/v1/readlists/${rlAllNullDates.id}/books/${books[3].id}/previous")
.andExpect { status { isNotFound() } }
mockMvc.get("/api/v1/readlists/${rlAllNullDates.id}/books/${books[3].id}/next")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[4].id) }
}
// second book: id=4
mockMvc.get("/api/v1/readlists/${rlAllNullDates.id}/books/${books[4].id}/previous")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[3].id) }
}
mockMvc.get("/api/v1/readlists/${rlAllNullDates.id}/books/${books[4].id}/next")
.andExpect { status { isNotFound() } }
// rlAllBooks: 3, 4, 0, 1, 2
// first book: id=3
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[3].id}/previous")
.andExpect { status { isNotFound() } }
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[3].id}/next")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[4].id) }
}
// second book: id=4
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[4].id}/previous")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[3].id) }
}
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[4].id}/next")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[0].id) }
}
// third book: id=0
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[0].id}/previous")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[4].id) }
}
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[0].id}/next")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[1].id) }
}
// fourth book: id=1
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[1].id}/previous")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[0].id) }
}
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[1].id}/next")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[2].id) }
}
// last book: id=2
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[2].id}/previous")
.andExpect {
status { isOk() }
jsonPath("$.id") { value(books[1].id) }
}
mockMvc.get("/api/v1/readlists/${rlAllBooks.id}/books/${books[2].id}/next")
.andExpect { status { isNotFound() } }
}
}
}