diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index b40e64e..8fe687b 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -1079,6 +1079,7 @@ export const catalog: Catalog = { "Nothing unread here": "Hier ist nichts ungelesen", }, plurals: { + "{n} of them look like contacts you already had": { one: "{n} davon sieht aus wie ein Kontakt, den Sie schon hatten", other: "{n} davon sehen aus wie Kontakte, die Sie schon hatten" }, "Your administrator changed {n} settings": { one: "Ihre Administration hat {n} Einstellung geändert", other: "Ihre Administration hat {n} Einstellungen geändert" }, "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" }, diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index 91b2831..d97c7cf 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -1052,6 +1052,7 @@ export const catalog: Catalog = { "Nothing unread here": "Aquí no hay nada sin leer", }, plurals: { + "{n} of them look like contacts you already had": { one: "{n} de ellos se parece a un contacto que ya tenías", other: "{n} de ellos se parecen a contactos que ya tenías" }, "Your administrator changed {n} settings": { one: "Tu administración cambió {n} ajuste", other: "Tu administración cambió {n} ajustes" }, "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" }, diff --git a/web/src/locales/fr.ts b/web/src/locales/fr.ts index a621da1..179ee17 100644 --- a/web/src/locales/fr.ts +++ b/web/src/locales/fr.ts @@ -1057,6 +1057,7 @@ export const catalog: Catalog = { "Nothing unread here": "Rien de non lu ici", }, plurals: { + "{n} of them look like contacts you already had": { one: "{n} d’entre eux ressemble à un contact que vous aviez déjà", other: "{n} d’entre eux ressemblent à des contacts que vous aviez déjà" }, "Your administrator changed {n} settings": { one: "Votre administration a modifié {n} paramètre", other: "Votre administration a modifié {n} paramètres" }, "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" }, diff --git a/web/src/locales/ja.ts b/web/src/locales/ja.ts index 0276882..dee7385 100644 --- a/web/src/locales/ja.ts +++ b/web/src/locales/ja.ts @@ -1060,6 +1060,7 @@ export const catalog: Catalog = { "Nothing unread here": "ここに未読はありません", }, plurals: { + "{n} of them look like contacts you already had": { other: "うち {n} 件はすでにある連絡先に似ています" }, "Your administrator changed {n} settings": { other: "管理者が {n} 件の設定を変更しました" }, "Already here: {n} contacts, nothing imported": { other: "すでに存在: {n} 件、インポートなし" }, "All {n} are already in your contacts": { other: "{n} 件はすでに連絡先にあります" }, diff --git a/web/src/locales/nl.ts b/web/src/locales/nl.ts index 68e6bd3..2d45772 100644 --- a/web/src/locales/nl.ts +++ b/web/src/locales/nl.ts @@ -1048,6 +1048,7 @@ export const catalog: Catalog = { "Nothing unread here": "Hier is niets ongelezen", }, plurals: { + "{n} of them look like contacts you already had": { one: "{n} daarvan lijkt op een contact dat u al had", other: "{n} daarvan lijken op contacten die u al had" }, "Your administrator changed {n} settings": { one: "Uw beheerder heeft {n} instelling gewijzigd", other: "Uw beheerder heeft {n} instellingen gewijzigd" }, "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" }, diff --git a/web/src/locales/pt-BR.ts b/web/src/locales/pt-BR.ts index 9260146..db29a4b 100644 --- a/web/src/locales/pt-BR.ts +++ b/web/src/locales/pt-BR.ts @@ -1055,6 +1055,7 @@ export const catalog: Catalog = { "Nothing unread here": "Não há nada não lido aqui", }, plurals: { + "{n} of them look like contacts you already had": { one: "{n} deles parece um contato que você já tinha", other: "{n} deles parecem contatos que você já tinha" }, "Your administrator changed {n} settings": { one: "Sua administração alterou {n} configuração", other: "Sua administração alterou {n} configurações" }, "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" }, diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index e5ce14c..ee4a535 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -1054,6 +1054,7 @@ export const catalog: Catalog = { "Nothing unread here": "Здесь нет непрочитанного", }, plurals: { + "{n} of them look like contacts you already had": { one: "{n} из них похож на контакт, который уже был", few: "{n} из них похожи на контакты, которые уже были", many: "{n} из них похожи на контакты, которые уже были", other: "{n} из них похожи на контакты, которые уже были" }, "Your administrator changed {n} settings": { one: "Администратор изменил {n} настройку", few: "Администратор изменил {n} настройки", many: "Администратор изменил {n} настроек", other: "Администратор изменил {n} настройки" }, "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} уже в ваших контактах" }, diff --git a/web/src/locales/uk.ts b/web/src/locales/uk.ts index 2df98b3..2a1244c 100644 --- a/web/src/locales/uk.ts +++ b/web/src/locales/uk.ts @@ -1048,6 +1048,7 @@ export const catalog: Catalog = { "Nothing unread here": "Тут немає непрочитаного", }, plurals: { + "{n} of them look like contacts you already had": { one: "{n} з них схожий на контакт, який уже був", few: "{n} з них схожі на контакти, які вже були", many: "{n} з них схожі на контакти, які вже були", other: "{n} з них схожі на контакти, які вже були" }, "Your administrator changed {n} settings": { one: "Адміністратор змінив {n} налаштування", few: "Адміністратор змінив {n} налаштування", many: "Адміністратор змінив {n} налаштувань", other: "Адміністратор змінив {n} налаштування" }, "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} уже у ваших контактах" }, diff --git a/web/src/locales/zh-Hans.ts b/web/src/locales/zh-Hans.ts index 330a28d..3549075 100644 --- a/web/src/locales/zh-Hans.ts +++ b/web/src/locales/zh-Hans.ts @@ -1059,6 +1059,7 @@ export const catalog: Catalog = { "Nothing unread here": "这里没有未读邮件", }, plurals: { + "{n} of them look like contacts you already had": { other: "其中 {n} 个与您已有的联系人相似" }, "Your administrator changed {n} settings": { other: "管理员更改了 {n} 项设置" }, "Already here: {n} contacts, nothing imported": { other: "已存在 {n} 个，未导入" }, "All {n} are already in your contacts": { other: "这 {n} 个已在您的联系人中" }, diff --git a/web/src/store/__tests__/contact-set-batching.test.ts b/web/src/store/__tests__/contact-set-batching.test.ts index 6cb5f03..9c536cf 100644 --- a/web/src/store/__tests__/contact-set-batching.test.ts +++ b/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.toEqual({ created: 1200, skipped: 0 }); + await expect(useContacts.getState().importLdif(ldifOf(1200), "book1")).resolves.toEqual({ created: 1200, skipped: 0, alike: 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.toEqual({ created: 100, skipped: 0 }); + await expect(useContacts.getState().importLdif(ldifOf(100), "book1")).resolves.toEqual({ created: 100, skipped: 0, alike: 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.toEqual({ created: 1200, skipped: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1200, skipped: 0, alike: 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.toEqual({ created: 1, skipped: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 }); }); }); diff --git a/web/src/store/__tests__/ldif-import.test.ts b/web/src/store/__tests__/ldif-import.test.ts index 19d3cab..3be9da6 100644 --- a/web/src/store/__tests__/ldif-import.test.ts +++ b/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).toEqual({ created: 2, skipped: 0 }); + expect(n).toEqual({ created: 2, skipped: 0, alike: 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).toEqual({ created: 2, skipped: 0 }); + expect(n).toEqual({ created: 2, skipped: 0, alike: 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.toEqual({ created: 1, skipped: 0 }); + await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 }); }); }); diff --git a/web/src/store/__tests__/ldif-likeness.test.ts b/web/src/store/__tests__/ldif-likeness.test.ts new file mode 100644 index 0000000..ab102e1 --- /dev/null +++ b/web/src/store/__tests__/ldif-likeness.test.ts @@ -0,0 +1,138 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CAP, client } from "@/jmap/client"; +import { useContacts } from "@/store/contacts"; +import type { ContactCard, JmapSession } from "@/jmap/types"; + +/* + * Counting look-alikes on an LDIF import, without acting on them. + * + * Mozilla's schema has no UID, so the import invents one and a re-import + * duplicates everything. Whether to guess an identity from a name and an + * address is still open on #223 -- and the harm that was actually reported was + * confusion rather than duplication: somebody imports a file twice and cannot + * tell what happened. So this counts and says so, and imports every card + * regardless. Reporting is not matching. + */ + +interface SetArgs { create?: Record> } + +function server(existing: Array & { id: string }>) { + const sets: SetArgs[] = []; + const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { + const body = JSON.parse(init.body as string) as { methodCalls: [string, Record, string][] }; + const methodResponses = body.methodCalls.map(([name, args, 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> }); + 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; +} + +/** A card already in the book, with the two fields likeness is read from. */ +const card = (id: string, full: string, ...emails: string[]) => ({ + id, uid: `uid-${id}`, addressBookIds: { book1: true }, + name: { full }, + emails: Object.fromEntries(emails.map((address, i) => [`e${i}`, { address }])), +}) as unknown as Partial & { id: string }; + +const entry = (cn: string, mail: string) => + `dn: cn=${cn}\ngivenName: ${cn.split(" ")[0]}\nsn: ${cn.split(" ").slice(-1)[0]}\ncn: ${cn}\nmail: ${mail}\n`; + +beforeEach(() => { + client.session = { + capabilities: { [CAP.core]: { maxObjectsInGet: 500, maxObjectsInSet: 500 }, [CAP.contacts]: {} }, + accounts: {}, primaryAccounts: {}, state: "s1", + } as unknown as JmapSession; + useContacts.setState({ accountId: "a1", available: true, books: {}, cards: {} as Record }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("telling somebody what an LDIF re-import duplicated", () => { + it("counts an entry that matches an existing card on name and address", async () => { + server([card("c1", "Jane Doe", "jane@example.com")]); + const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(r).toEqual({ created: 1, skipped: 0, alike: 1 }); + }); + + it("imports it anyway, which is the whole point of counting rather than matching", async () => { + const sets = server([card("c1", "Jane Doe", "jane@example.com")]); + await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(Object.keys(sets[0]!.create!)).toHaveLength(1); + }); + + it("does not count a name match with a different address", async () => { + // Two people who share a name are two people. This is exactly the guess + // the counting refuses to make on anyone's behalf. + server([card("c1", "Jane Doe", "jane.doe@other.example")]); + const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(r.alike).toBe(0); + }); + + it("does not count an address match under a different name", async () => { + server([card("c1", "Someone Else", "jane@example.com")]); + const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(r.alike).toBe(0); + }); + + it("recognises a match on a second address", async () => { + server([card("c1", "Jane Doe", "old@example.com", "jane@example.com")]); + const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(r.alike).toBe(1); + }); + + it("ignores case and spacing, which an export and a hand-typed card differ in", async () => { + server([card("c1", " JANE DOE ", "Jane@Example.com")]); + const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(r.alike).toBe(1); + }); + + it("counts each entry once however many of its addresses match", async () => { + server([card("c1", "Jane Doe", "jane@example.com", "j@example.com")]); + const two = `dn: cn=Jane Doe\ncn: Jane Doe\nmail: jane@example.com\nmozillaSecondEmail: j@example.com\n`; + const r = await useContacts.getState().importLdif(two, "book1"); + expect(r.alike).toBe(1); + }); + + it("looks only at the book being imported into", async () => { + const elsewhere = { ...card("c1", "Jane Doe", "jane@example.com"), addressBookIds: { book2: true } }; + server([elsewhere]); + const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(r.alike).toBe(0); + }); + + it("counts nothing against an empty book", async () => { + server([]); + const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); + expect(r).toEqual({ created: 1, skipped: 0, alike: 0 }); + }); + + it("does not count the file against itself", async () => { + // Two of the same person in one file are two new cards, not a duplicate of + // something that was already here. The scan is read before anything lands. + server([]); + const twice = entry("Jane Doe", "jane@example.com") + "\n" + entry("Jane Doe", "jane@example.com"); + const r = await useContacts.getState().importLdif(twice, "book1"); + expect(r).toEqual({ created: 2, skipped: 0, alike: 0 }); + }); +}); diff --git a/web/src/store/__tests__/vcard-dedupe.test.ts b/web/src/store/__tests__/vcard-dedupe.test.ts index 5cd4e60..4bfb1ab 100644 --- a/web/src/store/__tests__/vcard-dedupe.test.ts +++ b/web/src/store/__tests__/vcard-dedupe.test.ts @@ -73,7 +73,7 @@ afterEach(() => { 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 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 1, alike: 0 }); expect(Object.values(sets[0]!.create!).map((c) => (c.name as { full: string }).full)).toEqual(["Alan"]); }); @@ -81,26 +81,26 @@ describe("re-importing vCards the book already has", () => { // 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 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 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 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 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 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 0, skipped: 2, alike: 0 }); 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 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 2, skipped: 0, alike: 0 }); expect(Object.keys(sets[0]!.create!)).toHaveLength(2); }); }); @@ -116,6 +116,6 @@ describe("LDIF, which has nothing to match on", () => { * 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 }); + await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 2, skipped: 0, alike: 0 }); }); }); diff --git a/web/src/store/contacts.ts b/web/src/store/contacts.ts index e19c969..2f08567 100644 --- a/web/src/store/contacts.ts +++ b/web/src/store/contacts.ts @@ -41,22 +41,53 @@ import { useMail } from "./mail"; * happens to be holding costs one pass over a list nobody imports into twice a * day. */ -async function uidsInBook(accountId: Id, addressBookId: Id): Promise> { +async function scanBook(accountId: Id, addressBookId: Id): Promise<{ uids: Set; likeness: Set }> { const uids = new Set(); + const likeness = new Set(); const page = client.maxObjectsInGet; for (let position = 0; ; ) { const q = await client.call("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>("ContactCard/get", { accountId, ids: part, properties: ["uid", "addressBookIds"] }); - for (const c of g.list) if (c.uid && c.addressBookIds?.[addressBookId]) uids.add(c.uid); + const g = await client.call>("ContactCard/get", { accountId, ids: part, properties: ["uid", "addressBookIds", "name", "emails"] }); + for (const c of g.list) { + if (!c.addressBookIds?.[addressBookId]) continue; + if (c.uid) uids.add(c.uid); + for (const key of likenessKeys(c)) likeness.add(key); + } } 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; + return { uids, likeness }; +} + +/** + * What makes two cards *look* like the same person -- name and one address. + * + * Deliberately not used to skip or merge anything. It is a guess, and it is + * wrong in both directions: two colleagues who share a name and a shared alias + * collapse into one, and somebody whose address changed since the last export + * looks like a stranger. Either mistake is silent and one of them is + * unrecoverable, which is why #223 leaves the decision open. + * + * Counting is a different act from acting. An LDIF re-import duplicates + * everything -- Mozilla's schema has no UID, so the import invents one and + * nothing can match -- and the reported harm was confusion rather than data + * loss: somebody imports a file twice and cannot tell what happened. Being told + * "40 of these look like contacts you already had" answers that without + * touching a single card. + * + * One key per address, so a person whose second address matches is still + * recognised. + */ +function likenessKeys(c: Partial): string[] { + const name = contactDisplayName(c as ContactCard).trim().toLowerCase(); + if (!name) return []; + const addresses = Object.values(c.emails ?? {}).map((e) => e.address?.trim().toLowerCase()).filter(Boolean); + return addresses.map((a) => `${name}\u0000${a}`); } async function createCards(accountId: Id, create: Record): Promise<{ created: number; refused?: SetError }> { @@ -154,7 +185,7 @@ interface ContactsState { updateBook(id: Id, patch: Partial): Promise; destroyBook(id: Id): Promise; /** Import vCards, skipping any whose UID this book already holds. */ - importVCard(text: string, addressBookId: Id): Promise<{ created: number; skipped: number }>; + importVCard(text: string, addressBookId: Id): Promise<{ created: number; skipped: number; alike: number }>; /** * Import an address book in LDIF, read against Mozilla's schema. * @@ -162,7 +193,7 @@ interface ContactsState { * 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 }>; + importLdif(text: string, addressBookId: Id): Promise<{ created: number; skipped: number; alike: number }>; loadPrincipals(): Promise; suggest(query: string, limit?: number): Promise; addRecent(addrs: EmailAddress[]): void; @@ -460,7 +491,7 @@ export const useContacts = create((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 already = (await scanBook(accountId, addressBookId)).uids; const create: Record = {}; let skipped = 0; cards.forEach((c, i) => { @@ -482,7 +513,7 @@ export const useContacts = create((set, get) => ({ // The whole file was already here. Nothing to send, and nothing wrong. if (!Object.keys(create).length) { await get().loadAll(); - return { created: 0, skipped }; + return { created: 0, skipped, alike: 0 }; } try { const { created, refused } = await createCards(accountId, create); @@ -490,7 +521,9 @@ export const useContacts = create((set, get) => ({ // 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, skipped }; + /* No likeness count here: a vCard carries a UID, so anything that was + already present was skipped by name above rather than guessed at. */ + return { created, skipped, alike: 0 }; } finally { await get().loadAll(); } @@ -509,8 +542,16 @@ export const useContacts = create((set, get) => ({ const accountId = get().accountId!; const cards = parseLdif(text).map(cardFromLdif).filter((c): c is Partial => c !== null); if (!cards.length) throw new Error("it has no contacts in it"); + /* + * Read before anything is created, so "already had" means before this + * import rather than including it. Every card here is imported either way; + * this only counts. + */ + const before = await scanBook(accountId, addressBookId); + let alike = 0; const create: Record = {}; cards.forEach((c, i) => { + if (likenessKeys(c).some((k) => before.likeness.has(k))) alike++; // Built here rather than read from the file: LDIF identifies an entry by // its distinguished name, which says where it sat in somebody's // directory and is no use as a contact's identity anywhere else. @@ -529,8 +570,13 @@ export const useContacts = create((set, get) => ({ * 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. + * + * `alike` is what can be said without answering it: how many of these look + * like contacts that were already here. Reporting is not matching -- every + * card was imported -- and it is the confusion rather than the duplication + * that was reported as the harm. */ - return { created, skipped: 0 }; + return { created, skipped: 0, alike }; }, async loadPrincipals() { diff --git a/web/src/views/contacts/ContactsView.tsx b/web/src/views/contacts/ContactsView.tsx index 08ce7e1..a35bddb 100644 --- a/web/src/views/contacts/ContactsView.tsx +++ b/web/src/views/contacts/ContactsView.tsx @@ -136,7 +136,7 @@ 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 { created, skipped } = /^\s*BEGIN:VCARD/im.test(text) + const { created, skipped, alike } = /^\s*BEGIN:VCARD/im.test(text) ? await contacts.importVCard(text, book.id) : await contacts.importLdif(text, book.id); /* @@ -149,6 +149,18 @@ export function ContactsView({ id }: { id?: string }) { 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); + /* + * Said separately, and after, because it is a different kind of fact. + * LDIF has no UID to match on, so nothing was skipped and nothing was + * merged -- these are simply here twice now, and saying so is the whole + * of what can honestly be said without guessing (#223). + */ + if (alike) { + toast.show(plural(alike, { + one: "{n} of them looks like a contact you already had", + other: "{n} of them look like contacts you already had", + }), { duration: 9000 }); + } } catch (err) { toast.error(translate("Could not import this file: {error}", { error: (err as Error).message })); }