From 7701a67dff89b50bd855087560d03d97ff09af65 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 27 Sep 2026 07:46:01 +0200 Subject: [PATCH] fix: keep favourites and joins when the server refuses an edit #34 When the server refused an edit made on the device (a book or calendar that turned read-only, an event in a calendar where only one's own events may be changed), the engine put the item back by deleting its rows and inserting a fresh copy. A contact lost its star, ringtone and the link to the contacts it was joined with; an event got a new row id. The rows are now rewritten in place from the server's version, as after an accepted upload. A refused deletion still brings the item back as a new copy. --- .../__tests__/engine/real-planners.test.ts | 57 +++++++++++++++++++ src/device-sync/engine/item-sync.ts | 19 +++++++ 2 files changed, 76 insertions(+) diff --git a/src/device-sync/__tests__/engine/real-planners.test.ts b/src/device-sync/__tests__/engine/real-planners.test.ts index 9a040a0d..03291e0b 100644 --- a/src/device-sync/__tests__/engine/real-planners.test.ts +++ b/src/device-sync/__tests__/engine/real-planners.test.ts @@ -13,6 +13,7 @@ import { CALENDAR_AUTHORITY, CONTACTS_AUTHORITY, createHarness, + renameDeviceContact, rowWrites, type Harness, } from './harness'; @@ -235,6 +236,62 @@ describe('device sync engine with the real planners', () => { }); }); + describe('a device edit the server refuses (forbidden)', () => { + it('puts a contact back in place, keeping what only the device knows', async () => { + const h = real(); + const [ada] = addServerCards(h, ['Ada Lovelace']); + await h.run(); + const before = h.contactNamed('Ada Lovelace')!; + h.device.user.star(before.id); + renameDeviceContact(h, before.id, 'Ada King'); + h.server.setErrorFor('ContactCard', 'a', ada, { type: 'forbidden' }); + + const report = await h.run(); + + expect(report.itemErrors).toEqual([expect.objectContaining({ ref: `a/${ada}`, type: 'forbidden' })]); + expect(h.server.get('ContactCard', 'a', ada)).toMatchObject({ name: { full: 'Ada Lovelace' } }); + expect(h.contacts()).toEqual([expect.objectContaining({ id: before.id, name: 'Ada Lovelace', dirty: false })]); + expect(h.contacts()[0].row.starred).toBe(1); + }); + + it('brings a contact back whose deletion the server refuses', async () => { + const h = real(); + const [ada] = addServerCards(h, ['Ada Lovelace']); + await h.run(); + h.device.user.deleteContact(h.contactNamed('Ada Lovelace')!.id); + h.server.setErrorFor('ContactCard', 'a', ada, { type: 'forbidden' }); + + const report = await h.run(); + + expect(report.itemErrors).toEqual([expect.objectContaining({ ref: `a/${ada}`, type: 'forbidden' })]); + expect(h.server.get('ContactCard', 'a', ada)).toBeDefined(); + expect(h.contacts()).toEqual([expect.objectContaining({ sourceId: `a/${ada}`, name: 'Ada Lovelace', dirty: false, deleted: false })]); + }); + + it('puts an event back in place', async () => { + const h = real(); + const standup = h.server.addEvent('a', { + uid: 'standup-uid', + title: 'Standup', + start: '2026-09-28T09:00:00', + duration: 'PT15M', + timeZone: 'Europe/Berlin', + calendarIds: { [h.calendar]: true }, + }); + await h.run(CALENDAR_AUTHORITY); + const row = h.events()[0]; + h.device.user.updateEvent(Number(row._id), { [Events.TITLE]: 'Mine', [Events.CUSTOM_APP_PACKAGE]: 'org.example.app' }); + h.server.setErrorFor('CalendarEvent', 'a', standup, { type: 'forbidden' }); + + const report = await h.run(CALENDAR_AUTHORITY); + + expect(report.itemErrors).toEqual([expect.objectContaining({ ref: `a/${standup}`, type: 'forbidden' })]); + expect(h.events()).toEqual([ + expect.objectContaining({ _id: row._id, title: 'Standup', dirty: 0, [Events.CUSTOM_APP_PACKAGE]: 'org.example.app' }), + ]); + }); + }); + describe('a create whose target the server deleted after the claim', () => { it('claims a new contact again for a book that exists', async () => { const h = real(); diff --git a/src/device-sync/engine/item-sync.ts b/src/device-sync/engine/item-sync.ts index b0eae42d..ee169339 100644 --- a/src/device-sync/engine/item-sync.ts +++ b/src/device-sync/engine/item-sync.ts @@ -1390,6 +1390,8 @@ export abstract class ItemSync { }]; } await this.beforePlanning(acct, [object]); + // An edit is undone in place, so what only the device keeps stays: a star, a ringtone, a joined contact, row ids. + if (objectKind === kind && !p.meta.deleted) return [this.overwriteWork(acct, kind, local, object)]; const insert = objectKind.planDownload(object, null, acct); return [{ group: concatGroups(ref, remove, insert.ops), @@ -1398,6 +1400,23 @@ export abstract class ItemSync { }]; } + /** The rows become `object` where they are and DIRTY is cleared (the planner's write of an accepted version). */ + private overwriteWork(acct: string, kind: Kind, local: unknown, object: ServerObject): Work { + return { + group: kind.planAccepted(local, object, acct).ops, + state: this.listing(kind, acct, object.id, true), + applied: () => this.onDownloaded(kind, acct, object), + replan: async () => { + const again = await this.reload(kind, local); + return again === null ? null : this.overwriteWork(acct, kind, again, object); + }, + failed: (reason, message) => { + this.markStale(acct, object.id); + this.env.report.itemError({ ref: refOf(acct, object.id), side: 'upload', type: reason, description: message }); + }, + }; + } + /** * `invalidProperties` on `uid`: the object exists (a lost response, or the * client's automatic retry of the create). Cards name it in the error;