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

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)

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") },

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")

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;

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);
+    }
+  });
+});

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',

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;

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 });