fix(sync): propagate group membership for already-synced books (#4946)

Group membership synced only for newly-imported books. Re-grouping a
book already present on both devices bumped book.updatedAt and won the
library-index LWW race, but mergeBookMetadata dropped groupId/groupName
from the overlay, so the change never reached peers. New books instead
arrive via addBookToLibrary with the full remote object, which is why
their group did travel.

Carry groupId/groupName in mergeBookMetadata, matching native cloud
sync (transform.ts maps group_id/group_name). Values are assigned raw
so a group removal also propagates.

Fixes #4942

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin 2026-07-06 01:27:17 +09:00 committed by GitHub
parent ec45a080fc
commit 2963e75bdd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 73 additions and 7 deletions

View file

@ -130,6 +130,33 @@ describe('FileSyncEngine metadata reconciliation (#4756)', () => {
expect(indexedBook.title).toBe('New Title');
});
test('pulls newer remote group membership for a book the device already has (#4942)', async () => {
const local = makeLocalBook({ updatedAt: 100 });
const remote = makeLocalBook({ groupId: 'g1', groupName: 'Sci-Fi', updatedAt: 200 });
const capture: { index?: RemoteLibraryIndex | null } = {};
const provider = makeProvider(makeRemoteIndex(remote, 200), null, capture);
const updateBookMetadata = vi.fn(async (_book: Book) => {});
const store = makeStore({ updateBookMetadata });
const engine = new FileSyncEngine(provider, store);
const result = await engine.syncLibrary([local], {
strategy: 'silent',
syncBooks: false,
deviceId: 'pc-device',
});
expect(updateBookMetadata).toHaveBeenCalledTimes(1);
const merged = updateBookMetadata.mock.calls[0]![0];
expect(merged.groupId).toBe('g1');
expect(merged.groupName).toBe('Sci-Fi');
expect(result.metadataUpdated).toBe(1);
const indexedBook = capture.index!.books.find((b) => b.hash === 'h1')!;
expect(indexedBook.groupId).toBe('g1');
expect(indexedBook.groupName).toBe('Sci-Fi');
});
test('does not overwrite local metadata when the local copy is newer', async () => {
const local = makeLocalBook({ title: 'Local Newer', updatedAt: 300 });
const remote = makeLocalBook({ title: 'Remote Older', updatedAt: 200 });

View file

@ -129,6 +129,36 @@ describe('mergeBookMetadata (LWW field subset)', () => {
expect(m.progress).toEqual([1, 2]);
expect(m.updatedAt).toBe(9);
});
test('carries remote group membership (add-to-group) when remote is newer (#4942)', () => {
const local = { hash: 'h', title: 'T', author: 'A', updatedAt: 1 } as Book;
const remote = {
hash: 'h',
title: 'T',
author: 'A',
groupId: 'g1',
groupName: 'Sci-Fi',
updatedAt: 9,
} as Book;
const m = mergeBookMetadata(local, remote);
expect(m.groupId).toBe('g1');
expect(m.groupName).toBe('Sci-Fi');
});
test('propagates group removal when remote cleared membership (#4942)', () => {
const local = {
hash: 'h',
title: 'T',
author: 'A',
groupId: 'g1',
groupName: 'Sci-Fi',
updatedAt: 1,
} as Book;
const remote = { hash: 'h', title: 'T', author: 'A', updatedAt: 9 } as Book;
const m = mergeBookMetadata(local, remote);
expect(m.groupId).toBeUndefined();
expect(m.groupName).toBeUndefined();
});
});
describe('isRemoteBookMetadataNewer', () => {

View file

@ -87,14 +87,21 @@ export const mergeBookConfig = (
* Overlay the user-facing metadata of `remote` onto `local`, preserving every
* device-local / file-system field: `filePath`, `sourceTitle` (which names the
* on-disk file), `coverImageUrl` (a device-local blob URL the caller
* regenerates), reading progress, reading status, group membership, `hash`,
* `format`, `createdAt`, etc.
* regenerates), reading progress, reading status, `hash`, `format`,
* `createdAt`, etc.
*
* Only the fields a metadata edit actually changes travel this list mirrors
* `getBookWithUpdatedMetadata` in `utils/book.ts`, which is the local side of
* the same operation. The cover image is replicated separately as cover.png
* bytes (see the reconciliation pass in the engine), so it is intentionally
* absent here.
* Group membership (`groupId` / `groupName`) is user-facing metadata and travels
* too, matching the native cloud sync (`transform.ts` maps `group_id` /
* `group_name`). Without this, re-grouping a book already present on both
* devices bumps `updatedAt` and wins LWW, yet the group change is dropped by the
* overlay so it never propagated for already-synced books (#4942). Assigning
* the raw remote values (not `?? local`) lets a group removal (undefined) clear
* membership on peers too.
*
* The scalar fields a metadata edit changes mirror `getBookWithUpdatedMetadata`
* in `utils/book.ts`, which is the local side of the same operation. The cover
* image is replicated separately as cover.png bytes (see the reconciliation pass
* in the engine), so it is intentionally absent here.
*/
export const mergeBookMetadata = (local: Book, remote: Book): Book => ({
...local,
@ -102,6 +109,8 @@ export const mergeBookMetadata = (local: Book, remote: Book): Book => ({
author: remote.author,
metadata: remote.metadata ?? local.metadata,
primaryLanguage: remote.primaryLanguage ?? local.primaryLanguage,
groupId: remote.groupId,
groupName: remote.groupName,
updatedAt: remote.updatedAt,
});