fix: keep the groups of a contact created on the device #34

A contact created on the device and put into a group before its first
sync lost that group: the write after its create took the rows' own
memberships as what the server already had, found them missing there
and deleted the rows, so the group card was never patched.

The create's write now starts from no memberships, as its fallback
already did: the device's memberships count as additions, stay on the
device, keep the contact dirty until the group cards show them, and
upload as patches of the group cards.

Checked with a planner test and end to end through the engine.

src/device-sync/__tests__/contacts/groups.test.ts

@@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest';
 import { Data, Groups, MimeType } from '../../android-columns';
 import type { ContactCardWire, OpGroup, UploadPlan } from '../../planner';
 import { appleCard, googleCard } from './fixtures';
-import { Harness, JMAP } from './harness';
+import { ACCOUNT, Harness, JMAP } from './harness';
 
 function actionsOf(plan: UploadPlan<ContactCardWire>) {
   if (plan.kind !== 'upload') throw new Error(`expected an upload, got ${plan.kind}`);
@@ -130,6 +130,28 @@ describe('contacts groups: membership edits', () => {
     });
   });
 
+  it('keeps the groups of a contact created on the device and adds it to their cards', async () => {
+    const h = new Harness();
+    const gid = h.addCard({ uid: 'urn:uuid:g7', kind: 'group', name: { full: 'Team' }, members: { 'urn:uuid:someone': true } });
+    await h.download(gid);
+    const g = await localGroup(h, gid);
+    const rawId = h.device.user.insertContact(ACCOUNT, [
+      { [Data.MIMETYPE]: MimeType.STRUCTURED_NAME, data1: 'New Person', data2: 'New', data3: 'Person' },
+      { [Data.MIMETYPE]: MimeType.GROUP_MEMBERSHIP, [Data.DATA1]: g.groupId },
+    ]);
+    await h.upload(rawId);
+    expect(h.membershipGroups(rawId)).toEqual([`${JMAP}/${gid}`]);
+    expect(h.dirty(rawId)).toBe(true);
+    const uid = (await h.contact(rawId)).shadow!.uid as string;
+    const actions = h.planner.planMembershipUploads([await h.contact(rawId)], await h.groups(), h.ctx);
+    expect(actions).toEqual([{ kind: 'update', id: gid, patch: { [`members/${uid}`]: true } }]);
+    h.send(actions);
+    await h.applyOk(h.planner.planGroupAccepted(g, h.card(gid), h.ctx).ops);
+    await h.upload(rawId);
+    expect(h.dirty(rawId)).toBe(false);
+    expect(h.membershipGroups(rawId)).toEqual([`${JMAP}/${gid}`]);
+  });
+
   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' } });

src/device-sync/__tests__/contacts/real-engine.test.ts

@@ -6,7 +6,7 @@ import { describe, expect, it } from 'vitest';
 import { Data, Groups, MimeType } from '../../android-columns';
 import { calendarPlanner } from '../../calendar/planner';
 import { contactsPlanner } from '../../contacts/planner';
-import { addServerCards, CONTACTS_AUTHORITY, createHarness, rowWrites, type Harness } from '../engine/harness';
+import { addServerCards, ANDROID_ACCOUNT, CONTACTS_AUTHORITY, createHarness, rowWrites, type Harness } from '../engine/harness';
 import type { SetTarget } from '../fakes/fake-jmap-server';
 import { JPEG } from './fixtures';
 import { thumbnailOnlyPort } from './harness';
@@ -80,6 +80,21 @@ describe('contacts through the engine', () => {
     expect(await h.teardown(CONTACTS_AUTHORITY)).toEqual({ pending: 0 });
   });
 
+  it('puts a contact created on the device into the group it was added to', async () => {
+    const h = real();
+    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 id = h.device.user.insertContact(ANDROID_ACCOUNT, [
+      { [Data.MIMETYPE]: MimeType.STRUCTURED_NAME, [Data.DATA1]: 'New Person' },
+      { [Data.MIMETYPE]: MimeType.GROUP_MEMBERSHIP, [Data.DATA1]: Number(h.device.rows('groups')[0]._id) },
+    ]);
+
+    expect(await h.run()).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { created: 1, updated: 1 } } });
+    const uid = h.server.all('ContactCard', 'a').find((c) => c.kind !== 'group')!.uid as string;
+    expect(h.server.get('ContactCard', 'a', group)?.members).toEqual({ other: true, [uid]: true });
+    expect(memberships(h, id)).toHaveLength(1);
+  });
+
   it('keeps one row for a small photo the provider stores as its thumbnail, and never deletes it on the server', async () => {
     const h = real();
     const base = h.deps.provider;

src/device-sync/contacts/download.ts

@@ -239,6 +239,8 @@ export interface CleanWriteOptions {
   raw?: WriteRow;
   /** The rows' changes were just uploaded (planAccepted): a changed photo is the server's now. */
   accepted?: boolean;
+  /** The memberships the rows held at the last write, when there is no shadow to say: a new contact's are none. */
+  memberBase?: string[];
 }
 
 /** The JMAP account a local contact belongs to. */
@@ -260,7 +262,7 @@ export function cleanWrite(local: LocalContact, card: ContactCardWire, ctx: Ctx,
   const rows = cleanRows(local, remote, keyMap, base, options.accepted ?? false);
   const members = planMemberships({
     rows: local.rows,
-    base: shadowMemberOf(local.shadow),
+    base: shadowMemberOf(local.shadow) ?? options.memberBase ?? null,
     remote: remoteMemberships(server, ctx),
     evidence: rows.evidence,
     restore: true,

src/device-sync/contacts/upload.ts

@@ -280,7 +280,9 @@ export function planContactAccepted(local: LocalContact, server: ContactCardWire
   }
   const identity = objectRef(accountOf(local, ctx), server.id);
   const raw = { [RawContacts.SOURCE_ID]: identity, [RawContacts.SYNC3]: null, [RawContacts.SYNC4]: null };
-  const write = cleanWrite(local, server, ctx, { assertDirty: null, clearDirty: true, accepted: true, raw });
+  // Just created: the groups it was put in on the device are all still to upload.
+  const memberBase = local.shadow ? undefined : [];
+  const write = cleanWrite(local, server, ctx, { assertDirty: null, clearDirty: true, accepted: true, raw, memberBase });
   const ops = group(local, write.ops.length ? write.ops : [assertContact(local, null)]);
   ops.ref = identity;