mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: ask before a contact import replaces a card with the same UID
This commit is contained in:
+3
-1
@@ -253,7 +253,9 @@
|
|||||||
"into_address_book": "In Adressbuch importieren",
|
"into_address_book": "In Adressbuch importieren",
|
||||||
"into_named": "In {name} importieren",
|
"into_named": "In {name} importieren",
|
||||||
"no_address_book": "Erstellen Sie ein Adressbuch, bevor Sie Kontakte importieren.",
|
"no_address_book": "Erstellen Sie ein Adressbuch, bevor Sie Kontakte importieren.",
|
||||||
"picker_failed": "Die Dateiauswahl konnte nicht geöffnet werden"
|
"picker_failed": "Die Dateiauswahl konnte nicht geöffnet werden",
|
||||||
|
"replaces": "Ersetzt {name}",
|
||||||
|
"update_existing": "Vorhandenen aktualisieren"
|
||||||
},
|
},
|
||||||
"no_tags_hint": "Noch keine Kategorien. Geben Sie oben eine ein.",
|
"no_tags_hint": "Noch keine Kategorien. Geben Sie oben eine ein.",
|
||||||
"picker": {
|
"picker": {
|
||||||
|
|||||||
@@ -261,7 +261,9 @@
|
|||||||
"into_named": "Import into {name}",
|
"into_named": "Import into {name}",
|
||||||
"no_address_book": "Create an address book before importing contacts.",
|
"no_address_book": "Create an address book before importing contacts.",
|
||||||
"picker_failed": "Could not open the file picker",
|
"picker_failed": "Could not open the file picker",
|
||||||
|
"replaces": "Replaces {name}",
|
||||||
"unnamed": "Unnamed contact",
|
"unnamed": "Unnamed contact",
|
||||||
|
"update_existing": "Update existing",
|
||||||
"updated_count": "{count, plural, one {1 contact was already there and has been updated.} other {# contacts were already there and have been updated.}}"
|
"updated_count": "{count, plural, one {1 contact was already there and has been updated.} other {# contacts were already there and have been updated.}}"
|
||||||
},
|
},
|
||||||
"no_tags_hint": "No tags yet. Type one above.",
|
"no_tags_hint": "No tags yet. Type one above.",
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ import { Upload, X, Check, FileText, AlertTriangle, BookUser } from '../icons';
|
|||||||
import type { ContactCard } from '../../api/types';
|
import type { ContactCard } from '../../api/types';
|
||||||
import { parseVCard, detectDuplicates } from '../../lib/vcard';
|
import { parseVCard, detectDuplicates } from '../../lib/vcard';
|
||||||
import { getContactDisplayName, getContactPrimaryEmail } from '../../lib/contact-utils';
|
import { getContactDisplayName, getContactPrimaryEmail } from '../../lib/contact-utils';
|
||||||
import { useContactsStore, type ContactImportResult } from '../../stores/contacts-store';
|
import { selectImportUidMatches, useContactsStore, type ContactImportResult } from '../../stores/contacts-store';
|
||||||
import Button from '../Button';
|
import Button from '../Button';
|
||||||
import { spacing, radius, typography, type ThemePalette } from '../../theme/tokens';
|
import { spacing, radius, typography, type ThemePalette } from '../../theme/tokens';
|
||||||
import { useColors } from '../../theme/colors';
|
import { useColors } from '../../theme/colors';
|
||||||
@@ -49,6 +49,7 @@ export default function ContactImportSheet({
|
|||||||
const insets = useSafeAreaInsets();
|
const insets = useSafeAreaInsets();
|
||||||
const t = useLocaleStore((s) => s.t);
|
const t = useLocaleStore((s) => s.t);
|
||||||
const existing = useContactsStore((s) => s.contacts);
|
const existing = useContactsStore((s) => s.contacts);
|
||||||
|
const addressBooks = useContactsStore((s) => s.addressBooks);
|
||||||
const importContacts = useContactsStore((s) => s.importContacts);
|
const importContacts = useContactsStore((s) => s.importContacts);
|
||||||
|
|
||||||
const [parsed, setParsed] = React.useState<ContactCard[]>([]);
|
const [parsed, setParsed] = React.useState<ContactCard[]>([]);
|
||||||
@@ -57,6 +58,28 @@ export default function ContactImportSheet({
|
|||||||
const [error, setError] = React.useState<string | null>(null);
|
const [error, setError] = React.useState<string | null>(null);
|
||||||
const [importing, setImporting] = React.useState(false);
|
const [importing, setImporting] = React.useState(false);
|
||||||
const [result, setResult] = React.useState<ContactImportResult | null>(null);
|
const [result, setResult] = React.useState<ContactImportResult | null>(null);
|
||||||
|
// Cards whose UID the target's account already has: importing one replaces
|
||||||
|
// that card, which may be another contact (an older export, another app's
|
||||||
|
// "1", "2" UIDs). Left unselected, like an e-mail duplicate, until the user
|
||||||
|
// picks them.
|
||||||
|
const uidMatches = React.useMemo(
|
||||||
|
() => selectImportUidMatches({ contacts: existing, addressBooks }, parsed, targetBookId),
|
||||||
|
[existing, addressBooks, parsed, targetBookId],
|
||||||
|
);
|
||||||
|
// Another target book (another account) or a refreshed list can match
|
||||||
|
// cards the user has not seen marked yet: those start unselected too.
|
||||||
|
const shownMatches = React.useRef<Map<number, ContactCard>>(new Map());
|
||||||
|
React.useEffect(() => {
|
||||||
|
const before = shownMatches.current;
|
||||||
|
shownMatches.current = uidMatches;
|
||||||
|
const fresh = [...uidMatches.keys()].filter((idx) => !before.has(idx));
|
||||||
|
if (fresh.length === 0) return;
|
||||||
|
setSelected((prev) => {
|
||||||
|
const next = new Set(prev);
|
||||||
|
for (const idx of fresh) next.delete(idx);
|
||||||
|
return next;
|
||||||
|
});
|
||||||
|
}, [uidMatches]);
|
||||||
|
|
||||||
const slideY = React.useRef(new Animated.Value(900)).current;
|
const slideY = React.useRef(new Animated.Value(900)).current;
|
||||||
// Centred on a wide window, clear of side insets in landscape.
|
// Centred on a wide window, clear of side insets in landscape.
|
||||||
@@ -120,8 +143,9 @@ export default function ContactImportSheet({
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const dupes = detectDuplicates(existing, contacts);
|
const dupes = detectDuplicates(existing, contacts);
|
||||||
|
const replaces = selectImportUidMatches({ contacts: existing, addressBooks }, contacts, targetBookId);
|
||||||
const initial = new Set<number>();
|
const initial = new Set<number>();
|
||||||
contacts.forEach((_, idx) => { if (!dupes.has(idx)) initial.add(idx); });
|
contacts.forEach((_, idx) => { if (!dupes.has(idx) && !replaces.has(idx)) initial.add(idx); });
|
||||||
setParsed(contacts);
|
setParsed(contacts);
|
||||||
setDuplicates(dupes);
|
setDuplicates(dupes);
|
||||||
setSelected(initial);
|
setSelected(initial);
|
||||||
@@ -152,9 +176,15 @@ export default function ContactImportSheet({
|
|||||||
}
|
}
|
||||||
const toImport = parsed.filter((_, i) => selected.has(i));
|
const toImport = parsed.filter((_, i) => selected.has(i));
|
||||||
if (toImport.length === 0) return;
|
if (toImport.length === 0) return;
|
||||||
|
// Only the existing cards the user left selected are replaced.
|
||||||
|
const updateUids = new Set<string>();
|
||||||
|
for (const idx of selected) {
|
||||||
|
const uid = parsed[idx]?.uid;
|
||||||
|
if (uid && uidMatches.has(idx)) updateUids.add(uid);
|
||||||
|
}
|
||||||
setImporting(true);
|
setImporting(true);
|
||||||
try {
|
try {
|
||||||
const outcome = await importContacts(toImport, targetBookId);
|
const outcome = await importContacts(toImport, targetBookId, updateUids);
|
||||||
setResult(outcome);
|
setResult(outcome);
|
||||||
onImported?.(outcome.imported + outcome.updated);
|
onImported?.(outcome.imported + outcome.updated);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
@@ -276,6 +306,8 @@ export default function ContactImportSheet({
|
|||||||
const name = getContactDisplayName(contact);
|
const name = getContactDisplayName(contact);
|
||||||
const email = getContactPrimaryEmail(contact);
|
const email = getContactPrimaryEmail(contact);
|
||||||
const isDupe = duplicates.has(idx);
|
const isDupe = duplicates.has(idx);
|
||||||
|
const replaces = uidMatches.get(idx);
|
||||||
|
const replacesName = replaces ? getContactDisplayName(replaces) || getContactPrimaryEmail(replaces) : '';
|
||||||
const isSelected = selected.has(idx);
|
const isSelected = selected.has(idx);
|
||||||
return (
|
return (
|
||||||
<Pressable
|
<Pressable
|
||||||
@@ -294,8 +326,15 @@ export default function ContactImportSheet({
|
|||||||
{!!email && !!name && (
|
{!!email && !!name && (
|
||||||
<Text style={styles.rowEmail} numberOfLines={1}>{email}</Text>
|
<Text style={styles.rowEmail} numberOfLines={1}>{email}</Text>
|
||||||
)}
|
)}
|
||||||
|
{!!replaces && !!replacesName && replacesName !== (name || email) && (
|
||||||
|
<Text style={styles.rowEmail} numberOfLines={1}>
|
||||||
|
{t('contacts.import.replaces', 'Replaces {name}', { name: replacesName })}
|
||||||
|
</Text>
|
||||||
|
)}
|
||||||
</View>
|
</View>
|
||||||
{isDupe && (
|
{replaces ? (
|
||||||
|
<Text style={styles.dupeBadge}>{t('contacts.import.update_existing', 'Update existing')}</Text>
|
||||||
|
) : isDupe && (
|
||||||
<Text style={styles.dupeBadge}>{t('contacts.import.duplicate', 'Duplicate')}</Text>
|
<Text style={styles.dupeBadge}>{t('contacts.import.duplicate', 'Duplicate')}</Text>
|
||||||
)}
|
)}
|
||||||
</Pressable>
|
</Pressable>
|
||||||
|
|||||||
@@ -71,6 +71,7 @@ import {
|
|||||||
selectGroupMembers,
|
selectGroupMembers,
|
||||||
sortContactsByName,
|
sortContactsByName,
|
||||||
selectCreateTargetBookId,
|
selectCreateTargetBookId,
|
||||||
|
selectImportUidMatches,
|
||||||
CONTACTS_STALE_MS,
|
CONTACTS_STALE_MS,
|
||||||
} from '../contacts-store';
|
} from '../contacts-store';
|
||||||
import type { AddressBook, ContactCard } from '../../api/types';
|
import type { AddressBook, ContactCard } from '../../api/types';
|
||||||
@@ -429,6 +430,7 @@ describe('contacts-store', () => {
|
|||||||
const result = await useContactsStore.getState().importContacts(
|
const result = await useContactsStore.getState().importContacts(
|
||||||
[{ id: 'import-1', uid: 'urn:uuid:aaa', addressBookIds: { x: true }, name: { full: 'New' } }],
|
[{ id: 'import-1', uid: 'urn:uuid:aaa', addressBookIds: { x: true }, name: { full: 'New' } }],
|
||||||
'ab-1',
|
'ab-1',
|
||||||
|
new Set(['urn:uuid:aaa']),
|
||||||
);
|
);
|
||||||
expect(mockCreateContact).not.toHaveBeenCalled();
|
expect(mockCreateContact).not.toHaveBeenCalled();
|
||||||
expect(mockUpdateContact).toHaveBeenCalledWith('c1', { name: { full: 'New' } }, undefined);
|
expect(mockUpdateContact).toHaveBeenCalledWith('c1', { name: { full: 'New' } }, undefined);
|
||||||
@@ -445,10 +447,52 @@ describe('contacts-store', () => {
|
|||||||
const result = await useContactsStore.getState().importContacts(
|
const result = await useContactsStore.getState().importContacts(
|
||||||
[{ id: 'import-1', uid: 'urn:uuid:bbb', name: { full: 'B' } }],
|
[{ id: 'import-1', uid: 'urn:uuid:bbb', name: { full: 'B' } }],
|
||||||
'ab-1',
|
'ab-1',
|
||||||
|
new Set(['urn:uuid:bbb']),
|
||||||
);
|
);
|
||||||
expect(mockUpdateContact).toHaveBeenCalledWith('m', { name: { full: 'B' } }, undefined);
|
expect(mockUpdateContact).toHaveBeenCalledWith('m', { name: { full: 'B' } }, undefined);
|
||||||
expect(result).toMatchObject({ imported: 0, updated: 1, failed: 0, failures: [] });
|
expect(result).toMatchObject({ imported: 0, updated: 1, failed: 0, failures: [] });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Another app's export numbers its cards "1", "2": the same UID can be
|
||||||
|
// another contact, which the import replaced wholesale without asking.
|
||||||
|
it('replaces a card with the same UID only when the user chose to', async () => {
|
||||||
|
useContactsStore.setState({
|
||||||
|
addressBooks: [{ id: 'ab-1', name: 'P' }],
|
||||||
|
contacts: [card('c1', { uid: '1', name: { full: 'Anna' } }), card('c2', { uid: '2', name: { full: 'Ben' } })],
|
||||||
|
});
|
||||||
|
mockCreateContact.mockRejectedValueOnce(new Error('Contact with UID 3 already exists with id c9. (uid)'));
|
||||||
|
const result = await useContactsStore.getState().importContacts(
|
||||||
|
[
|
||||||
|
{ id: 'import-1', uid: '1', name: { full: 'Carla' } },
|
||||||
|
{ id: 'import-2', uid: '2', name: { full: 'Ben B.' } },
|
||||||
|
{ id: 'import-3', uid: '3', name: { full: 'Dora' } },
|
||||||
|
],
|
||||||
|
'ab-1',
|
||||||
|
new Set(['2']),
|
||||||
|
);
|
||||||
|
expect(mockUpdateContact).toHaveBeenCalledTimes(1);
|
||||||
|
expect(mockUpdateContact).toHaveBeenCalledWith('c2', { name: { full: 'Ben B.' } }, undefined);
|
||||||
|
expect(result).toMatchObject({ imported: 0, updated: 1, failed: 2 });
|
||||||
|
expect(result.failures.map((f) => f.name)).toEqual(['Carla', 'Dora']);
|
||||||
|
expect(useContactsStore.getState().contacts.map((c) => c.name?.full)).toEqual(['Anna', 'Ben B.']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('tells the import sheet which cards an import would replace, in the target book\'s account', () => {
|
||||||
|
const state = {
|
||||||
|
addressBooks: [
|
||||||
|
{ id: 'ab-1', name: 'P' },
|
||||||
|
{ id: 'acc-team:ab-9', originalId: 'ab-9', name: 'Team', isShared: true, accountId: 'acc-team' },
|
||||||
|
] as AddressBook[],
|
||||||
|
contacts: [
|
||||||
|
card('c1', { uid: '1' }),
|
||||||
|
card('acc-team:t1', { uid: '2', isShared: true, accountId: 'acc-team' }),
|
||||||
|
],
|
||||||
|
};
|
||||||
|
const incoming = [{ uid: '1' }, { uid: '2' }, { uid: '3' }, {}];
|
||||||
|
expect([...selectImportUidMatches(state, incoming, 'ab-1')].map(([i, c]) => [i, c.id])).toEqual([[0, 'c1']]);
|
||||||
|
expect([...selectImportUidMatches(state, incoming, 'acc-team:ab-9')].map(([i, c]) => [i, c.id]))
|
||||||
|
.toEqual([[1, 'acc-team:t1']]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('address books', () => {
|
describe('address books', () => {
|
||||||
|
|||||||
@@ -117,12 +117,15 @@ export interface ContactsState {
|
|||||||
deleteContact: (id: string) => Promise<void>;
|
deleteContact: (id: string) => Promise<void>;
|
||||||
bulkDelete: (ids: string[]) => Promise<void>;
|
bulkDelete: (ids: string[]) => Promise<void>;
|
||||||
/**
|
/**
|
||||||
* Creates the cards in the book; one whose UID the account already has is
|
* Creates the cards in the book. One whose UID the account already has is
|
||||||
* updated in place instead (a re-import of an export).
|
* updated in place instead (a re-import of an export), but only when its
|
||||||
|
* UID is in `updateUids`: the user chose to replace that card. Any other
|
||||||
|
* card whose UID is taken is not written and counts as failed.
|
||||||
*/
|
*/
|
||||||
importContacts: (
|
importContacts: (
|
||||||
contacts: Partial<ContactCard>[],
|
contacts: Partial<ContactCard>[],
|
||||||
addressBookId: string,
|
addressBookId: string,
|
||||||
|
updateUids?: ReadonlySet<string>,
|
||||||
) => Promise<ContactImportResult>;
|
) => Promise<ContactImportResult>;
|
||||||
|
|
||||||
addContactKeyword: (id: string, keyword: string) => Promise<void>;
|
addContactKeyword: (id: string, keyword: string) => Promise<void>;
|
||||||
@@ -194,6 +197,37 @@ export function existingUidCardId(err: unknown): string | null {
|
|||||||
return m ? m[1] : null;
|
return m ? m[1] : null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The account's cards by UID; `accountId` undefined is the own account. */
|
||||||
|
function cardsByUid(contacts: ContactCard[], accountId: string | undefined): Map<string, ContactCard> {
|
||||||
|
const byUid = new Map<string, ContactCard>();
|
||||||
|
for (const c of contacts) {
|
||||||
|
// As the server compares them: "x" and "urn:uuid:x" are two UIDs.
|
||||||
|
if (c.uid && (c.isShared ? c.accountId : undefined) === accountId) byUid.set(c.uid, c);
|
||||||
|
}
|
||||||
|
return byUid;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The cards an import into `addressBookId` would replace: its account holds
|
||||||
|
* one with the incoming card's UID, in whatever book. That may be another
|
||||||
|
* contact altogether (an older export, another app's "1", "2" UIDs), so the
|
||||||
|
* import sheet asks first. Keyed by the incoming card's index.
|
||||||
|
*/
|
||||||
|
export function selectImportUidMatches(
|
||||||
|
state: Pick<ContactsState, 'contacts' | 'addressBooks'>,
|
||||||
|
incoming: Partial<ContactCard>[],
|
||||||
|
addressBookId: string | null,
|
||||||
|
): Map<number, ContactCard> {
|
||||||
|
const book = state.addressBooks.find((b) => b.id === addressBookId);
|
||||||
|
const byUid = cardsByUid(state.contacts, book?.isShared ? book.accountId : undefined);
|
||||||
|
const matches = new Map<number, ContactCard>();
|
||||||
|
incoming.forEach((card, idx) => {
|
||||||
|
const known = card.uid ? byUid.get(card.uid) : undefined;
|
||||||
|
if (known) matches.set(idx, known);
|
||||||
|
});
|
||||||
|
return matches;
|
||||||
|
}
|
||||||
|
|
||||||
function persistCategory(category: ContactCategory): void {
|
function persistCategory(category: ContactCategory): void {
|
||||||
void AsyncStorage.setItem(SELECTED_CATEGORY_STORAGE_KEY, JSON.stringify(category)).catch(
|
void AsyncStorage.setItem(SELECTED_CATEGORY_STORAGE_KEY, JSON.stringify(category)).catch(
|
||||||
(err) => console.warn('[contacts-store] persist category failed', err),
|
(err) => console.warn('[contacts-store] persist category failed', err),
|
||||||
@@ -603,7 +637,7 @@ export const useContactsStore = create<ContactsState>()(
|
|||||||
if (removed.size > 0) requestDeviceSync(CONTACTS_AUTHORITY);
|
if (removed.size > 0) requestDeviceSync(CONTACTS_AUTHORITY);
|
||||||
},
|
},
|
||||||
|
|
||||||
importContacts: async (incoming, addressBookId) => {
|
importContacts: async (incoming, addressBookId, updateUids) => {
|
||||||
let imported = 0;
|
let imported = 0;
|
||||||
let updated = 0;
|
let updated = 0;
|
||||||
let refetch = false;
|
let refetch = false;
|
||||||
@@ -612,12 +646,10 @@ export const useContactsStore = create<ContactsState>()(
|
|||||||
const { check } = generationGuard();
|
const { check } = generationGuard();
|
||||||
// Stalwart keeps one card per UID in an account and refuses a second
|
// Stalwart keeps one card per UID in an account and refuses a second
|
||||||
// ("Contact with UID ... already exists"), so re-importing an export
|
// ("Contact with UID ... already exists"), so re-importing an export
|
||||||
// failed card by card. A card the account already has is updated.
|
// failed card by card. A card the account already has is updated,
|
||||||
const byUid = new Map<string, ContactCard>();
|
// when the user chose to: the same UID may be another contact.
|
||||||
for (const c of get().contacts) {
|
const byUid = cardsByUid(get().contacts, accountId);
|
||||||
// As the server compares them: "x" and "urn:uuid:x" are two UIDs.
|
const mayUpdate = (uid: string | undefined) => !!uid && !!updateUids?.has(uid);
|
||||||
if (c.uid && (c.isShared ? c.accountId : undefined) === accountId) byUid.set(c.uid, c);
|
|
||||||
}
|
|
||||||
const updateExisting = async (serverId: string, data: Partial<ContactCard>) => {
|
const updateExisting = async (serverId: string, data: Partial<ContactCard>) => {
|
||||||
// The card stays in its own book and keeps its UID.
|
// The card stays in its own book and keeps its UID.
|
||||||
const { uid: _uid, ...patch } = data;
|
const { uid: _uid, ...patch } = data;
|
||||||
@@ -637,6 +669,7 @@ export const useContactsStore = create<ContactsState>()(
|
|||||||
const known = data.uid ? byUid.get(data.uid) : undefined;
|
const known = data.uid ? byUid.get(data.uid) : undefined;
|
||||||
try {
|
try {
|
||||||
if (known) {
|
if (known) {
|
||||||
|
if (!mayUpdate(data.uid)) throw new Error(`A contact with UID ${data.uid} already exists`);
|
||||||
await updateExisting(known.originalId || known.id, data);
|
await updateExisting(known.originalId || known.id, data);
|
||||||
updated++;
|
updated++;
|
||||||
continue;
|
continue;
|
||||||
@@ -648,7 +681,7 @@ export const useContactsStore = create<ContactsState>()(
|
|||||||
// A card with this UID the list does not hold (not loaded yet,
|
// A card with this UID the list does not hold (not loaded yet,
|
||||||
// or in another book): the server names it.
|
// or in another book): the server names it.
|
||||||
const existingId = existingUidCardId(err);
|
const existingId = existingUidCardId(err);
|
||||||
if (!existingId) throw err;
|
if (!existingId || !mayUpdate(data.uid)) throw err;
|
||||||
await updateExisting(existingId, data);
|
await updateExisting(existingId, data);
|
||||||
updated++;
|
updated++;
|
||||||
continue;
|
continue;
|
||||||
|
|||||||
Reference in New Issue
Block a user