forked from akai/readest
fix(sync): stop re-pinning statusless books to the top of the library after every sync (#4677)
The sync POST handler rewrote books.updated_at = now() whenever a pushed book's resolved reading_status differed from the server row's. A book that was imported locally and never given a status sends reading_status: undefined, while the server stores null, so `undefined !== null` reported a spurious status change. The 1-day re-sync window re-pushes every recently touched book on each sync, so the server stamped those books with a fresh, batch-identical timestamp every cycle, floating them to the top of the date-sorted library (and above a book the user had just read). Normalize nullish reading_status values before comparing so a statusless book never registers as a status change. Verified on-device via CDP: PUSH_SENT carried the old timestamp while PUSH_RETURNED came back with a fresh now() for exactly the statusless books. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,32 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { resolveReadingStatusMerge } from '@/pages/api/sync';
|
||||
import { readingStatusChanged, resolveReadingStatusMerge } from '@/pages/api/sync';
|
||||
|
||||
const iso = (ms: number) => new Date(ms).toISOString();
|
||||
|
||||
describe('readingStatusChanged', () => {
|
||||
// The bug: a locally-imported book the client never gave a status to sends
|
||||
// `reading_status: undefined`, while the server row stores `null`. Comparing
|
||||
// them with `!==` reports a spurious change, so the server rewrites
|
||||
// `updated_at = now()` on every push and the book floats to the top of the
|
||||
// date-sorted library after every sync.
|
||||
it('treats client undefined and server null as the same (no change)', () => {
|
||||
expect(readingStatusChanged(undefined, null)).toBe(false);
|
||||
expect(readingStatusChanged(null, undefined)).toBe(false);
|
||||
expect(readingStatusChanged(null, null)).toBe(false);
|
||||
expect(readingStatusChanged(undefined, undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('reports a real status change', () => {
|
||||
expect(readingStatusChanged('finished', null)).toBe(true);
|
||||
expect(readingStatusChanged(undefined, 'reading')).toBe(true);
|
||||
expect(readingStatusChanged('reading', 'finished')).toBe(true);
|
||||
});
|
||||
|
||||
it('reports no change when both sides hold the same status', () => {
|
||||
expect(readingStatusChanged('finished', 'finished')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveReadingStatusMerge', () => {
|
||||
it('keeps the client status when its status timestamp is newer', () => {
|
||||
const out = resolveReadingStatusMerge(
|
||||
|
||||
@@ -44,6 +44,19 @@ export function pickWinningPages(
|
||||
* decided the other way by updated_at (which page-turn progress dominates) —
|
||||
* issue #4634.
|
||||
*/
|
||||
/**
|
||||
* `undefined` (the client omitted reading_status entirely — e.g. a locally
|
||||
* imported book that never had a status set) and `null` (the DB default) both
|
||||
* mean "no reading status". Collapse them so a statusless book never registers
|
||||
* as a status change. Without this, the `statusChanged` branch below rewrites
|
||||
* `updated_at = now()` on every push for such books, and since the 1-day
|
||||
* re-sync window re-pushes recently-touched books each cycle, they get a fresh
|
||||
* timestamp every sync and pin themselves to the top of the date-sorted
|
||||
* library.
|
||||
*/
|
||||
export const readingStatusChanged = (client?: string | null, server?: string | null): boolean =>
|
||||
(client ?? null) !== (server ?? null);
|
||||
|
||||
export function resolveReadingStatusMerge(
|
||||
client: Pick<DBBook, 'reading_status' | 'reading_status_updated_at'>,
|
||||
server: Pick<DBBook, 'reading_status' | 'reading_status_updated_at'>,
|
||||
@@ -419,7 +432,10 @@ export async function POST(req: NextRequest) {
|
||||
// Only rewrite when the resolved status VALUE differs from the
|
||||
// server's — a timestamp-only difference on the same value is a
|
||||
// no-op, and rewriting it would churn updated_at + re-propagate.
|
||||
const statusChanged = status.reading_status !== serverBook.reading_status;
|
||||
const statusChanged = readingStatusChanged(
|
||||
status.reading_status,
|
||||
serverBook.reading_status,
|
||||
);
|
||||
if (statusChanged) {
|
||||
// Server wins the row, but the client's status is newer. Write
|
||||
// server's row with the fresher status and bump updated_at so
|
||||
|
||||
Reference in New Issue
Block a user