Merge pull request #228 from Coffey-Labs/feat/vcard-uid-dedupe

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

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 an address book in LDIF, read against Mozilla's schema. */
-  importLdif(text: string, addressBookId: Id): Promise<number>;
+  /** Import vCards, skipping any whose UID this book already holds. */
+  importVCard(text: string, addressBookId: Id): Promise<{ created: number; skipped: 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 {