From 49e0b12e97a1207acba2df428111b0f32a5af995 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:38:45 +0000 Subject: [PATCH] fix(koreader): allow plugin API to reset read status to unread The catalog read-status enum omitted unread, so a device write could only be undone from the web dashboard. Add it to the settable enum and the plugin status dialog, and mark Unread as current when a book has no status row. Closes #853 --- .../bookorbit_catalog_detail.lua | 5 +- .../bookorbit_catalog_util.lua | 1 + .../bookorbit_catalog_status_options_test.lua | 145 ++++++++++++++++++ packages/types/src/koreader.ts | 6 +- .../dto/koreader-catalog-query.dto.test.ts | 29 +++- .../dto/koreader-catalog-query.dto.ts | 1 + .../koreader-catalog.controller.test.ts | 9 ++ .../koreader/koreader-catalog.service.test.ts | 11 ++ 8 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 koreader-plugin/spec/bookorbit_catalog_status_options_test.lua diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_detail.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_detail.lua index a2bbee477..be674197d 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_detail.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_detail.lua @@ -708,10 +708,13 @@ end function CatalogDetail:showSetStatusDialog(detail) local dialog local buttons = {} + -- A book with no status row comes back with readStatus nil, which the header + -- already renders as Unread. Fold it here too so the Unread row is marked. + local current = detail.readStatus or "unread" for _, status in ipairs(SETTABLE_READ_STATUSES) do table.insert(buttons, { { - text = status.text .. (detail.readStatus == status.id and " *" or ""), + text = status.text .. (current == status.id and " *" or ""), callback = function() UIManager:close(dialog) self:applyReadStatus(detail, status.id) diff --git a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua index f87e51c9d..6bd69c8a1 100644 --- a/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua +++ b/koreader-plugin/bookorbit.koplugin/bookorbit_catalog_util.lua @@ -84,6 +84,7 @@ CatalogUtil.READ_STATUS_LABELS = { -- Read statuses the user can set from the catalog detail page. Kept in sync -- with the server's manual read-status enum. CatalogUtil.SETTABLE_READ_STATUSES = { + { id = "unread", text = _("Unread") }, { id = "want_to_read", text = _("To read") }, { id = "reading", text = _("Reading") }, { id = "on_hold", text = _("On hold") }, diff --git a/koreader-plugin/spec/bookorbit_catalog_status_options_test.lua b/koreader-plugin/spec/bookorbit_catalog_status_options_test.lua new file mode 100644 index 000000000..375152b4e --- /dev/null +++ b/koreader-plugin/spec/bookorbit_catalog_status_options_test.lua @@ -0,0 +1,145 @@ +-- The read status dialog is the only place a device can undo its own status +-- write, so it has to offer Unread and mark it as current. A book with no +-- status row comes back from the catalog with readStatus nil, which the detail +-- header already renders as Unread, so the marker has to fold nil in too. + +package.path = "koreader-plugin/bookorbit.koplugin/?.lua;koreader-plugin/spec/?.lua;" .. package.path + +local function identity(value) return value end + +local function stubWidget() + local widget = {} + widget.__index = widget + function widget:new(opts) + opts = opts or {} + return setmetatable(opts, widget) + end + function widget:extend(opts) + local child = setmetatable(opts or {}, { __index = self }) + child.__index = child + child.new = widget.new + child.extend = widget.extend + return child + end + function widget:getSize() return { w = 0, h = 0 } end + return widget +end + +local last_dialog +local ButtonDialog = {} +ButtonDialog.__index = ButtonDialog +function ButtonDialog:new(opts) + last_dialog = setmetatable(opts or {}, ButtonDialog) + return last_dialog +end + +package.loaded["ui/bidi"] = { auto = identity, dirpath = identity, filepath = identity } +package.loaded["ffi/blitbuffer"] = { COLOR_GRAY = 0 } +package.loaded["ui/widget/button"] = stubWidget() +package.loaded["ui/widget/buttondialog"] = ButtonDialog +package.loaded["ui/widget/container/centercontainer"] = stubWidget() +package.loaded["ui/widget/container/leftcontainer"] = stubWidget() +package.loaded["ui/widget/container/inputcontainer"] = stubWidget() +package.loaded["ui/widget/horizontalgroup"] = stubWidget() +package.loaded["ui/widget/horizontalspan"] = stubWidget() +package.loaded["ui/widget/verticalgroup"] = stubWidget() +package.loaded["ui/widget/verticalspan"] = stubWidget() +package.loaded["ui/widget/linewidget"] = stubWidget() +package.loaded["ui/widget/textboxwidget"] = stubWidget() +package.loaded["ui/widget/textviewer"] = stubWidget() +package.loaded["ui/widget/infomessage"] = stubWidget() +package.loaded["ui/widget/keyvaluepage"] = stubWidget() +package.loaded["ui/geometry"] = stubWidget() +package.loaded["ui/gesturerange"] = stubWidget() +package.loaded["ui/font"] = { getFace = function() return { size = 12 } end } +package.loaded["ui/size"] = { line = { medium = 1 } } +package.loaded["ui/network/manager"] = { isConnected = function() return true end } +package.loaded["device"] = { screen = { scaleBySize = function(_, value) return value end } } +package.loaded["ui/uimanager"] = { show = function() end, close = function() end } +package.loaded["document/documentregistry"] = { hasProvider = function() return true end } +package.loaded["util"] = { + trim = function(value) return tostring(value or ""):match("^%s*(.-)%s*$") end, + fixUtf8 = function(value) return value end, + htmlToPlainTextIfHtml = identity, +} +package.loaded["ffi/util"] = { + template = function(pattern, a, b) + return (pattern:gsub("%%1", tostring(a)):gsub("%%2", tostring(b))) + end, +} +package.loaded["gettext"] = setmetatable({}, { __call = function(_, value) return value end }) +local widget_stub = stubWidget() +package.loaded["bookorbit_catalog_widgets"] = { + buildCoverWidget = function() return widget_stub:new() end, + buildDetailPill = function() return widget_stub:new() end, + buildDetailProgressBar = function() return widget_stub:new() end, + detailRatingStarWidth = function() return 20 end, + DetailRelatedCard = stubWidget(), + DetailTabButton = stubWidget(), + DetailRatingStar = stubWidget(), +} + +local CatalogUtil = require("bookorbit_catalog_util") +local CatalogDetail = require("bookorbit_catalog_detail") + +local function assertEqual(actual, expected, label) + if actual ~= expected then + error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual)), 2) + end +end + +-- Settable statuses + +local settable = {} +for index, status in ipairs(CatalogUtil.SETTABLE_READ_STATUSES) do + settable[status.id] = index + assertEqual(CatalogUtil.READ_STATUS_LABELS[status.id] ~= nil, true, + status.id .. " has a display label") +end + +assertEqual(settable.unread, 1, "the unread reset leads the list, matching the web dashboard order") +assertEqual(settable.want_to_read ~= nil, true, "want_to_read stays settable") +assertEqual(settable.reading ~= nil, true, "reading stays settable") +assertEqual(settable.on_hold ~= nil, true, "on_hold stays settable") +assertEqual(settable.read ~= nil, true, "read stays settable") +assertEqual(settable.abandoned ~= nil, true, "abandoned stays settable") +assertEqual(settable.rereading, nil, "rereading is server-derived, not device-settable") +assertEqual(settable.skimmed, nil, "skimmed is not offered on the device") +assertEqual(#CatalogUtil.SETTABLE_READ_STATUSES, 6, "no other status leaked into the dialog") + +-- Status dialog + +local applied = {} +local menu = setmetatable({}, { __index = CatalogDetail }) +function menu:applyReadStatus(detail, status) + table.insert(applied, { id = detail.id, status = status }) +end + +local function rowFor(label) + for _, row in ipairs(last_dialog.buttons) do + local button = row[1] + if button.text == label or button.text == label .. " *" then return button end + end +end + +local function isMarked(label) + local button = rowFor(label) + return button ~= nil and button.text == label .. " *" +end + +menu:showSetStatusDialog({ id = 1, readStatus = nil }) +assertEqual(rowFor("Unread") ~= nil, true, "the dialog offers the unread reset") +assertEqual(isMarked("Unread"), true, "a book with no status row shows Unread as current") +assertEqual(isMarked("Read"), false, "no other status is marked current") + +menu:showSetStatusDialog({ id = 2, readStatus = "read" }) +assertEqual(isMarked("Read"), true, "a finished book marks Read as current") +assertEqual(isMarked("Unread"), false, "a finished book does not also mark Unread") + +menu:showSetStatusDialog({ id = 3, readStatus = "reading" }) +rowFor("Unread").callback() +assertEqual(#applied, 1, "tapping the reset issues exactly one write") +assertEqual(applied[1].id, 3, "the reset targets the open book") +assertEqual(applied[1].status, "unread", "the reset sends the unread status the plugin API now accepts") + +print("bookorbit_catalog_status_options_test.lua: ok") diff --git a/packages/types/src/koreader.ts b/packages/types/src/koreader.ts index 9652539c9..7daef2b0c 100644 --- a/packages/types/src/koreader.ts +++ b/packages/types/src/koreader.ts @@ -148,8 +148,10 @@ export type KoreaderCatalogSortOrder = "asc" | "desc"; export type KoreaderCatalogReadStatusFilter = "unread" | "reading" | "finished"; // Read statuses the catalog detail page can set on a book. A subset of the -// full ReadStatus enum, chosen for reading-device ergonomics. -export type KoreaderCatalogSettableReadStatus = "want_to_read" | "reading" | "on_hold" | "read" | "abandoned"; +// full ReadStatus enum, chosen for reading-device ergonomics. "unread" is the +// default state, so it has to stay settable or a device write becomes a one-way +// door that only the web dashboard can undo. +export type KoreaderCatalogSettableReadStatus = "unread" | "want_to_read" | "reading" | "on_hold" | "read" | "abandoned"; export interface KoreaderCatalogSeriesSummary { total: number; diff --git a/server/src/modules/koreader/dto/koreader-catalog-query.dto.test.ts b/server/src/modules/koreader/dto/koreader-catalog-query.dto.test.ts index 03aa3d5cb..af246eeb5 100644 --- a/server/src/modules/koreader/dto/koreader-catalog-query.dto.test.ts +++ b/server/src/modules/koreader/dto/koreader-catalog-query.dto.test.ts @@ -3,7 +3,11 @@ import 'reflect-metadata'; import { plainToInstance } from 'class-transformer'; import { validate } from 'class-validator'; -import { KoreaderCatalogDashboardQueryDto } from './koreader-catalog-query.dto'; +import { + KOREADER_CATALOG_SETTABLE_READ_STATUSES, + KoreaderCatalogDashboardQueryDto, + KoreaderCatalogSetReadStatusDto, +} from './koreader-catalog-query.dto'; // The current plugin builds its dashboard shelves from the ordinary catalog // endpoints and no longer sends this parameter, but devices still on 1.4.x do, @@ -30,3 +34,26 @@ describe('KoreaderCatalogDashboardQueryDto', () => { await expect(validate(invalidId)).resolves.not.toHaveLength(0); }); }); + +describe('KoreaderCatalogSetReadStatusDto', () => { + // The plugin API is the only way a device can undo its own status write, so + // "unread" has to stay in this enum. Pinning the whole list makes a later + // trim fail here instead of stranding books on the device. + it('offers the reset back to unread alongside the other settable statuses', () => { + expect([...KOREADER_CATALOG_SETTABLE_READ_STATUSES]).toEqual(['unread', 'want_to_read', 'reading', 'on_hold', 'read', 'abandoned']); + }); + + it('accepts every settable status', async () => { + for (const status of KOREADER_CATALOG_SETTABLE_READ_STATUSES) { + const dto = plainToInstance(KoreaderCatalogSetReadStatusDto, { status }); + await expect(validate(dto), status).resolves.toHaveLength(0); + } + }); + + it('rejects statuses outside the settable enum and every empty reset', async () => { + for (const status of ['rereading', 'skimmed', 'Unread', 'none', '', null, undefined]) { + const dto = plainToInstance(KoreaderCatalogSetReadStatusDto, { status }); + await expect(validate(dto), String(status)).resolves.not.toHaveLength(0); + } + }); +}); diff --git a/server/src/modules/koreader/dto/koreader-catalog-query.dto.ts b/server/src/modules/koreader/dto/koreader-catalog-query.dto.ts index d97f10c7e..cfd960b79 100644 --- a/server/src/modules/koreader/dto/koreader-catalog-query.dto.ts +++ b/server/src/modules/koreader/dto/koreader-catalog-query.dto.ts @@ -25,6 +25,7 @@ export const KOREADER_CATALOG_SORT_ORDERS = ['asc', 'desc'] as const satisfies r export const KOREADER_CATALOG_READ_STATUS_FILTERS = ['unread', 'reading', 'finished'] as const satisfies readonly KoreaderCatalogReadStatusFilter[]; export const KOREADER_CATALOG_SETTABLE_READ_STATUSES = [ + 'unread', 'want_to_read', 'reading', 'on_hold', diff --git a/server/src/modules/koreader/koreader-catalog.controller.test.ts b/server/src/modules/koreader/koreader-catalog.controller.test.ts index 4ad4763cc..908ef95f7 100644 --- a/server/src/modules/koreader/koreader-catalog.controller.test.ts +++ b/server/src/modules/koreader/koreader-catalog.controller.test.ts @@ -60,6 +60,15 @@ describe('KoreaderCatalogController', () => { expect(catalogService.streamFile).toHaveBeenCalledWith(user, 100, reply); }); + it('forwards an unread reset rather than treating it as a missing status', async () => { + const { controller, catalogService } = makeController(); + const user = { id: 7 } as never; + catalogService.setReadStatus.mockResolvedValueOnce({ readStatus: 'unread' }); + + await expect(controller.setReadStatus(user, 10, { status: 'unread' } as never)).resolves.toEqual({ readStatus: 'unread' }); + expect(catalogService.setReadStatus).toHaveBeenCalledWith(user, 10, 'unread'); + }); + it('passes a named dashboard section through and omits smartScopeId unless it is set', async () => { const { controller, catalogService } = makeController(); const user = { id: 7 } as never; diff --git a/server/src/modules/koreader/koreader-catalog.service.test.ts b/server/src/modules/koreader/koreader-catalog.service.test.ts index b2f46b250..c66842b7d 100644 --- a/server/src/modules/koreader/koreader-catalog.service.test.ts +++ b/server/src/modules/koreader/koreader-catalog.service.test.ts @@ -868,6 +868,17 @@ describe('KoreaderCatalogService', () => { expect(res).toEqual({ readStatus: 'reading' }); }); + it('resets read status to unread over the same manual path', async () => { + const { service, bookService, userBookStatusService } = makeService(); + const user = makeUser({ id: 7 }); + + const res = await service.setReadStatus(user, 10, 'unread'); + + expect(bookService.verifyBookAccess).toHaveBeenCalledWith(10, user); + expect(userBookStatusService.setManual).toHaveBeenCalledWith(7, 10, 'unread'); + expect(res).toEqual({ readStatus: 'unread' }); + }); + it('sets and clears rating through the book service', async () => { const { service, bookService } = makeService(); const user = makeUser({ id: 7 });