diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index 2e6fde3..6680d81 100644 --- a/web/src/locales/de.ts +++ b/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" }, diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index b97f057..3d32615 100644 --- a/web/src/locales/es.ts +++ b/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" }, diff --git a/web/src/locales/fr.ts b/web/src/locales/fr.ts index 6e19aed..93ce238 100644 --- a/web/src/locales/fr.ts +++ b/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é" }, diff --git a/web/src/locales/ja.ts b/web/src/locales/ja.ts index 5952e17..818156e 100644 --- a/web/src/locales/ja.ts +++ b/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} 件、インポートなし" }, diff --git a/web/src/locales/nl.ts b/web/src/locales/nl.ts index bfc9028..e63c944 100644 --- a/web/src/locales/nl.ts +++ b/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" }, diff --git a/web/src/locales/pt-BR.ts b/web/src/locales/pt-BR.ts index a76083e..ded6aeb 100644 --- a/web/src/locales/pt-BR.ts +++ b/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" }, diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index 2b1e44e..5f188f7 100644 --- a/web/src/locales/ru.ts +++ b/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} события, ничего не импортировано" }, diff --git a/web/src/locales/uk.ts b/web/src/locales/uk.ts index 928ea95..c13134d 100644 --- a/web/src/locales/uk.ts +++ b/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} події, нічого не імпортовано" }, diff --git a/web/src/locales/zh-Hans.ts b/web/src/locales/zh-Hans.ts index 7687aaf..0f58c57 100644 --- a/web/src/locales/zh-Hans.ts +++ b/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} 个,未导入" }, diff --git a/web/src/store/__tests__/contact-set-batching.test.ts b/web/src/store/__tests__/contact-set-batching.test.ts index c784a35..6cb5f03 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.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 }); }); }); diff --git a/web/src/store/__tests__/ldif-import.test.ts b/web/src/store/__tests__/ldif-import.test.ts index 6889ffb..19d3cab 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).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 }); }); }); diff --git a/web/src/store/__tests__/vcard-dedupe.test.ts b/web/src/store/__tests__/vcard-dedupe.test.ts new file mode 100644 index 0000000..5cd4e60 --- /dev/null +++ b/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> } + +function server(opts: { parsed?: unknown[]; existing?: Array<{ id: string; uid: string; addressBookIds: Record }> } = {}) { + 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][] }; + 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> }); + 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 }); + 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: jane@example.com\n\ndn: cn=Alan Turing\ngivenName: Alan\nsn: Turing\ncn: Alan Turing\nmail: alan@example.org\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 }); + }); +}); diff --git a/web/src/store/contacts.ts b/web/src/store/contacts.ts index 929ffe2..e19c969 100644 --- a/web/src/store/contacts.ts +++ b/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> { + const uids = 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); + } + 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): Promise<{ created: number; refused?: SetError }> { const keys = Object.keys(create); let created = 0; @@ -122,9 +153,16 @@ interface ContactsState { createBook(name: string): Promise; updateBook(id: Id, patch: Partial): Promise; destroyBook(id: Id): Promise; - importVCard(text: string, addressBookId: Id): Promise; - /** Import an address book in LDIF, read against Mozilla's schema. */ - importLdif(text: string, addressBookId: Id): Promise; + /** 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; suggest(query: string, limit?: number): Promise; addRecent(addrs: EmailAddress[]): void; @@ -422,18 +460,37 @@ 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 create: Record = {}; + 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((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() { diff --git a/web/src/views/contacts/ContactsView.tsx b/web/src/views/contacts/ContactsView.tsx index db40ce1..08ce7e1 100644 --- a/web/src/views/contacts/ContactsView.tsx +++ b/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 })); } diff --git a/web/src/views/mail/VCardCard.tsx b/web/src/views/mail/VCardCard.tsx index 52a287b..7ee84fb 100644 --- a/web/src/views/mail/VCardCard.tsx +++ b/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 {