diff --git a/server/src/db/schema/reader.ts b/server/src/db/schema/reader.ts index bf52d56aa..4b681fc26 100644 --- a/server/src/db/schema/reader.ts +++ b/server/src/db/schema/reader.ts @@ -1,4 +1,5 @@ import { date, index, integer, jsonb, pgTable, primaryKey, real, serial, text, timestamp, uniqueIndex, varchar } from 'drizzle-orm/pg-core'; +import type { ReadStatus, ReadStatusSource } from '@projectx/types'; import { bookFiles, books } from './books'; import { libraries } from './libraries'; @@ -14,9 +15,9 @@ export const userBookStatus = pgTable( .notNull() .references(() => books.id, { onDelete: 'cascade' }), // 'unread' | 'want_to_read' | 'reading' | 'on_hold' | 'rereading' | 'read' | 'skimmed' | 'abandoned' - status: varchar('status', { length: 20 }).notNull().default('unread'), + status: varchar('status', { length: 20 }).$type().notNull().default('unread'), // 'auto' (derived from progress) | 'manual' (user-set; never auto-overridden) - source: varchar('source', { length: 10 }).notNull().default('auto'), + source: varchar('source', { length: 10 }).$type().notNull().default('auto'), startedAt: timestamp('started_at'), finishedAt: timestamp('finished_at'), updatedAt: timestamp('updated_at') diff --git a/server/src/modules/user-book-status/dto/set-status.dto.ts b/server/src/modules/user-book-status/dto/set-status.dto.ts index f320c9092..04571eff2 100644 --- a/server/src/modules/user-book-status/dto/set-status.dto.ts +++ b/server/src/modules/user-book-status/dto/set-status.dto.ts @@ -1,7 +1,8 @@ import { IsIn } from 'class-validator'; import type { ReadStatus } from '@projectx/types'; +import { READ_STATUSES } from '../user-book-status.constants'; export class SetStatusDto { - @IsIn(['unread', 'want_to_read', 'reading', 'on_hold', 'rereading', 'read', 'skimmed', 'abandoned']) + @IsIn(READ_STATUSES) status!: ReadStatus; } diff --git a/server/src/modules/user-book-status/user-book-status.constants.ts b/server/src/modules/user-book-status/user-book-status.constants.ts new file mode 100644 index 000000000..6c5273ae3 --- /dev/null +++ b/server/src/modules/user-book-status/user-book-status.constants.ts @@ -0,0 +1,16 @@ +import type { ReadStatus, ReadStatusSource } from '@projectx/types'; + +export const READ_STATUSES: readonly ReadStatus[] = ['unread', 'want_to_read', 'reading', 'on_hold', 'rereading', 'read', 'skimmed', 'abandoned']; + +export const READ_STATUS_SOURCES: readonly ReadStatusSource[] = ['auto', 'manual']; + +const READ_STATUS_SET = new Set(READ_STATUSES); +const READ_STATUS_SOURCE_SET = new Set(READ_STATUS_SOURCES); + +export function isReadStatus(value: string): value is ReadStatus { + return READ_STATUS_SET.has(value as ReadStatus); +} + +export function isReadStatusSource(value: string): value is ReadStatusSource { + return READ_STATUS_SOURCE_SET.has(value as ReadStatusSource); +} diff --git a/server/src/modules/user-book-status/user-book-status.repository.test.ts b/server/src/modules/user-book-status/user-book-status.repository.test.ts new file mode 100644 index 000000000..f832d1426 --- /dev/null +++ b/server/src/modules/user-book-status/user-book-status.repository.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest'; + +import type { UserBookStatusRow } from '../../db/schema'; +import { deriveLifecycle } from './user-book-status.repository'; + +function makeRow(overrides: Partial = {}): UserBookStatusRow { + return { + userId: 1, + bookId: 10, + status: 'reading', + source: 'auto', + startedAt: new Date('2024-01-01T00:00:00.000Z'), + finishedAt: new Date('2024-02-01T00:00:00.000Z'), + updatedAt: new Date('2024-03-01T00:00:00.000Z'), + ...overrides, + }; +} + +describe('deriveLifecycle', () => { + const now = new Date('2025-01-15T12:00:00.000Z'); + const existingStarted = new Date('2024-04-01T10:00:00.000Z'); + const existingFinished = new Date('2024-08-01T10:00:00.000Z'); + const existing = makeRow({ startedAt: existingStarted, finishedAt: existingFinished, status: 'read' }); + + it('clears both timestamps for unread and want_to_read', () => { + expect(deriveLifecycle('unread', now, existing)).toEqual({ startedAt: null, finishedAt: null }); + expect(deriveLifecycle('want_to_read', now, existing)).toEqual({ startedAt: null, finishedAt: null }); + }); + + it('initializes startedAt and clears finishedAt for active statuses', () => { + for (const status of ['reading', 'on_hold', 'rereading'] as const) { + expect(deriveLifecycle(status, now, null)).toEqual({ startedAt: now, finishedAt: null }); + expect(deriveLifecycle(status, now, existing)).toEqual({ startedAt: existingStarted, finishedAt: null }); + } + }); + + it('sets finishedAt to now for read and keeps existing startedAt', () => { + expect(deriveLifecycle('read', now, null)).toEqual({ startedAt: now, finishedAt: now }); + expect(deriveLifecycle('read', now, existing)).toEqual({ startedAt: existingStarted, finishedAt: now }); + }); + + it('clears stale finishedAt for skimmed and abandoned', () => { + expect(deriveLifecycle('skimmed', now, existing)).toEqual({ startedAt: existingStarted, finishedAt: null }); + expect(deriveLifecycle('abandoned', now, existing)).toEqual({ startedAt: existingStarted, finishedAt: null }); + }); + + it('initializes startedAt for skimmed and abandoned when missing', () => { + const withoutStarted = makeRow({ startedAt: null, finishedAt: existingFinished, status: 'reading' }); + + expect(deriveLifecycle('skimmed', now, withoutStarted)).toEqual({ startedAt: now, finishedAt: null }); + expect(deriveLifecycle('abandoned', now, withoutStarted)).toEqual({ startedAt: now, finishedAt: null }); + }); +}); diff --git a/server/src/modules/user-book-status/user-book-status.repository.ts b/server/src/modules/user-book-status/user-book-status.repository.ts index 1be4407af..d8d9501a3 100644 --- a/server/src/modules/user-book-status/user-book-status.repository.ts +++ b/server/src/modules/user-book-status/user-book-status.repository.ts @@ -6,10 +6,27 @@ import { DB } from '../../db'; import * as schema from '../../db/schema'; import { userBookStatus } from '../../db/schema'; import type { ReadStatus, ReadStatusSource } from '@projectx/types'; +import type { UserBookStatusRow } from '../../db/schema'; type Db = NodePgDatabase; -const STARTED_FROM_ACTIVE_STATUSES: ReadStatus[] = ['reading', 'on_hold', 'rereading']; -const RESET_FINISHED_AT_STATUSES: ReadStatus[] = ['unread', 'want_to_read', 'reading', 'on_hold', 'rereading', 'skimmed']; + +export type UserBookStatusLifecycle = Pick; + +export function deriveLifecycle(status: ReadStatus, now: Date, existing: UserBookStatusRow | null): UserBookStatusLifecycle { + switch (status) { + case 'unread': + case 'want_to_read': + return { startedAt: null, finishedAt: null }; + case 'reading': + case 'on_hold': + case 'rereading': + case 'skimmed': + case 'abandoned': + return { startedAt: existing?.startedAt ?? now, finishedAt: null }; + case 'read': + return { startedAt: existing?.startedAt ?? now, finishedAt: now }; + } +} @Injectable() export class UserBookStatusRepository { @@ -41,9 +58,7 @@ export class UserBookStatusRepository { existing?: Awaited>, ): Promise { const row = existing !== undefined ? existing : await this.findOne(userId, bookId); - - const startedAt = row?.startedAt ?? (STARTED_FROM_ACTIVE_STATUSES.includes(status) ? now : null); - const finishedAt = status === 'read' ? now : RESET_FINISHED_AT_STATUSES.includes(status) ? null : (row?.finishedAt ?? null); + const { startedAt, finishedAt } = deriveLifecycle(status, now, row); await this.db .insert(userBookStatus) diff --git a/server/src/modules/user-book-status/user-book-status.service.test.ts b/server/src/modules/user-book-status/user-book-status.service.test.ts index 254674b0b..d3e58d95b 100644 --- a/server/src/modules/user-book-status/user-book-status.service.test.ts +++ b/server/src/modules/user-book-status/user-book-status.service.test.ts @@ -1,6 +1,10 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { UserBookStatusService } from './user-book-status.service'; +import { InternalServerErrorException } from '@nestjs/common'; +import type { ReadStatus, ReadStatusSource } from '@projectx/types'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + import type { UserBookStatusRow } from '../../db/schema'; +import { UserBookStatusRepository } from './user-book-status.repository'; +import { UserBookStatusService } from './user-book-status.service'; function makeRow(overrides: Partial = {}): UserBookStatusRow { return { @@ -16,140 +20,155 @@ function makeRow(overrides: Partial = {}): UserBookStatusRow } const mockRepo = { - findOne: vi.fn(), - findByBookIds: vi.fn(), - upsert: vi.fn(), + findOne: vi.fn<(...args: [number, number]) => Promise>(), + findByBookIds: vi.fn<(...args: [number, number[]]) => Promise>(), + upsert: vi.fn<(...args: [number, number, ReadStatus, ReadStatusSource, Date, (UserBookStatusRow | null)?]) => Promise>(), }; let service: UserBookStatusService; beforeEach(() => { vi.clearAllMocks(); + mockRepo.findOne.mockResolvedValue(null); + mockRepo.findByBookIds.mockResolvedValue([]); mockRepo.upsert.mockResolvedValue(undefined); - service = new UserBookStatusService(mockRepo as any); + service = new UserBookStatusService(mockRepo as unknown as UserBookStatusRepository); }); describe('setManual', () => { - it('calls upsert with source manual and correct args', async () => { + it('calls upsert with manual source', async () => { await service.setManual(1, 10, 'reading'); + expect(mockRepo.upsert).toHaveBeenCalledOnce(); - const [userId, bookId, status, source] = mockRepo.upsert.mock.calls[0]; + const [userId, bookId, status, source, now] = mockRepo.upsert.mock.calls[0]; expect(userId).toBe(1); expect(bookId).toBe(10); expect(status).toBe('reading'); expect(source).toBe('manual'); - }); - - it('passes a Date instance for now', async () => { - await service.setManual(1, 10, 'read'); - const now = mockRepo.upsert.mock.calls[0][4]; expect(now).toBeInstanceOf(Date); }); }); -describe('autoUpdate — default thresholds', () => { - it('does not call upsert when no existing record and percentage is below reading threshold', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 0.1); - expect(mockRepo.upsert).not.toHaveBeenCalled(); - }); +describe('autoUpdate with default thresholds', () => { + it.each([ + { percentage: 0.1, expectedStatus: null }, + { percentage: 0.25, expectedStatus: 'reading' }, + { percentage: 50, expectedStatus: 'reading' }, + { percentage: 98, expectedStatus: 'read' }, + { percentage: 100, expectedStatus: 'read' }, + ])('derives expected status for percentage=$percentage', async ({ percentage, expectedStatus }) => { + await service.autoUpdate(1, 10, percentage); + + if (expectedStatus === null) { + expect(mockRepo.upsert).not.toHaveBeenCalled(); + return; + } - it('calls upsert with reading when no existing record and percentage equals reading threshold', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 0.25); expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading'); + expect(mockRepo.upsert.mock.calls[0][2]).toBe(expectedStatus); }); +}); - it('calls upsert with reading when no existing record and percentage is above reading but below finish', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 50); - expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading'); - }); +describe('autoUpdate with custom thresholds', () => { + it.each([ + { percentage: 0.5, readingThreshold: 1, finishThreshold: 90, existingStatus: 'reading', expectedStatus: 'unread' }, + { percentage: 1, readingThreshold: 1, finishThreshold: 90, existingStatus: 'unread', expectedStatus: 'reading' }, + { percentage: 90, readingThreshold: 1, finishThreshold: 90, existingStatus: 'reading', expectedStatus: 'read' }, + ])( + 'derives expected status for percentage=$percentage', + async ({ percentage, readingThreshold, finishThreshold, existingStatus, expectedStatus }) => { + mockRepo.findOne.mockResolvedValue(makeRow({ status: existingStatus, source: 'auto' })); - it('calls upsert with read when no existing record and percentage equals finish threshold', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 98); - expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('read'); - }); + await service.autoUpdate(1, 10, percentage, readingThreshold, finishThreshold); + + expect(mockRepo.upsert).toHaveBeenCalledOnce(); + expect(mockRepo.upsert.mock.calls[0][2]).toBe(expectedStatus); + }, + ); + + it('falls back to defaults for null and undefined thresholds', async () => { + await service.autoUpdate(1, 10, 98, null, undefined); - it('calls upsert with read when no existing record and percentage is above finish threshold', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 100); expect(mockRepo.upsert).toHaveBeenCalledOnce(); expect(mockRepo.upsert.mock.calls[0][2]).toBe('read'); }); }); -describe('autoUpdate — custom thresholds', () => { - it('derives unread when percentage is below custom reading threshold', async () => { +describe('autoUpdate normalization and guard behavior', () => { + it('does not override manual statuses', async () => { + mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'manual' })); + + await service.autoUpdate(1, 10, 100); + + expect(mockRepo.upsert).not.toHaveBeenCalled(); + }); + + it('skips updates when derived status is unchanged', async () => { mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' })); - await service.autoUpdate(1, 10, 0.5, 1, 90); + + await service.autoUpdate(1, 10, 50); + + expect(mockRepo.upsert).not.toHaveBeenCalled(); + }); + + it('passes existing row to upsert for lifecycle derivation', async () => { + const existing = makeRow({ status: 'reading', source: 'auto' }); + mockRepo.findOne.mockResolvedValue(existing); + + await service.autoUpdate(1, 10, 99); + + expect(mockRepo.upsert).toHaveBeenCalledOnce(); + expect(mockRepo.upsert.mock.calls[0][5]).toBe(existing); + }); + + it('clamps percentage values outside 0..100', async () => { + mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' })); + + await service.autoUpdate(1, 10, 120); + expect(mockRepo.upsert).toHaveBeenCalledOnce(); + expect(mockRepo.upsert.mock.calls[0][2]).toBe('read'); + + vi.clearAllMocks(); + mockRepo.findOne.mockResolvedValue(makeRow({ status: 'read', source: 'auto' })); + mockRepo.upsert.mockResolvedValue(undefined); + + await service.autoUpdate(1, 10, -12); expect(mockRepo.upsert).toHaveBeenCalledOnce(); expect(mockRepo.upsert.mock.calls[0][2]).toBe('unread'); }); - it('derives reading when percentage equals custom reading threshold', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 1.0, 1, 90); + it('treats non-finite percentages as 0', async () => { + mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' })); + + await service.autoUpdate(1, 10, Number.NaN); + expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading'); + expect(mockRepo.upsert.mock.calls[0][2]).toBe('unread'); }); - it('derives read when percentage equals custom finish threshold', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 90, 1, 90); - expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('read'); - }); - - it('falls back to default thresholds when null is passed', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 0.25, null, null); - expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading'); - }); - - it('falls back to default thresholds when undefined is passed', async () => { - mockRepo.findOne.mockResolvedValue(null); - await service.autoUpdate(1, 10, 98, undefined, undefined); - expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('read'); - }); -}); - -describe('autoUpdate — existing record logic', () => { - it('calls upsert when existing auto record is unread and derived is reading', async () => { + it('falls back to default thresholds for non-finite values', async () => { mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'auto' })); - await service.autoUpdate(1, 10, 50); + + await service.autoUpdate(1, 10, 50, Number.NaN, Number.POSITIVE_INFINITY); + expect(mockRepo.upsert).toHaveBeenCalledOnce(); expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading'); }); - it('does not call upsert when existing auto record matches derived status', async () => { - mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' })); - await service.autoUpdate(1, 10, 50); - expect(mockRepo.upsert).not.toHaveBeenCalled(); - }); + it('clamps custom thresholds into the 0..100 range', async () => { + mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'auto' })); + + await service.autoUpdate(1, 10, 50, -5, 150); - it('calls upsert when existing auto record is reading and derived is read', async () => { - mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' })); - await service.autoUpdate(1, 10, 99); expect(mockRepo.upsert).toHaveBeenCalledOnce(); - expect(mockRepo.upsert.mock.calls[0][2]).toBe('read'); + expect(mockRepo.upsert.mock.calls[0][2]).toBe('reading'); }); - it('does not call upsert when existing record source is manual', async () => { - mockRepo.findOne.mockResolvedValue(makeRow({ status: 'unread', source: 'manual' })); - await service.autoUpdate(1, 10, 100); - expect(mockRepo.upsert).not.toHaveBeenCalled(); - }); + it('normalizes inverted thresholds to keep read threshold <= finish threshold', async () => { + mockRepo.findOne.mockResolvedValue(makeRow({ status: 'reading', source: 'auto' })); + + await service.autoUpdate(1, 10, 79, 90, 80); - it('calls upsert with unread when existing auto read record drops below reading threshold', async () => { - mockRepo.findOne.mockResolvedValue(makeRow({ status: 'read', source: 'auto' })); - await service.autoUpdate(1, 10, 0.1); expect(mockRepo.upsert).toHaveBeenCalledOnce(); expect(mockRepo.upsert.mock.calls[0][2]).toBe('unread'); }); @@ -157,17 +176,18 @@ describe('autoUpdate — existing record logic', () => { describe('findOne', () => { it('returns null when repo returns null', async () => { - mockRepo.findOne.mockResolvedValue(null); const result = await service.findOne(1, 10); expect(result).toBeNull(); }); - it('maps row to DTO with all fields as ISO strings', async () => { + it('maps valid rows into DTO shape', async () => { const started = new Date('2024-03-01T08:00:00.000Z'); const finished = new Date('2024-06-01T12:00:00.000Z'); const updated = new Date('2024-06-01T12:00:00.000Z'); mockRepo.findOne.mockResolvedValue(makeRow({ status: 'read', source: 'manual', startedAt: started, finishedAt: finished, updatedAt: updated })); + const result = await service.findOne(1, 10); + expect(result).toEqual({ status: 'read', source: 'manual', @@ -177,45 +197,51 @@ describe('findOne', () => { }); }); - it('returns null startedAt and finishedAt when they are null on the row', async () => { - mockRepo.findOne.mockResolvedValue(makeRow({ startedAt: null, finishedAt: null })); - const result = await service.findOne(1, 10); - expect(result?.startedAt).toBeNull(); - expect(result?.finishedAt).toBeNull(); + it('throws when row status is invalid', async () => { + mockRepo.findOne.mockResolvedValue( + makeRow({ + status: 'not_a_status' as unknown as UserBookStatusRow['status'], + }), + ); + + await expect(service.findOne(1, 10)).rejects.toThrowError(InternalServerErrorException); + }); + + it('throws when row source is invalid', async () => { + mockRepo.findOne.mockResolvedValue( + makeRow({ + source: 'not_a_source' as unknown as UserBookStatusRow['source'], + }), + ); + + await expect(service.findOne(1, 10)).rejects.toThrowError(InternalServerErrorException); }); }); describe('findByBookIds', () => { - it('returns empty Map when repo returns empty array', async () => { - mockRepo.findByBookIds.mockResolvedValue([]); + it('returns empty map when repo returns no rows', async () => { const result = await service.findByBookIds(1, []); expect(result.size).toBe(0); }); - it('maps a single row correctly into the Map', async () => { - const updated = new Date('2024-05-01T00:00:00.000Z'); - mockRepo.findByBookIds.mockResolvedValue([makeRow({ bookId: 10, status: 'reading', source: 'auto', updatedAt: updated })]); - const result = await service.findByBookIds(1, [10]); - expect(result.size).toBe(1); - expect(result.get(10)).toEqual({ - status: 'reading', - source: 'auto', - startedAt: null, - finishedAt: null, - updatedAt: updated.toISOString(), - }); - }); - - it('maps multiple rows keyed by bookId', async () => { + it('maps rows keyed by bookId', async () => { const updated1 = new Date('2024-05-01T00:00:00.000Z'); const updated2 = new Date('2024-06-01T00:00:00.000Z'); mockRepo.findByBookIds.mockResolvedValue([ makeRow({ bookId: 10, status: 'reading', source: 'auto', updatedAt: updated1 }), makeRow({ bookId: 20, status: 'read', source: 'manual', updatedAt: updated2 }), ]); + const result = await service.findByBookIds(1, [10, 20]); + expect(result.size).toBe(2); - expect(result.get(10)?.status).toBe('reading'); + expect(result.get(10)).toEqual({ + status: 'reading', + source: 'auto', + startedAt: null, + finishedAt: null, + updatedAt: updated1.toISOString(), + }); expect(result.get(20)?.status).toBe('read'); }); }); diff --git a/server/src/modules/user-book-status/user-book-status.service.ts b/server/src/modules/user-book-status/user-book-status.service.ts index 7d0861997..0c8d913df 100644 --- a/server/src/modules/user-book-status/user-book-status.service.ts +++ b/server/src/modules/user-book-status/user-book-status.service.ts @@ -1,10 +1,13 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, InternalServerErrorException } from '@nestjs/common'; import type { ReadStatus, UserBookStatus } from '@projectx/types'; import { UserBookStatusRepository } from './user-book-status.repository'; import type { UserBookStatusRow } from '../../db/schema'; +import { isReadStatus, isReadStatusSource } from './user-book-status.constants'; const DEFAULT_FINISH_THRESHOLD = 98; const READING_THRESHOLD = 0.25; +const MIN_PERCENTAGE = 0; +const MAX_PERCENTAGE = 100; @Injectable() export class UserBookStatusService { @@ -25,9 +28,10 @@ export class UserBookStatusService { if (existing?.source === 'manual') return; - const readTh = readingThreshold ?? READING_THRESHOLD; - const finishTh = finishThreshold ?? DEFAULT_FINISH_THRESHOLD; - const derived: ReadStatus = percentage >= finishTh ? 'read' : percentage >= readTh ? 'reading' : 'unread'; + const normalizedPercentage = this.normalizePercentage(percentage); + const { readThreshold, finishThreshold: normalizedFinishThreshold } = this.normalizeThresholds(readingThreshold, finishThreshold); + const derived: ReadStatus = + normalizedPercentage >= normalizedFinishThreshold ? 'read' : normalizedPercentage >= readThreshold ? 'reading' : 'unread'; if (!existing && derived === 'unread') return; if (existing?.status === derived) return; @@ -50,12 +54,48 @@ export class UserBookStatusService { } private toDto(row: UserBookStatusRow): UserBookStatus { + const status = this.toReadStatus(row.status); + const source = this.toReadStatusSource(row.source); + return { - status: row.status as ReadStatus, - source: row.source as 'auto' | 'manual', + status, + source, startedAt: row.startedAt?.toISOString() ?? null, finishedAt: row.finishedAt?.toISOString() ?? null, updatedAt: row.updatedAt.toISOString(), }; } + + private normalizePercentage(value: number): number { + if (!Number.isFinite(value)) return MIN_PERCENTAGE; + return Math.min(MAX_PERCENTAGE, Math.max(MIN_PERCENTAGE, value)); + } + + private normalizeThresholds(readingThreshold?: number | null, finishThreshold?: number | null) { + const normalizedReading = this.normalizeThreshold(readingThreshold, READING_THRESHOLD); + const normalizedFinish = this.normalizeThreshold(finishThreshold, DEFAULT_FINISH_THRESHOLD); + if (normalizedReading <= normalizedFinish) { + return { readThreshold: normalizedReading, finishThreshold: normalizedFinish }; + } + return { readThreshold: normalizedFinish, finishThreshold: normalizedFinish }; + } + + private normalizeThreshold(value: number | null | undefined, fallback: number): number { + if (value == null || !Number.isFinite(value)) return fallback; + return Math.min(MAX_PERCENTAGE, Math.max(MIN_PERCENTAGE, value)); + } + + private toReadStatus(value: string): ReadStatus { + if (!isReadStatus(value)) { + throw new InternalServerErrorException(`Invalid read status value: ${value}`); + } + return value; + } + + private toReadStatusSource(value: string): 'auto' | 'manual' { + if (!isReadStatusSource(value)) { + throw new InternalServerErrorException(`Invalid read status source value: ${value}`); + } + return value; + } }