From 2cbd76bc7fa7f682742c2307338d2fb2dee2fa7a Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:14:25 +0000 Subject: [PATCH] fix(stats): preserve historical KOReader completion dates Use the source finish day when importing completed KOReader books so completion timelines reflect reading history instead of the sync date. Preserve newer manual edits and retain fallback behavior for unusable dates. Closes #1783 --- .../koreader/koreader-plugin.service.test.ts | 41 ++++++++++- .../koreader/koreader-plugin.service.ts | 21 +++++- server/test/reading-attempts.e2e-spec.ts | 73 +++++++++++++++++++ 3 files changed, 129 insertions(+), 6 deletions(-) diff --git a/server/src/modules/koreader/koreader-plugin.service.test.ts b/server/src/modules/koreader/koreader-plugin.service.test.ts index 0f205bf48..b3d522fe3 100644 --- a/server/src/modules/koreader/koreader-plugin.service.test.ts +++ b/server/src/modules/koreader/koreader-plugin.service.test.ts @@ -49,6 +49,7 @@ describe('KoreaderPluginService', () => { findOne: ReturnType; findByBookIds: ReturnType; setManual: ReturnType; + updateManual: ReturnType; }; let userBookNoteService: { findByBookIds: ReturnType; @@ -87,6 +88,7 @@ describe('KoreaderPluginService', () => { findOne: vi.fn().mockResolvedValue(null), findByBookIds: vi.fn().mockResolvedValue(new Map()), setManual: vi.fn().mockResolvedValue(undefined), + updateManual: vi.fn().mockResolvedValue(undefined), }; userBookNoteService = { findByBookIds: vi.fn().mockResolvedValue(new Map()), @@ -400,7 +402,13 @@ describe('KoreaderPluginService', () => { it('applies a status when no server status exists', async () => { const result = await service.uploadBookStates(makeUser(), statesDto([{ hash: HASH_A, status: 'complete', statusModified: '2026-06-01' }])); - expect(userBookStatusService.setManual).toHaveBeenCalledWith(7, 20, 'read', 'UTC'); + expect(userBookStatusService.updateManual).toHaveBeenCalledWith( + 7, + 20, + { status: 'read', finishedAt: new Date('2026-06-01T00:00:00.000Z') }, + { endedOn: '2026-06-01' }, + 'UTC', + ); expect(result.results[0]).toMatchObject({ hash: HASH_A, statusApplied: true, ratingApplied: false, reviewApplied: false }); }); @@ -410,7 +418,32 @@ describe('KoreaderPluginService', () => { statesDto([{ hash: HASH_A, status: 'complete', statusModified: '2026-06-01' }]), ); - expect(userBookStatusService.setManual).toHaveBeenCalledWith(7, 20, 'read', 'America/Chicago'); + expect(userBookStatusService.updateManual).toHaveBeenCalledWith( + 7, + 20, + { status: 'read', finishedAt: new Date('2026-06-01T00:00:00.000Z') }, + { endedOn: '2026-06-01' }, + 'America/Chicago', + ); + }); + + it.each([undefined, '2026-02-30', '0000-01-01'])('keeps completion uploads compatible without a usable date: %s', async (statusModified) => { + const result = await service.uploadBookStates(makeUser(), statesDto([{ hash: HASH_A, status: 'complete', statusModified }])); + + expect(userBookStatusService.setManual).toHaveBeenCalledWith(7, 20, 'read', 'UTC'); + expect(userBookStatusService.updateManual).not.toHaveBeenCalled(); + expect(result.results[0]!.statusApplied).toBe(true); + }); + + it('does not introduce a date-order failure when a device finish precedes the existing start', async () => { + userBookStatusService.findByBookIds.mockResolvedValue( + new Map([[20, { status: 'reading', source: 'manual', startedAt: '2026-07-01T00:00:00.000Z', updatedAt: '2026-06-01T08:00:00.000Z' }]]), + ); + + await service.uploadBookStates(makeUser(), statesDto([{ hash: HASH_A, status: 'complete', statusModified: '2026-06-02' }])); + + expect(userBookStatusService.setManual).toHaveBeenCalledWith(7, 20, 'read', 'UTC'); + expect(userBookStatusService.updateManual).not.toHaveBeenCalled(); }); it('treats an identical status as applied without writing', async () => { @@ -419,6 +452,7 @@ describe('KoreaderPluginService', () => { const result = await service.uploadBookStates(makeUser(), statesDto([{ hash: HASH_A, status: 'complete', statusModified: '2026-01-01' }])); expect(userBookStatusService.setManual).not.toHaveBeenCalled(); + expect(userBookStatusService.updateManual).not.toHaveBeenCalled(); expect(result.results[0]!.statusApplied).toBe(true); }); @@ -437,6 +471,7 @@ describe('KoreaderPluginService', () => { const older = await service.uploadBookStates(makeUser(), statesDto([{ hash: HASH_A, status: 'complete', statusModified: '2026-06-04' }])); expect(userBookStatusService.setManual).not.toHaveBeenCalled(); + expect(userBookStatusService.updateManual).not.toHaveBeenCalled(); expect(tie.results[0]!.statusApplied).toBe(false); expect(older.results[0]!.statusApplied).toBe(false); }); @@ -594,7 +629,7 @@ describe('KoreaderPluginService', () => { ]), ); - expect(userBookStatusService.setManual).toHaveBeenCalledTimes(1); + expect(userBookStatusService.updateManual).toHaveBeenCalledTimes(1); expect(userBookStatusService.findOne).toHaveBeenCalledTimes(1); expect(result.results.map((entry) => entry.statusApplied)).toEqual([true, true]); }); diff --git a/server/src/modules/koreader/koreader-plugin.service.ts b/server/src/modules/koreader/koreader-plugin.service.ts index b6958908f..24b695e17 100644 --- a/server/src/modules/koreader/koreader-plugin.service.ts +++ b/server/src/modules/koreader/koreader-plugin.service.ts @@ -5,7 +5,7 @@ import type { ReadStatus, UserBookStatus } from '@bookorbit/types'; import type { RequestUser } from '../../common/types/request-user'; import { mapWithConcurrency } from '../../common/utils/batch.utils'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; -import { resolveTimeZone } from '../../common/utils/timezone.utils'; +import { isDateKey, resolveTimeZone } from '../../common/utils/timezone.utils'; import { ACHIEVEMENT_EVENT_BOOK_RATING_CHANGED, AchievementEventsService } from '../achievement/achievement-events.service'; import { UserBookNoteService, type UserBookNoteDto } from '../user-book-note/user-book-note.service'; import { BookService } from '../book/book.service'; @@ -426,7 +426,7 @@ export class KoreaderPluginService { } /** - * Status writes stay on UserBookStatusService because setManual drives reading-attempt + * Status writes stay on UserBookStatusService because it drives reading-attempt * lifecycle bookkeeping that a bulk row write would silently skip. It also projects a * status that can differ from the requested one, so a book written during this batch is * re-read only if a later entry resolves to it again. @@ -449,7 +449,22 @@ export class KoreaderPluginService { if (!statusModified || statusModified <= serverDate) return false; } - await this.userBookStatusService.setManual(userId, bookId, mapped, state.timeZone); + const finishedOn = + mapped === 'read' && statusModified && isDateKey(statusModified) && (!existing?.startedAt || statusModified >= existing.startedAt.slice(0, 10)) + ? statusModified + : undefined; + if (finishedOn) { + // KOReader supplies a calendar day, which must not shift with the reader's timezone. + await this.userBookStatusService.updateManual( + userId, + bookId, + { status: mapped, finishedAt: new Date(`${finishedOn}T00:00:00.000Z`) }, + { endedOn: finishedOn }, + state.timeZone, + ); + } else { + await this.userBookStatusService.setManual(userId, bookId, mapped, state.timeZone); + } state.staleStatusBookIds.add(bookId); return true; } diff --git a/server/test/reading-attempts.e2e-spec.ts b/server/test/reading-attempts.e2e-spec.ts index b700d6d4a..4b000a157 100644 --- a/server/test/reading-attempts.e2e-spec.ts +++ b/server/test/reading-attempts.e2e-spec.ts @@ -117,6 +117,79 @@ describe('Reading attempts main-flow simulation (docker e2e)', { timeout: TIMEOU expect(history.items[0]?.outcome).toBe('completed'); }); + it('uses a historical KOReader finish day for the book and completion statistics without overwriting later manual edits', async () => { + const book = await createBook(); + const fileHash = randomUUID().replaceAll('-', ''); + await ctx.db.update(schema.bookFiles).set({ fileHash }).where(eq(schema.bookFiles.id, book.fileId)); + const username = `attempt-date-${randomUUID()}`; + const password = 'HistoricalFinish123'; + const credentials = await ctx.app.inject({ + method: 'POST', + url: '/api/v1/koreader/credentials', + headers: auth(ctx.adminToken), + payload: { username, password }, + }); + expect(credentials.statusCode).toBe(201); + + const year = new Date().getUTCFullYear() - 1; + const finishedOn = `${year}-08-22`; + const startTime = Date.parse(`${finishedOn}T18:00:00.000Z`) / 1000; + const deviceFields = { deviceId: 'historical-finish-e2e', deviceModel: 'E2E', pluginVersion: '1.7.0' }; + const deviceHeaders = { 'x-auth-user': username, 'x-auth-key': password }; + const stats = await ctx.app.inject({ + method: 'POST', + url: '/api/v1/koreader/plugin/page-stats', + headers: deviceHeaders, + payload: { + ...deviceFields, + books: [{ hash: fileHash, events: [{ page: 100, totalPages: 100, startTime, durationSeconds: 60 }] }], + }, + }); + expect(stats.statusCode).toBe(201); + expect(stats.json().results[0].accepted).toBe(1); + + const timeline = async () => { + const response = await ctx.app.inject({ + method: 'GET', + url: '/api/v1/user-statistics/completion-timeline?days=3650', + headers: auth(ctx.adminToken), + }); + expect(response.statusCode).toBe(200); + return response.json() as Array<{ year: number; month: number; count: number }>; + }; + const before = await timeline(); + const uploadState = () => + ctx.app.inject({ + method: 'POST', + url: '/api/v1/koreader/plugin/book-states', + headers: deviceHeaders, + payload: { ...deviceFields, books: [{ hash: fileHash, status: 'complete', statusModified: finishedOn }] }, + }); + expect((await uploadState()).statusCode).toBe(201); + expect((await uploadState()).statusCode).toBe(201); + + const history = await listAttempts(book.bookId); + expect(history.total).toBe(1); + expect(history.items[0]).toMatchObject({ outcome: 'completed', endedOn: finishedOn }); + const detail = await ctx.app.inject({ method: 'GET', url: `/api/v1/books/${book.bookId}`, headers: auth(ctx.adminToken) }); + expect(detail.statusCode).toBe(200); + expect(detail.json().readStatus.finishedAt).toBe(`${finishedOn}T00:00:00.000Z`); + const after = await timeline(); + expect(after.find((point) => point.year === year && point.month === 8)?.count).toBe( + (before.find((point) => point.year === year && point.month === 8)?.count ?? 0) + 1, + ); + const now = new Date(); + expect(after.find((point) => point.year === now.getUTCFullYear() && point.month === now.getUTCMonth() + 1)?.count).toBe( + before.find((point) => point.year === now.getUTCFullYear() && point.month === now.getUTCMonth() + 1)?.count, + ); + + expect((await patchStatus(book.bookId, { finishedAt: `${year}-08-23` })).statusCode).toBe(200); + expect((await uploadState()).statusCode).toBe(201); + const editedHistory = await listAttempts(book.bookId); + expect(editedHistory.total).toBe(1); + expect(editedHistory.items[0]?.endedOn).toBe(`${year}-08-23`); + }); + it('3. starts a manual reread and resets progress without deleting history', async () => { const book = await createBook(); await patchStatus(book.bookId, { status: 'read', startedAt: '2024-01-01', finishedAt: '2024-01-10' });