fix: say when merging contacts left a card behind, and don't let Undo copy it

This commit is contained in:
Linus Rath
2026-11-04 11:36:52 +01:00
parent bdf25e4183
commit be57f23bf6
3 changed files with 48 additions and 12 deletions
+2 -1
View File
@@ -743,7 +743,8 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps =
setSelectedContact(plan.keep.id);
setView("detail");
}
toast.success(tMerge("merged"), {
// Not every card went: say so, and still offer Undo for what changed.
(undo.incomplete ? toast.error : toast.success)(tMerge(undo.incomplete ? "merge_failed" : "merged"), {
action: {
label: tMerge("undo"),
onClick: () => {
@@ -108,6 +108,29 @@ describe('mergeContacts', () => {
expect(contacts.find((c) => c.uid === 'urn:uuid:old')).toBeTruthy();
});
// A delete the server refused left the card in place while the toast said
// merged, and Undo then created it a second time.
it('says when a card could not be deleted, and Undo does not create it again', async () => {
useContactStore.setState({ contacts: [older, newer, team] });
const client = makeClient();
client.deleteContact.mockRejectedValueOnce(new Error('server said no'));
const undo = await useContactStore.getState().mergeContacts(client as unknown as IJMAPClient, planMerge([older, newer]));
expect(undo?.incomplete).toBe(true);
expect(undo?.others).toEqual([]);
expect(useContactStore.getState().contacts.some((c) => c.id === 'c-old')).toBe(true);
await useContactStore.getState().undoMergeContacts(client as unknown as IJMAPClient, undo!);
expect(client.createContact).not.toHaveBeenCalled();
});
it('deletes nothing and still hands back Undo when moving a group member fails', async () => {
useContactStore.setState({ contacts: [older, newer, team] });
const client = makeClient();
client.updateContact.mockResolvedValueOnce(undefined).mockRejectedValueOnce(new Error('group write failed'));
const undo = await useContactStore.getState().mergeContacts(client as unknown as IJMAPClient, planMerge([older, newer]));
expect(undo).toMatchObject({ keepId: 'c-new', incomplete: true, others: [] });
expect(client.deleteContact).not.toHaveBeenCalled();
});
it('deletes nothing when the kept card cannot be written', async () => {
useContactStore.setState({ contacts: [older, newer, team] });
const client = makeClient();
+23 -11
View File
@@ -27,6 +27,8 @@ export interface MergeUndo {
others: ContactCard[];
/** The groups' members before the merge. */
groups: Array<{ groupId: string; members: Record<string, boolean> }>;
/** Some step after the kept card's update failed or was stopped: not all cards are gone. */
incomplete?: boolean;
}
/** A contact that is about to be created, projected for plugins (no id yet). */
@@ -1745,20 +1747,30 @@ export const useContactStore = create<ContactStore>()(
}
const groups: MergeUndo['groups'] = [];
const moves = planGroupMoves(get().contacts.filter(c => c.kind === 'group'), keep, others);
for (const { group, members } of moves) {
if (sync) {
const accountId = group.isShared ? group.accountId : undefined;
await resolveAccountClient(client!, group.localAccountId)
.updateContact(group.originalId || group.id, { members }, accountId);
try {
for (const { group, members } of moves) {
if (sync) {
const accountId = group.isShared ? group.accountId : undefined;
await resolveAccountClient(client!, group.localAccountId)
.updateContact(group.originalId || group.id, { members }, accountId);
}
groups.push({ groupId: group.id, members: { ...(group.members ?? {}) } });
set((state) => ({
contacts: state.contacts.map(c => c.id === group.id ? { ...c, members } : c),
}));
void contactHooks.onContactGroupMemberChange.emit({ groupId: group.id, added: [keep.id], removed: others.map(o => o.id) });
}
groups.push({ groupId: group.id, members: { ...(group.members ?? {}) } });
set((state) => ({
contacts: state.contacts.map(c => c.id === group.id ? { ...c, members } : c),
}));
void contactHooks.onContactGroupMemberChange.emit({ groupId: group.id, added: [keep.id], removed: others.map(o => o.id) });
} catch (error) {
// The kept card is changed already: hand back what Undo needs, delete nothing.
console.error('Failed to move group members while merging:', error);
return { keepId: keep.id, before, others: [], groups, incomplete: true };
}
await get().bulkDeleteContacts(sync ? client : null, others.map(o => o.id));
return { keepId: keep.id, before, others, groups };
// A failed delete, or a plugin that stopped it, leaves cards in place:
// Undo must not create those again.
const left = new Set(get().contacts.map(c => c.id));
const deleted = others.filter(o => !left.has(o.id));
return { keepId: keep.id, before, others: deleted, groups, ...(deleted.length < others.length ? { incomplete: true } : {}) };
},
undoMergeContacts: async (client, undo) => {