refactor: decide in one place whether a card is in read-only books only #34

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, ANDROID_ACCOUNT, CONTACTS_AUTHORITY, createHarness, rowWrites, type Harness } from '../engine/harness';
+import { addServerCards, ANDROID_ACCOUNT, CONTACTS_AUTHORITY, createHarness, renameDeviceContact, rowWrites, type Harness } from '../engine/harness';
 import type { SetTarget } from '../fakes/fake-jmap-server';
 import { JPEG } from './fixtures';
 
@@ -101,6 +101,22 @@ describe('contacts through the engine', () => {
     expect(await h.teardown(CONTACTS_AUTHORITY)).toEqual({ pending: 0 });
   });
 
+  it('puts back an edit of a card that is only in a read-only book, without asking the server', async () => {
+    const h = real();
+    const shared = h.server.addAddressBook('a', { name: 'Shared', myRights: { mayRead: true, mayWrite: false, mayShare: false, mayDelete: false } });
+    h.prefs.contactsSelection[`a/${shared}`] = true;
+    const [ada] = addServerCards(h, ['Ada Lovelace'], 'a', shared);
+    expect((await h.run()).outcome).toBe('ok');
+
+    renameDeviceContact(h, h.contactNamed('Ada Lovelace')!.id, 'Ada King');
+    const report = await h.run();
+
+    expect(report).toMatchObject({ outcome: 'ok', stats: { skipped: 1, uploaded: { updated: 0 } } });
+    expect(report.itemErrors).toEqual([expect.objectContaining({ side: 'upload', type: 'readOnly' })]);
+    expect(h.contactNamed('Ada Lovelace')).toMatchObject({ dirty: false });
+    expect(h.server.get('ContactCard', 'a', ada)).toMatchObject({ name: { full: 'Ada Lovelace' } });
+  });
+
   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 } });

src/device-sync/engine/contacts-sync.ts

@@ -265,8 +265,7 @@ export class ContactsSync extends ItemSync {
     for (const group of await this.loadGroupsWhere()) {
       if (!group.sourceId || group.deleted) continue;
       this.groupRows.set(group.sourceId, group.groupId);
-      const keys = this.cardKeys(accountOfRef(group.sourceId), group.shadow);
-      if (keys.length > 0 && keys.every((k) => this.book(k)?.myRights?.mayWrite === false)) this.readOnlyGroups.add(group.sourceId);
+      if (this.inReadOnlyBooksOnly(accountOfRef(group.sourceId), group.shadow)) this.readOnlyGroups.add(group.sourceId);
       for (const uid of onIds(group.shadow?.members as Record<string, boolean> | undefined)) {
         let set = this.membership.get(uid);
         if (!set) this.membership.set(uid, (set = new Set()));
@@ -293,6 +292,12 @@ export class ContactsSync extends ItemSync {
     return !!book && book.myRights?.mayWrite !== false;
   }
 
+  /** Whether the card is in address books, and only in ones that take no writes. */
+  private inReadOnlyBooksOnly(acct: string | null, card: Pick<ContactCardWire, 'addressBookIds'> | null | undefined): boolean {
+    const keys = this.cardKeys(acct, card);
+    return keys.length > 0 && keys.every((k) => this.book(k)?.myRights?.mayWrite === false);
+  }
+
   protected collectionGone(key: string): boolean {
     const parsed = parseCollectionKey(key);
     const books = parsed ? this.books.get(parsed.accountId) : undefined;
@@ -373,10 +378,7 @@ export class ContactsSync extends ItemSync {
       mintKey: (taken) => env.mintKey(taken),
       mintUid: () => env.mintUid(),
       selectedCollections: (card) => this.cardKeys(acct, card).filter((k) => this.syncedKeys.has(k)),
-      isReadOnly: (card) => {
-        const keys = this.cardKeys(acct, card);
-        return keys.length > 0 && keys.every((k) => this.book(k)?.myRights?.mayWrite === false);
-      },
+      isReadOnly: (card) => this.inReadOnlyBooksOnly(acct, card),
       isSelected: (key) => this.syncedKeys.has(key),
       createTarget: () => this.createTarget(),
       photoBytes: (entry) => this.photoBytes(entry),