Skip vCards on re-import that the address book already has

The contacts half of the rule that shipped for events, and only the half that
can be decided. A vCard carries a UID its author meant, so a card whose UID
this book already holds is that card, and re-importing an export left a second
copy of every one of them. Reported on #174 by the reporter's colleague, and
decided on #173: skip on a UID that is already here, import what arrives
without one, since nothing can be matched on an identity that is not there.

LDIF is deliberately untouched and now says so in the type. Mozilla's schema
defines no UID and the dn is not an identity outside the directory it came
from, so the import invents a UID that can never match one already present.
Guessing instead from a name and an address is the open question on #223, and
a guess that merges two people who share a name is worse than a duplicate
somebody can see and delete.

Both imports answer with the same shape, so a caller does not have to know
which one it called. LDIF's skipped is always 0, which is the honest number
rather than a missing field.

The UIDs are asked of the server rather than read from the cards in the store.
The store's copy is complete once the view has loaded, and importing does not
wait for a view.

Two callers, two messages. The contacts import reports both counts, as the
calendar import does: "Imported 3 contacts" over a file of two hundred reads
as a failure when the rest were already here. And a vCard attached to a
message -- usually one you have been sent before -- now says it is already in
your contacts rather than reporting that it added none.

Refs #223; the LDIF half stays open.

web/src/locales/de.ts

@@ -1078,6 +1078,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "Hier ist nichts ungelesen",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { one: "Bereits vorhanden: {n} Kontakt, nichts importiert", other: "Bereits vorhanden: {n} Kontakte, nichts importiert" },
+    "All {n} are already in your contacts": { one: "Bereits in Ihren Kontakten", other: "Alle {n} sind bereits in Ihren Kontakten" },
     "Exported {n} events": { one: "{n} Termin exportiert", other: "{n} Termine exportiert" },
     "Imported {n} events": { one: "{n} Termin importiert", other: "{n} Termine importiert" },
     "Already here: {n} events, nothing imported": { one: "Bereits vorhanden: {n} Termin, nichts importiert", other: "Bereits vorhanden: {n} Termine, nichts importiert" },

web/src/locales/es.ts

@@ -1051,6 +1051,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "Aquí no hay nada sin leer",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { one: "Ya estaba aquí: {n} contacto, no se importó nada", other: "Ya estaban aquí: {n} contactos, no se importó nada" },
+    "All {n} are already in your contacts": { one: "Ya está en tus contactos", other: "Los {n} ya están en tus contactos" },
     "Exported {n} events": { one: "{n} evento exportado", other: "{n} eventos exportados" },
     "Imported {n} events": { one: "{n} evento importado", other: "{n} eventos importados" },
     "Already here: {n} events, nothing imported": { one: "Ya estaba aquí: {n} evento, no se importó nada", other: "Ya estaban aquí: {n} eventos, no se importó nada" },

web/src/locales/fr.ts

@@ -1056,6 +1056,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "Rien de non lu ici",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { one: "Déjà présent : {n} contact, rien d’importé", other: "Déjà présents : {n} contacts, rien d’importé" },
+    "All {n} are already in your contacts": { one: "Déjà dans vos contacts", other: "Les {n} sont déjà dans vos contacts" },
     "Exported {n} events": { one: "{n} événement exporté", other: "{n} événements exportés" },
     "Imported {n} events": { one: "{n} événement importé", other: "{n} événements importés" },
     "Already here: {n} events, nothing imported": { one: "Déjà présent : {n} événement, rien d’importé", other: "Déjà présents : {n} événements, rien d’importé" },

web/src/locales/ja.ts

@@ -1059,6 +1059,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "ここに未読はありません",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { other: "すでに存在: {n} 件、インポートなし" },
+    "All {n} are already in your contacts": { other: "{n} 件はすでに連絡先にあります" },
     "Exported {n} events": { other: "{n} 件の予定をエクスポートしました" },
     "Imported {n} events": { other: "{n} 件の予定をインポートしました" },
     "Already here: {n} events, nothing imported": { other: "すでに存在: {n} 件、インポートなし" },

web/src/locales/nl.ts

