diff --git a/src/device-sync/__tests__/contacts/groups.test.ts b/src/device-sync/__tests__/contacts/groups.test.ts index c83c2637..d9ac7869 100644 --- a/src/device-sync/__tests__/contacts/groups.test.ts +++ b/src/device-sync/__tests__/contacts/groups.test.ts @@ -111,6 +111,25 @@ describe('contacts groups: membership edits', () => { expect(h.planner.planMembershipUploads([await h.contact(rawId)], await h.groups(), h.ctx)).toEqual([]); }); + it('keeps a rename the server does not have when a membership patch of the group is accepted', async () => { + const h = new Harness(); + const gid = h.addCard({ uid: 'urn:uuid:g2', kind: 'group', name: { full: 'Work' }, members: { 'urn:uuid:someone': true } }); + await h.download(gid); + const { rawId } = await h.seed(appleCard()); + const g = await localGroup(h, gid); + h.device.user.addToGroup(rawId, g.groupId); + await h.upload(rawId); + // Renamed after the group's own upload was planned, before the membership phase read it. + h.device.user.updateGroup(g.groupId, { [Groups.TITLE]: 'Colleagues' }); + const read = await localGroup(h, gid); + h.send(h.planner.planMembershipUploads([await h.contact(rawId)], await h.groups(), h.ctx)); + await h.applyOk(h.planner.planGroupAccepted(read, h.card(gid), h.ctx).ops); + expect(await localGroup(h, gid)).toMatchObject({ title: 'Colleagues', dirty: true, shadow: { members: { [appleCard().uid as string]: true } } }); + expect(h.planner.planGroupUpload(await localGroup(h, gid), h.ctx)).toEqual({ + kind: 'upload', actions: [{ kind: 'update', id: gid, patch: { 'name/full': 'Colleagues' } }], + }); + }); + it('sends the whole map for the first member of an empty group', async () => { const h = new Harness(); const empty = h.addCard({ uid: 'urn:uuid:g3', kind: 'group', name: { full: 'Empty' } }); diff --git a/src/device-sync/__tests__/contacts/real-engine.test.ts b/src/device-sync/__tests__/contacts/real-engine.test.ts index 79207b23..758b5e3c 100644 --- a/src/device-sync/__tests__/contacts/real-engine.test.ts +++ b/src/device-sync/__tests__/contacts/real-engine.test.ts @@ -3,10 +3,11 @@ // themselves are tested in the other files of this folder. import { describe, expect, it } from 'vitest'; -import { MimeType } from '../../android-columns'; +import { Groups, MimeType } from '../../android-columns'; import { calendarPlanner } from '../../calendar/planner'; import { contactsPlanner } from '../../contacts/planner'; import { addServerCards, CONTACTS_AUTHORITY, createHarness, type Harness } from '../engine/harness'; +import type { SetTarget } from '../fakes/fake-jmap-server'; function real(options: Parameters[0] = {}): Harness { const h = createHarness(options); @@ -76,4 +77,28 @@ describe('contacts through the engine', () => { // Turning sync off finds nothing waiting. expect(await h.teardown(CONTACTS_AUTHORITY)).toEqual({ pending: 0 }); }); + + it('keeps a group rename the server refused when a membership patch of that group goes through', async () => { + const h = real(); + const [ada] = addServerCards(h, ['Ada Lovelace']); + const group = h.server.addCard('a', { uid: 'g1', kind: 'group', name: { full: 'Friends' }, members: { other: true }, addressBookIds: { [h.book]: true } }); + expect((await h.run()).outcome).toBe('ok'); + const row = () => h.device.rows('groups').find((g) => g.sourceid === `a/${group}`)!; + const groupId = Number(row()._id); + h.device.user.updateGroup(groupId, { [Groups.TITLE]: 'Best friends' }); + h.device.user.addToGroup(h.contactNamed('Ada Lovelace')!.id, groupId); + const rename = (t: SetTarget) => t.op === 'update' && t.id === group && !!t.object && 'name/full' in t.object; + h.server.setErrorFor('ContactCard', 'a', rename, { type: 'invalidProperties', properties: ['name'] }); + + const report = await h.run(); + expect(report.itemErrors).toMatchObject([{ ref: `a/${group}`, type: 'invalidProperties' }]); + const uid = h.server.get('ContactCard', 'a', ada)!.uid as string; + expect(h.server.get('ContactCard', 'a', group)).toMatchObject({ name: { full: 'Friends' }, members: { other: true, [uid]: true } }); + expect(row()).toMatchObject({ [Groups.TITLE]: 'Best friends', [Groups.DIRTY]: 1 }); + + // The rename is still the device's and goes up once the server takes it. + expect((await h.run()).outcome).toBe('ok'); + expect(h.server.get('ContactCard', 'a', group)).toMatchObject({ name: { full: 'Best friends' } }); + expect(row()).toMatchObject({ [Groups.TITLE]: 'Best friends', [Groups.DIRTY]: 0 }); + }); }); diff --git a/src/device-sync/contacts/groups.ts b/src/device-sync/contacts/groups.ts index c9d0b3f7..45829807 100644 --- a/src/device-sync/contacts/groups.ts +++ b/src/device-sync/contacts/groups.ts @@ -193,12 +193,17 @@ export function planGroupAccepted(local: LocalGroup, server: ContactCardWire, ct const identity = objectRef(account, server.id); const raw: WriteRow = { [Groups.SOURCE_ID]: identity, [Groups.SYNC3]: null, [Groups.SYNC4]: null, ...cardColumns(server, ctx) }; const title = groupTitle(server); + const keepDirtyOps = { ref: identity, ops: [updateGroup(local, raw)] }; + // A rename the server still lacks (the upload it accepted was another one, a membership + // patch) stays the device's: title and DIRTY are kept, and it uploads against the new shadow. + const shadowTitle = groupTitle(local.shadow); + const renamed = local.dirty && !!local.title?.trim() && !sameCell(local.title, shadowTitle); + if (renamed && sameCell(title, shadowTitle)) { + return { ops: { ref: identity, ops: [assertGroup(local, null), updateGroup(local, raw)] }, keepDirtyOps }; + } const ops: WriteRow = { ...raw, [Groups.DIRTY]: 0 }; if (!sameCell(local.title, title)) ops[Groups.TITLE] = title ?? ''; - return { - ops: { ref: identity, ops: [assertGroup(local, null), updateGroup(local, ops)] }, - keepDirtyOps: { ref: identity, ops: [updateGroup(local, raw)] }, - }; + return { ops: { ref: identity, ops: [assertGroup(local, null), updateGroup(local, ops)] }, keepDirtyOps }; } /**