diff --git a/stores/__tests__/contact-store.test.ts b/stores/__tests__/contact-store.test.ts index f20e6a4b3..89da2d6d4 100644 --- a/stores/__tests__/contact-store.test.ts +++ b/stores/__tests__/contact-store.test.ts @@ -806,6 +806,156 @@ describe('contact-store', () => { }); }); + describe('address-book id namespacing on mutations (#133, #1043)', () => { + const makeClient = () => ({ + getContactsAccountId: vi.fn(() => 'acct-primary'), + createContact: vi.fn(async (c: Partial) => ({ ...makeContact(), ...c, id: 'created-id' })), + updateContact: vi.fn().mockResolvedValue(undefined), + deleteContact: vi.fn().mockResolvedValue(undefined), + }); + + it('strips the local-account prefix from a personal book on update', async () => { + // Multi-account Pro mode namespaces EVERY book, including the active + // account's own default one, as `local::bookId`. Updating such a contact + // used to send the namespaced id and Stalwart rejected the card with + // "Contact has to belong to at least one address book". + useContactStore.setState({ + addressBooks: [ + { id: 'me@host::ab-1', originalId: 'ab-1', localAccountId: 'me@host', name: 'Personal', isDefault: true }, + ], + contacts: [makeContact({ + id: 'me@host::c-1', + originalId: 'c-1', + localAccountId: 'me@host', + addressBookIds: { 'me@host::ab-1': true }, + })], + }); + const client = makeClient(); + + await useContactStore.getState().updateContact( + client as unknown as IJMAPClient, + 'me@host::c-1', + { name: { components: [{ kind: 'given', value: 'Jane' }], isOrdered: true }, addressBookIds: { 'me@host::ab-1': true } }, + ); + + expect(client.updateContact).toHaveBeenCalledWith( + 'c-1', + expect.objectContaining({ addressBookIds: { 'ab-1': true } }), + undefined, + ); + // Local state keeps the namespaced form the sidebar filters by. + expect(useContactStore.getState().contacts[0].addressBookIds).toEqual({ 'me@host::ab-1': true }); + }); + + it('strips both local-account and shared-account prefixes on update', async () => { + useContactStore.setState({ + addressBooks: [ + { id: 'me@host::group-1:ab-7', originalId: 'ab-7', localAccountId: 'me@host', name: 'Team', isShared: true, accountId: 'group-1' }, + ], + contacts: [makeContact({ + id: 'me@host::group-1:c-9', + originalId: 'c-9', + localAccountId: 'me@host', + isShared: true, + accountId: 'group-1', + addressBookIds: { 'me@host::group-1:ab-7': true }, + })], + }); + const client = makeClient(); + + await useContactStore.getState().updateContact( + client as unknown as IJMAPClient, + 'me@host::group-1:c-9', + { addressBookIds: { 'me@host::group-1:ab-7': true } }, + ); + + expect(client.updateContact).toHaveBeenCalledWith('c-9', { addressBookIds: { 'ab-7': true } }, 'group-1'); + }); + + it('falls back to prefix stripping when the book is not in state', async () => { + useContactStore.setState({ + addressBooks: [], + contacts: [makeContact({ + id: 'me@host::group-1:c-9', + originalId: 'c-9', + localAccountId: 'me@host', + isShared: true, + accountId: 'group-1', + })], + }); + const client = makeClient(); + + await useContactStore.getState().updateContact( + client as unknown as IJMAPClient, + 'me@host::group-1:c-9', + { addressBookIds: { 'me@host::group-1:ab-7': true } }, + ); + + expect(client.updateContact).toHaveBeenCalledWith('c-9', { addressBookIds: { 'ab-7': true } }, 'group-1'); + }); + + it('leaves raw ids untouched in single-account mode', async () => { + useContactStore.setState({ + addressBooks: [{ id: 'ab-1', name: 'Personal', isDefault: true }], + contacts: [makeContact({ id: 'c-1' })], + }); + const client = makeClient(); + + await useContactStore.getState().updateContact( + client as unknown as IJMAPClient, + 'c-1', + { addressBookIds: { 'ab-1': true } }, + ); + + expect(client.updateContact).toHaveBeenCalledWith('c-1', { addressBookIds: { 'ab-1': true } }, undefined); + }); + + it('strips the local-account prefix on create too', async () => { + useContactStore.setState({ + addressBooks: [ + { id: 'me@host::ab-1', originalId: 'ab-1', localAccountId: 'me@host', name: 'Personal', isDefault: true }, + ], + }); + const client = makeClient(); + + await useContactStore.getState().createContact( + client as unknown as IJMAPClient, + { addressBookIds: { 'me@host::ab-1': true } }, + ); + + expect(client.createContact).toHaveBeenCalledWith( + expect.objectContaining({ addressBookIds: { 'ab-1': true } }), + undefined, + ); + }); + + it('moves to a namespaced personal book with the raw id and keeps the display id locally', async () => { + const target = { id: 'me@host::ab-2', originalId: 'ab-2', localAccountId: 'me@host', name: 'Work' }; + useContactStore.setState({ + addressBooks: [ + { id: 'me@host::ab-1', originalId: 'ab-1', localAccountId: 'me@host', name: 'Personal', isDefault: true }, + target, + ], + contacts: [makeContact({ + id: 'me@host::c-1', + originalId: 'c-1', + localAccountId: 'me@host', + addressBookIds: { 'me@host::ab-1': true }, + })], + }); + const client = makeClient(); + + await useContactStore.getState().moveContactToAddressBook( + client as unknown as IJMAPClient, + ['me@host::c-1'], + target, + ); + + expect(client.updateContact).toHaveBeenCalledWith('c-1', { addressBookIds: { 'ab-2': true } }, undefined); + expect(useContactStore.getState().contacts[0].addressBookIds).toEqual({ 'me@host::ab-2': true }); + }); + }); + describe('persistence/partialize', () => { it('should persist contacts when supportsSync is false', () => { const { partialize } = (useContactStore as unknown as { persist: { getOptions: () => { partialize: (state: Record) => Record } } }).persist.getOptions(); diff --git a/stores/contact-store.ts b/stores/contact-store.ts index 77e2e51f4..3ea3e98a3 100644 --- a/stores/contact-store.ts +++ b/stores/contact-store.ts @@ -122,6 +122,41 @@ function stripLocalAccountPrefix(id: string, localAccountId?: string): string { return id.startsWith(prefix) ? id.slice(prefix.length) : id; } +/** + * Map the address-book ids the UI works with (namespaced per shared account + * as `accountId:bookId`, and per local account as `local::bookId` in + * multi-account Pro mode - possibly both) back to the raw server ids that + * `ContactCard/set` accepts. Resolution goes through the loaded address-book + * list first, since every book carries its `originalId`; ids that are not in + * the list (stale state, tests) fall back to stripping the known prefixes. + * Sending a namespaced id makes Stalwart reject the card with "Contact has + * to belong to at least one address book" (#133, #1043). + */ +function resolveRawAddressBookIds( + addressBookIds: Record, + books: AddressBook[], + owner?: { localAccountId?: string; accountId?: string; isShared?: boolean }, +): { addressBookIds: Record; sharedAccountId?: string; localAccountId?: string } { + const raw: Record = {}; + let sharedAccountId: string | undefined; + let localAccountId = owner?.localAccountId; + for (const [bookId, value] of Object.entries(addressBookIds)) { + const book = books.find(b => b.id === bookId); + if (book?.localAccountId) localAccountId = book.localAccountId; + if (book?.isShared && book.accountId) sharedAccountId = book.accountId; + if (book?.originalId) { + raw[book.originalId] = value; + continue; + } + let stripped = stripLocalAccountPrefix(bookId, localAccountId); + if (owner?.isShared && owner.accountId && stripped.startsWith(`${owner.accountId}:`)) { + stripped = stripped.slice(owner.accountId.length + 1); + } + raw[stripped] = value; + } + return { addressBookIds: raw, sharedAccountId, localAccountId }; +} + export function getContactDisplayName(contact: ContactCard): string { if (contact.name) { // Try given + surname from components first @@ -508,28 +543,15 @@ export const useContactStore = create()( let cleanedContact = contact; let localAccountId = contact.localAccountId; - // De-namespace addressBookIds if they reference a shared address book + // De-namespace addressBookIds (shared and/or local-account prefixes) if (contact.addressBookIds) { - const books = get().addressBooks; - const deNamespaced: Record = {}; - let sharedAccountId: string | undefined; - for (const [bookId, value] of Object.entries(contact.addressBookIds)) { - const book = books.find(b => b.id === bookId); - if (book?.localAccountId) localAccountId = book.localAccountId; - if (book?.isShared && book.originalId) { - deNamespaced[book.originalId] = value; - sharedAccountId = book.accountId; - } else if (book?.originalId) { - deNamespaced[book.originalId] = value; - } else { - deNamespaced[bookId] = value; - } - } - if (sharedAccountId) { - accountId = sharedAccountId; - cleanedContact = { ...contact, addressBookIds: deNamespaced, isShared: true, accountId: sharedAccountId }; + const resolved = resolveRawAddressBookIds(contact.addressBookIds, get().addressBooks, contact); + localAccountId = resolved.localAccountId; + if (resolved.sharedAccountId) { + accountId = resolved.sharedAccountId; + cleanedContact = { ...contact, addressBookIds: resolved.addressBookIds, isShared: true, accountId: resolved.sharedAccountId }; } else { - cleanedContact = { ...contact, addressBookIds: deNamespaced }; + cleanedContact = { ...contact, addressBookIds: resolved.addressBookIds }; } } @@ -562,17 +584,13 @@ export const useContactStore = create()( const accountId = contact?.isShared ? contact.accountId : undefined; client = resolveAccountClient(client, contact?.localAccountId); - // De-namespace addressBookIds for shared contacts before sending to JMAP server + // De-namespace addressBookIds before sending to the JMAP server. This + // applies to personal books too: in multi-account Pro mode every id + // carries the `local::` prefix, and the server rejects it (#1043). let cleanedUpdates = updates; - if (contact?.isShared && contact?.accountId && updates.addressBookIds) { - const prefix = `${contact.accountId}:`; - const deNamespaced = Object.fromEntries( - Object.entries(updates.addressBookIds).map(([k, v]) => [ - k.startsWith(prefix) ? k.slice(prefix.length) : k, - v - ]) - ); - cleanedUpdates = { ...updates, addressBookIds: deNamespaced }; + if (updates.addressBookIds) { + const resolved = resolveRawAddressBookIds(updates.addressBookIds, get().addressBooks, contact); + cleanedUpdates = { ...updates, addressBookIds: resolved.addressBookIds }; } await client.updateContact(originalId, cleanedUpdates, accountId); @@ -969,12 +987,13 @@ export const useContactStore = create()( // Same account: just update the addressBookIds if ((sourceAccountId || primaryAccountId) === (targetAccountId || primaryAccountId)) { - await client.updateContact(originalId, { addressBookIds: { [targetBookOriginalId]: true } }, sourceAccountId); - const isTargetPrimary = !targetAccountId || targetAccountId === primaryAccountId; - const localBookId = isTargetPrimary ? targetBookOriginalId : `${targetAccountId}:${targetBookOriginalId}`; + const ownerClient = resolveAccountClient(client, contact.localAccountId); + await ownerClient.updateContact(originalId, { addressBookIds: { [targetBookOriginalId]: true } }, sourceAccountId); + // Keep the id in the same (possibly namespaced) form the sidebar + // filters by; `addressBook.id` already is that form. set((state) => ({ contacts: state.contacts.map(c => - c.id === id ? { ...c, addressBookIds: { [localBookId]: true } } : c + c.id === id ? { ...c, addressBookIds: { [addressBook.id]: true } } : c ), })); } else {