fix: mark a repeated address within an imported file as a duplicate, and end the contact import with a toast that shows them

This commit is contained in:
Linus Rath
2026-11-04 13:59:29 +01:00
parent 5365b1a461
commit 77986dddc9
5 changed files with 95 additions and 8 deletions
@@ -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(<ContactImportDialog existingContacts={[]} onImport={onImport} onClose={vi.fn()} initialFile={file()} />);
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(
<ContactImportDialog existingContacts={[]} onImport={async (cards) => 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();
});
});
+13 -1
View File
@@ -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);
+16
View File
@@ -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) && (
+11
View File
@@ -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[] = [
{
+10 -7
View File
@@ -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<string, string>();
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;