@@ -1047,6 +1047,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "Hier is niets ongelezen",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { one: "Al aanwezig: {n} contact, niets geïmporteerd", other: "Al aanwezig: {n} contacten, niets geïmporteerd" },
+    "All {n} are already in your contacts": { one: "Staat al in uw contacten", other: "Alle {n} staan al in uw contacten" },
     "Exported {n} events": { one: "{n} afspraak geëxporteerd", other: "{n} afspraken geëxporteerd" },
     "Imported {n} events": { one: "{n} afspraak geïmporteerd", other: "{n} afspraken geïmporteerd" },
     "Already here: {n} events, nothing imported": { one: "Al aanwezig: {n} afspraak, niets geïmporteerd", other: "Al aanwezig: {n} afspraken, niets geïmporteerd" },

web/src/locales/pt-BR.ts

@@ -1054,6 +1054,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "Não há nada não lido aqui",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { one: "Já estava aqui: {n} contato, nada importado", other: "Já estavam aqui: {n} contatos, nada importado" },
+    "All {n} are already in your contacts": { one: "Já está nos seus contatos", other: "Todos os {n} já estão nos seus contatos" },
     "Exported {n} events": { one: "{n} evento exportado", other: "{n} eventos exportados" },
     "Imported {n} events": { one: "{n} evento importado", other: "{n} eventos importados" },
     "Already here: {n} events, nothing imported": { one: "Já estava aqui: {n} evento, nada importado", other: "Já estavam aqui: {n} eventos, nada importado" },

web/src/locales/ru.ts

@@ -1053,6 +1053,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "Здесь нет непрочитанного",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { one: "Уже есть: {n} контакт, ничего не импортировано", few: "Уже есть: {n} контакта, ничего не импортировано", many: "Уже есть: {n} контактов, ничего не импортировано", other: "Уже есть: {n} контакта, ничего не импортировано" },
+    "All {n} are already in your contacts": { one: "Уже в ваших контактах", few: "Все {n} уже в ваших контактах", many: "Все {n} уже в ваших контактах", other: "Все {n} уже в ваших контактах" },
     "Exported {n} events": { one: "Экспортировано {n} событие", few: "Экспортировано {n} события", many: "Экспортировано {n} событий", other: "Экспортировано {n} события" },
     "Imported {n} events": { one: "Импортировано {n} событие", few: "Импортировано {n} события", many: "Импортировано {n} событий", other: "Импортировано {n} события" },
     "Already here: {n} events, nothing imported": { one: "Уже есть: {n} событие, ничего не импортировано", few: "Уже есть: {n} события, ничего не импортировано", many: "Уже есть: {n} событий, ничего не импортировано", other: "Уже есть: {n} события, ничего не импортировано" },

web/src/locales/uk.ts

@@ -1047,6 +1047,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "Тут немає непрочитаного",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { one: "Уже є: {n} контакт, нічого не імпортовано", few: "Уже є: {n} контакти, нічого не імпортовано", many: "Уже є: {n} контактів, нічого не імпортовано", other: "Уже є: {n} контакти, нічого не імпортовано" },
+    "All {n} are already in your contacts": { one: "Уже у ваших контактах", few: "Усі {n} уже у ваших контактах", many: "Усі {n} уже у ваших контактах", other: "Усі {n} уже у ваших контактах" },
     "Exported {n} events": { one: "Експортовано {n} подію", few: "Експортовано {n} події", many: "Експортовано {n} подій", other: "Експортовано {n} події" },
     "Imported {n} events": { one: "Імпортовано {n} подію", few: "Імпортовано {n} події", many: "Імпортовано {n} подій", other: "Імпортовано {n} події" },
     "Already here: {n} events, nothing imported": { one: "Уже є: {n} подія, нічого не імпортовано", few: "Уже є: {n} події, нічого не імпортовано", many: "Уже є: {n} подій, нічого не імпортовано", other: "Уже є: {n} події, нічого не імпортовано" },

web/src/locales/zh-Hans.ts

