From 77986dddc93fc90780f62fc6f7bd2b1497f7d96d Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:59:29 +0100 Subject: [PATCH] fix: mark a repeated address within an imported file as a duplicate, and end the contact import with a toast that shows them --- .../__tests__/contact-import-ux-p2.test.tsx | 45 +++++++++++++++++++ components/contacts/contact-import-dialog.tsx | 14 +++++- components/contacts/contacts-app.tsx | 16 +++++++ lib/__tests__/vcard.test.ts | 11 +++++ lib/vcard.ts | 17 ++++--- 5 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 components/contacts/__tests__/contact-import-ux-p2.test.tsx diff --git a/components/contacts/__tests__/contact-import-ux-p2.test.tsx b/components/contacts/__tests__/contact-import-ux-p2.test.tsx new file mode 100644 index 000000000..9bc0fc74f --- /dev/null +++ b/components/contacts/__tests__/contact-import-ux-p2.test.tsx @@ -0,0 +1,45 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import type { ContactCard } from '@/lib/jmap/types'; +import { ContactImportDialog } from '../contact-import-dialog'; + +// pim-p2-15 (2026-11-04): a CSV with the same address on two rows imported both, +// and the import ended in a dialog page that only had Close. + +const CSV = [ + 'First Name,Last Name,E-mail Address', + 'Anna,Schmidt,anna@example.org', + 'Bob,Meyer,bob@example.org', + 'Anna,Schmidt,ANNA@example.org', + '', +].join('\r\n'); + +const file = () => new File([CSV], 'contacts-export.csv', { type: 'text/csv' }); + +describe('ContactImportDialog, pass 2', () => { + it('marks the second row with the same address as a duplicate, and imports the rest', async () => { + const onImport = vi.fn(async (cards: ContactCard[]) => cards.length); + render(); + fireEvent.click(await screen.findByRole('button', { name: 'continue' })); + expect(await screen.findByText('Bob Meyer')).toBeInTheDocument(); + expect(screen.getAllByText('import.duplicate')).toHaveLength(1); + fireEvent.click(screen.getByRole('button', { name: 'import.import_button' })); + await waitFor(() => expect(onImport).toHaveBeenCalled()); + const cards = onImport.mock.calls[0][0] as ContactCard[]; + expect(cards.map((c) => Object.values(c.emails ?? {})[0]?.address)).toEqual(['anna@example.org', 'bob@example.org']); + }); + + it('closes when done and leaves the word to the caller', async () => { + const onClose = vi.fn(); + const onImported = vi.fn(); + render( + cards.length} onClose={onClose} onImported={onImported} initialFile={file()} />, + ); + fireEvent.click(await screen.findByRole('button', { name: 'continue' })); + await screen.findByText('Bob Meyer'); + fireEvent.click(screen.getByRole('button', { name: 'import.import_button' })); + await waitFor(() => expect(onImported).toHaveBeenCalledWith(2, undefined)); + expect(onClose).toHaveBeenCalled(); + expect(screen.queryByText('import.success')).toBeNull(); + }); +}); diff --git a/components/contacts/contact-import-dialog.tsx b/components/contacts/contact-import-dialog.tsx index 9886cf576..598a43c60 100644 --- a/components/contacts/contact-import-dialog.tsx +++ b/components/contacts/contact-import-dialog.tsx @@ -26,6 +26,11 @@ interface ContactImportDialogProps { addressBooks?: AddressBook[]; /** The book open in Contacts: "Import into" starts there. */ defaultAddressBookId?: string; + /** + * Done: the dialog closes and the caller says so (a toast with Show) instead of a + * page of its own that only has Close. Without it the dialog shows the count. + */ + onImported?: (count: number, addressBookId?: string) => void; } /** The file's bytes; jsdom's File has no arrayBuffer(). */ @@ -52,6 +57,7 @@ export function ContactImportDialog({ initialFile, addressBooks = [], defaultAddressBookId, + onImported, }: ContactImportDialogProps) { const t = useTranslations("contacts"); const shownName = useCollectionNames(); @@ -194,7 +200,13 @@ export function ContactImportDialog({ setIsImporting(true); try { - const count = await onImport(toImport, addressBooks.length > 0 && bookId ? bookId : undefined); + const target = addressBooks.length > 0 && bookId ? bookId : undefined; + const count = await onImport(toImport, target); + if (onImported) { + onClose(); + onImported(count, target); + return; + } setResult(count); } catch (error) { console.error('Failed to import contacts:', error); diff --git a/components/contacts/contacts-app.tsx b/components/contacts/contacts-app.tsx index 4e6656441..70d00e408 100644 --- a/components/contacts/contacts-app.tsx +++ b/components/contacts/contacts-app.tsx @@ -104,6 +104,7 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps = const linkSegments = useLiteLinkSegments('contacts', routeSegments); const shownName = useCollectionNames(); const t = useTranslations("contacts"); + const tImport = useTranslations("ux_p2_pim_cm"); const skeleton = useSkeletonLabels(); // Organisation > Devices > No printing: no Print buttons (Ctrl+P prints a note). const printOff = useStrictestDevices().noPrint; @@ -601,6 +602,19 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps = addressBookId, ); }, [supportsSync, client, importContacts]); + // Done importing: a toast instead of a dialog page, and Show opens where they went. + const handleImported = useCallback((count: number, addressBookId?: string) => { + if (count <= 0) return; + toast.success(t("import.success", { count }), { + action: { + label: tImport("import_show"), + onClick: () => { + setSearchQuery(""); + handleSelectCategory(addressBookId ? { addressBookId } : "all"); + }, + }, + }); + }, [t, tImport, handleSelectCategory, setSearchQuery]); // "Import into" starts at the address book on screen. const openBookId = typeof activeCategory === "object" && "addressBookId" in activeCategory ? activeCategory.addressBookId : undefined; @@ -1679,6 +1693,7 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps = onClose={launchedFile.done} addressBooks={supportsSync ? writableBooks : []} defaultAddressBookId={openBookId} + onImported={handleImported} /> )} {showImportDialog && ( @@ -1688,6 +1703,7 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps = onClose={() => setShowImportDialog(false)} addressBooks={supportsSync ? writableBooks : []} defaultAddressBookId={openBookId} + onImported={handleImported} /> )} {(mergeCards || mergePickFor) && ( diff --git a/lib/__tests__/vcard.test.ts b/lib/__tests__/vcard.test.ts index 5f956778a..127a5221d 100644 --- a/lib/__tests__/vcard.test.ts +++ b/lib/__tests__/vcard.test.ts @@ -1177,6 +1177,17 @@ describe("detectDuplicates", () => { expect(dupes.get(0)).toBe("existing-1"); }); + // pim-p2-15 (2026-11-04): a CSV with the same address on two rows imported both. + it("marks a later card of the same file with the same address as a duplicate of the first", () => { + const incoming: ContactCard[] = [ + { id: "row-1", addressBookIds: {}, emails: { e0: { address: "anna@example.org" } } }, + { id: "row-2", addressBookIds: {}, emails: { e0: { address: "bob@example.org" } } }, + { id: "row-3", addressBookIds: {}, emails: { e0: { address: " Anna@Example.org" } } }, + ]; + const dupes = detectDuplicates([], incoming); + expect([...dupes.entries()]).toEqual([[2, "row-1"]]); + }); + it("returns empty map when no duplicates", () => { const existing: ContactCard[] = [ { diff --git a/lib/vcard.ts b/lib/vcard.ts index b52d69aca..6ed7b6231 100644 --- a/lib/vcard.ts +++ b/lib/vcard.ts @@ -1596,16 +1596,19 @@ export function detectDuplicates( } } + // A file can hold the same person twice (a CSV export often does): the first + // one counts, the later ones are duplicates of it, by its id. + const seenInFile = new Map(); incoming.forEach((card, idx) => { - if (card.emails) { - for (const e of Object.values(card.emails)) { - const match = existingEmails.get(e.address.toLowerCase()); - if (match) { - dupes.set(idx, match); - return; - } + const addresses = Object.values(card.emails ?? {}).map((e) => e.address?.trim().toLowerCase()).filter(Boolean) as string[]; + for (const address of addresses) { + const match = existingEmails.get(address) ?? seenInFile.get(address); + if (match) { + dupes.set(idx, match); + return; } } + for (const address of addresses) seenInFile.set(address, card.id); }); return dupes;