@@ -1058,6 +1058,8 @@ export const catalog: Catalog = {
     "Nothing unread here": "这里没有未读邮件",
   },
   plurals: {
+    "Already here: {n} contacts, nothing imported": { other: "已存在 {n} 个，未导入" },
+    "All {n} are already in your contacts": { other: "这 {n} 个已在您的联系人中" },
     "Exported {n} events": { other: "已导出 {n} 个日程" },
     "Imported {n} events": { other: "已导入 {n} 个日程" },
     "Already here: {n} events, nothing imported": { other: "已存在 {n} 个，未导入" },

web/src/store/__tests__/contact-set-batching.test.ts

@@ -100,14 +100,14 @@ afterEach(() => {
 describe("importing an LDIF bigger than the server will take at once", () => {
   it("splits it into calls the server will accept, and files all of it", async () => {
     const sets = server({ max: MAX });
-    await expect(useContacts.getState().importLdif(ldifOf(1200), "book1")).resolves.toBe(1200);
+    await expect(useContacts.getState().importLdif(ldifOf(1200), "book1")).resolves.toEqual({ created: 1200, skipped: 0 });
     expect(sizes(sets)).toEqual([500, 500, 200]);
   });
 
   it("splits by what the session advertises, not by a number of its own", async () => {
     client.session!.capabilities[CAP.core] = { maxObjectsInGet: 40, maxObjectsInSet: 40 };
     const sets = server({ max: 40 });
-    await expect(useContacts.getState().importLdif(ldifOf(100), "book1")).resolves.toBe(100);
+    await expect(useContacts.getState().importLdif(ldifOf(100), "book1")).resolves.toEqual({ created: 100, skipped: 0 });
     expect(sizes(sets)).toEqual([40, 40, 20]);
   });
 
@@ -134,7 +134,7 @@ describe("importing an LDIF bigger than the server will take at once", () => {
 describe("importing a vCard file bigger than the server will take at once", () => {
   it("splits it into calls the server will accept, and files all of it", async () => {
     const sets = server({ max: MAX, parsed: vcardsOf(1200) });
-    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toBe(1200);
+    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1200, skipped: 0 });
     expect(sizes(sets)).toEqual([500, 500, 200]);
   });
 
@@ -155,7 +155,7 @@ describe("importing a vCard file bigger than the server will take at once", () =
 
   it("counts what got in when only some of it did", async () => {
     server({ max: MAX, parsed: vcardsOf(2), notCreated: { c1: { type: "invalidProperties" } } });
-    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toBe(1);
+    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0 });
   });
 });
 

web/src/store/__tests__/ldif-import.test.ts

@@ -63,7 +63,7 @@ describe("importing an LDIF address book", () => {
   it("creates every entry in one call, not one call each", async () => {
     const sets = server();
     const n = await useContacts.getState().importLdif(TWO, "book1");
-    expect(n).toBe(2);
+    expect(n).toEqual({ created: 2, skipped: 0 });
     expect(sets).toHaveLength(1);
     expect(Object.keys(sets[0]!.create!)).toEqual(["c0", "c1"]);
   });
@@ -105,7 +105,7 @@ describe("importing an LDIF address book", () => {
   it("skips entries too empty to be a person, and imports the rest", async () => {
     const sets = server();
     const n = await useContacts.getState().importLdif(`${TWO}\ndn: cn=Nobody\nobjectClass: top\n`, "book1");
-    expect(n).toBe(2);
+    expect(n).toEqual({ created: 2, skipped: 0 });
     expect(Object.keys(sets[0]!.create!)).toHaveLength(2);
   });
 
@@ -116,6 +116,6 @@ describe("importing an LDIF address book", () => {
 
   it("counts what got in when only some of it did", async () => {
     server({ notCreated: { c1: { type: "invalidProperties" } } });
-    await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toBe(1);
+    await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 1, skipped: 0 });
   });
 });

web/src/store/__tests__/vcard-dedupe.test.ts

@@ -0,0 +1,121 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { CAP, client } from "@/jmap/client";
+import { useContacts } from "@/store/contacts";
+import type { ContactCard, JmapSession, UploadResponse } from "@/jmap/types";
+
+/*
+ * Re-importing an address book you already have.
+ *
+ * A vCard UID is an identity its author meant, so a card whose UID a book
+ * already holds is that card and importing it again used to leave a second
+ * copy. Reported on #174 by the reporter's colleague, decided on #173 for
+ * events, tracked as #223. The LDIF half is deliberately absent -- Mozilla's
+ * schema has no UID, so the import invents one and there is nothing to match.
+ */
+
+const MAX = 500;
+
+interface SetArgs { create?: Record<string, Record<string, unknown>> }
+
+function server(opts: { parsed?: unknown[]; existing?: Array<{ id: string; uid: string; addressBookIds: Record<string, boolean> }> } = {}) {
+  const sets: SetArgs[] = [];
+  const existing = opts.existing ?? [];
+  const fetchMock = vi.fn(async (_url: string, init: RequestInit) => {
+    const body = JSON.parse(init.body as string) as { methodCalls: [string, Record<string, unknown>, string][] };
+    const methodResponses = body.methodCalls.map(([name, args, id]) => {
+      if (name === "ContactCard/parse") {
+        const blobId = (args.blobIds as string[])[0]!;
+        return [name, { accountId: "a1", parsed: { [blobId]: opts.parsed ?? [] }, notParsable: [] }, id];
+      }
+      if (name === "ContactCard/query") {
+        const position = (args.position as number) ?? 0;
+        return [name, { accountId: "a1", queryState: "1", canCalculateChanges: false, position, ids: position ? [] : existing.map((c) => c.id), total: existing.length }, id];
+      }
+      if (name === "ContactCard/get") {
+        const want = new Set((args.ids as string[]) ?? []);
+        return [name, { accountId: "a1", state: "1", list: existing.filter((c) => want.has(c.id)), notFound: [] }, id];
+      }
+      if (name === "ContactCard/set") {
+        sets.push({ create: args.create as Record<string, Record<string, unknown>> });
+        return [name, {
+          accountId: "a1", oldState: "1", newState: "2",
+          created: Object.fromEntries(Object.keys((args.create ?? {}) as object).map((k) => [k, { id: `new-${k}` }])),
+          notCreated: {},
+        }, id];
+      }
+      return [name, { accountId: "a1", state: "1", list: [], notFound: [], ids: [], total: 0, queryState: "q", position: 0, canCalculateChanges: false }, id];
+    });
+    return { ok: true, status: 200, json: async () => ({ methodResponses, sessionState: "1" }) } as Response;
+  });
+  vi.stubGlobal("fetch", fetchMock);
+  return sets;
+}
+
+const card = (uid: string, name: string) => ({
+  "@type": "Card", version: "1.0", uid, kind: "individual", name: { full: name },
+});
+const here = (uid: string, bookId = "book1") => ({ id: `srv-${uid}`, uid, addressBookIds: { [bookId]: true } });
+
+beforeEach(() => {
+  client.session = {
+    capabilities: { [CAP.core]: { maxObjectsInGet: MAX, maxObjectsInSet: MAX }, [CAP.contacts]: {} },
+    accounts: {}, primaryAccounts: {}, state: "s1",
+  } as unknown as JmapSession;
+  useContacts.setState({ accountId: "a1", available: true, books: {}, cards: {} as Record<string, ContactCard> });
+  vi.spyOn(client, "upload").mockResolvedValue({ accountId: "a1", blobId: "blob1", type: "text/vcard", size: 1 } as UploadResponse);
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.restoreAllMocks();
+});
+
+describe("re-importing vCards the book already has", () => {
+  it("skips a card whose uid is already in this book", async () => {
+    const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")], existing: [here("ada@x")] });
+    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 1 });
+    expect(Object.values(sets[0]!.create!).map((c) => (c.name as { full: string }).full)).toEqual(["Alan"]);
+  });
+
+  it("imports a card whose uid is in a different book", async () => {
+    // The same person legitimately filed in two address books is not a
+    // duplicate, any more than the same event in two calendars is.
+    const sets = server({ parsed: [card("ada@x", "Ada")], existing: [here("ada@x", "book2")] });
+    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0 });
+    expect(Object.keys(sets[0]!.create!)).toHaveLength(1);
+  });
+
+  it("imports a card that arrived with no uid, rather than guessing at one", async () => {
+    const noUid = { "@type": "Card", version: "1.0", kind: "individual", name: { full: "Anon" } };
+    const sets = server({ parsed: [noUid], existing: [here("ada@x")] });
+    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0 });
+    expect(Object.values(sets[0]!.create!)[0]!.uid).toEqual(expect.any(String));
+  });
+
+  it("sends nothing at all when the whole file is already here", async () => {
+    const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")], existing: [here("ada@x"), here("alan@x")] });
+    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 0, skipped: 2 });
+    expect(sets).toHaveLength(0);
+  });
+
+  it("imports everything into an empty book", async () => {
+    const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")] });
+    await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 2, skipped: 0 });
+    expect(Object.keys(sets[0]!.create!)).toHaveLength(2);
+  });
+});
+
+describe("LDIF, which has nothing to match on", () => {
+  const TWO = `dn: cn=Jane Doe\ngivenName: Jane\nsn: Doe\ncn: Jane Doe\nmail: [email protected]\n\ndn: cn=Alan Turing\ngivenName: Alan\nsn: Turing\ncn: Alan Turing\nmail: [email protected]\n`;
+
+  it("imports the same file twice over, and says nothing was skipped", async () => {
+    /*
+     * Not an oversight. Mozilla's schema defines no UID and the dn is not an
+     * identity anywhere but the directory it came from, so the import invents
+     * a UID -- which can never match one already here. Whether to guess from a
+     * name and an address instead is the open question on #223.
+     */
+    server({ existing: [here("anything")] });
+    await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 2, skipped: 0 });
+  });
+});

web/src/store/contacts.ts

@@ -28,6 +28,37 @@ import { useMail } from "./mail";
  *
  * [#173]: https://github.com/Coffey-Labs/ihasmail/issues/173
  */
+/**
+ * The UIDs an address book already holds.
+ *
+ * Read once per import rather than once per card, and narrowed to the target
+ * book from `addressBookIds` here rather than through a filter -- the same
+ * arrangement, and for the same reasons, as the calendar's scan in #222.
+ *
+ * Asked of the server rather than read from the cards already in the store.
+ * The store's copy is complete once the view has loaded, and importing is not
+ * something that waits for a view: a scan that is right whatever the client
+ * happens to be holding costs one pass over a list nobody imports into twice a
+ * day.
+ */
+async function uidsInBook(accountId: Id, addressBookId: Id): Promise<Set<string>> {
+  const uids = new Set<string>();
+  const page = client.maxObjectsInGet;
+  for (let position = 0; ; ) {
+    const q = await client.call<QueryResponse>("ContactCard/query", { accountId, position, limit: page, calculateTotal: true });
+    const ids = q.ids ?? [];
+    if (!ids.length) break;
+    for (const part of chunk(ids, page)) {
+      const g = await client.call<GetResponse<ContactCard>>("ContactCard/get", { accountId, ids: part, properties: ["uid", "addressBookIds"] });
+      for (const c of g.list) if (c.uid && c.addressBookIds?.[addressBookId]) uids.add(c.uid);
+    }
+    position += ids.length;
+    // `total` is optional, so the empty page above is what actually ends this.
+    if (q.total != null && position >= q.total) break;
+  }
+  return uids;
+}
+
 async function createCards(accountId: Id, create: Record<string, unknown>): Promise<{ created: number; refused?: SetError }> {
   const keys = Object.keys(create);
   let created = 0;
@@ -122,9 +153,16 @@ interface ContactsState {
   createBook(name: string): Promise<Id>;
   updateBook(id: Id, patch: Partial<AddressBook>): Promise<void>;
   destroyBook(id: Id): Promise<void>;
-  importVCard(text: string, addressBookId: Id): Promise<number>;
+  /** Import vCards, skipping any whose UID this book already holds. */
-  /** Import an address book in LDIF, read against Mozilla's schema. */
+  importVCard(text: string, addressBookId: Id): Promise<{ created: number; skipped: number }>;
-  importLdif(text: string, addressBookId: Id): Promise<number>;
+  /**
+   * Import an address book in LDIF, read against Mozilla's schema.
+   *
+   * `skipped` is always 0: Mozilla's schema has no UID, so there is nothing to
+   * recognise a re-import by. Answered in the same shape as the vCard import so
+   * the caller does not have to know which one it called.
+   */
+  importLdif(text: string, addressBookId: Id): Promise<{ created: number; skipped: number }>;
   loadPrincipals(): Promise<void>;
   suggest(query: string, limit?: number): Promise<Suggestion[]>;
   addRecent(addrs: EmailAddress[]): void;
@@ -422,18 +460,37 @@ export const useContacts = create<ContactsState>((set, get) => ({
     const entry = parsed.parsed?.[up.blobId];
     const cards: ContactCard[] = entry ? (Array.isArray(entry) ? entry : [entry]) : [];
     if (!cards.length) throw new Error("No contacts found in file");
+    const already = await uidsInBook(accountId, addressBookId);
     const create: Record<string, unknown> = {};
+    let skipped = 0;
     cards.forEach((c, i) => {
       const { id: _id, addressBookIds: _ab, ...rest } = c as ContactCard & { id?: Id };
+      /*
+       * A vCard UID is an identity its author meant, so a card whose UID this
+       * book already holds is the same card and re-importing an export used to
+       * leave a second copy of every one of them. Asked for on #174 after the
+       * reporter's colleague hit it, and decided on #173 for events: skip on a
+       * UID that is already here, import what arrives without one, since
+       * nothing can be matched on an identity that is not there.
+       */
+      if (rest.uid && already.has(rest.uid)) {
+        skipped++;
+        return;
+      }
       create[`c${i}`] = { ...rest, uid: rest.uid || crypto.randomUUID(), addressBookIds: { [addressBookId]: true } };
     });
+    // The whole file was already here. Nothing to send, and nothing wrong.
+    if (!Object.keys(create).length) {
+      await get().loadAll();
+      return { created: 0, skipped };
+    }
     try {
       const { created, refused } = await createCards(accountId, create);
       // Nothing at all got in: say why rather than report importing none as
       // though the file had been empty. The LDIF import said this already; a
       // vCard import that quietly returned 0 was the odd one out.
       if (!created) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its contacts");
-      return created;
+      return { created, skipped };
     } finally {
       await get().loadAll();
     }
@@ -467,7 +524,13 @@ export const useContacts = create<ContactsState>((set, get) => ({
     } finally {
       await get().loadAll();
     }
-    return created;
+    /*
+     * Nothing skipped, and nothing that could be. The UID above is invented
+     * here because Mozilla's schema does not define one, so a re-import has no
+     * identity to be recognised by -- see #223, where whether to guess at one
+     * from a name and an address is still an open question.
+     */
+    return { created, skipped: 0 };
   },
 
   async loadPrincipals() {

web/src/views/contacts/ContactsView.tsx

@@ -136,10 +136,19 @@ export function ContactsView({ id }: { id?: string }) {
        * LDIF may arrive as .ldif, .ldi, .txt or with no extension at all, and
        * the name is the least reliable thing about it.
        */
-      const n = /^\s*BEGIN:VCARD/im.test(text)
+      const { created, skipped } = /^\s*BEGIN:VCARD/im.test(text)
         ? await contacts.importVCard(text, book.id)
         : await contacts.importLdif(text, book.id);
-      toast.success(plural(n, { one: "Imported {n} contact", other: "Imported {n} contacts" }));
+      /*
+       * The two counts kept apart, as the calendar import keeps them: "Imported
+       * 3 contacts" over a file of two hundred reads as a failure when the rest
+       * were simply already here, and a re-import of an unchanged export would
+       * otherwise report importing nothing at all.
+       */
+      const imported = plural(created, { one: "Imported {n} contact", other: "Imported {n} contacts" });
+      if (!created) toast.success(plural(skipped, { one: "Already here: {n} contact, nothing imported", other: "Already here: {n} contacts, nothing imported" }));
+      else if (skipped) toast.success(`${imported} · ${plural(skipped, { one: "{n} was already here", other: "{n} were already here" })}`);
+      else toast.success(imported);
     } catch (err) {
       toast.error(translate("Could not import this file: {error}", { error: (err as Error).message }));
     }

web/src/views/mail/VCardCard.tsx

@@ -17,9 +17,15 @@ export function VCardCard({ part, accountId }: { part: EmailBodyPart; accountId:
       const text = await client.fetchBlobText(accountId, part.blobId!, "text/vcard");
       const book = Object.values(contacts.books).find((b) => b.isDefault) ?? Object.values(contacts.books)[0];
       if (!book) throw new Error("No address book available");
-      const n = await contacts.importVCard(text, book.id);
+      const { created, skipped } = await contacts.importVCard(text, book.id);
       setDone(true);
-      toast.success(plural(n, { one: "Added {n} contact", other: "Added {n} contacts" }));
+      /*
+       * A card attached to a message is usually one you have already been sent
+       * once. Saying "Added 0 contacts" for that would read as a failure; it is
+       * the opposite -- there was nothing to do.
+       */
+      if (!created && skipped) toast.success(plural(skipped, { one: "Already in your contacts", other: "All {n} are already in your contacts" }));
+      else toast.success(plural(created, { one: "Added {n} contact", other: "Added {n} contacts" }));
     } catch (err) {
       toast.error((err as Error).message);
     } finally {