mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: save contacts with calendar links and cleared fields correctly
Stalwart rejects a whole ContactCard over one property it does not
know, and several reached it:
- the form sent `calendarUri`, `schedulingUri` and `freeBusyUri`,
so any contact with a calendar, scheduling or free/busy link failed
to save ("Invalid property.")
- vCard import sent `source`, and Pro mode left `localAccountId` on
the card
- vCard import wrote addresses as flat `street`/`locality` fields
instead of RFC 9553 `components`
The card now passes through `lib/jmap/contact-wire.ts` on its way to
the server: client-only fields are dropped, the links are written as
`calendars` / `schedulingAddresses` (and SOURCE as a `directories`
entry), flat addresses become components, and reads map the links back
for the form and detail view.
Clearing a field (nickname, notes, a link) never reached the server:
the form set it to `undefined`, which JSON drops, and an omitted
property keeps its value. Updates now send `null` for cleared keys.
Deleting an address book that still held contacts always failed with
`addressBookHasContents`. The delete now passes
`onDestroyRemoveContents`, which the confirm dialog already promised.
This commit is contained in:
@@ -0,0 +1,77 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { contactFromWire, contactToWire } from '../jmap/contact-wire';
|
||||
import type { ContactCard } from '../jmap/types';
|
||||
|
||||
describe('contactToWire', () => {
|
||||
it('drops client-only fields, which ContactCard/set rejects', () => {
|
||||
const wire = contactToWire({
|
||||
name: { full: 'A' },
|
||||
originalId: 'x', accountId: 'a', accountName: 'n', isShared: true, localAccountId: 'l',
|
||||
}, 'create');
|
||||
expect(wire).toEqual({ name: { full: 'A' } });
|
||||
});
|
||||
|
||||
it('writes the calendar URIs as RFC 9553 calendars and schedulingAddresses', () => {
|
||||
const wire = contactToWire({
|
||||
calendarUri: 'https://x.test/cal',
|
||||
freeBusyUri: 'https://x.test/fb',
|
||||
schedulingUri: 'mailto:a@x.test',
|
||||
}, 'create');
|
||||
expect(wire).toEqual({
|
||||
calendars: {
|
||||
cal: { '@type': 'Calendar', kind: 'calendar', uri: 'https://x.test/cal' },
|
||||
fb: { '@type': 'Calendar', kind: 'freeBusy', uri: 'https://x.test/fb' },
|
||||
},
|
||||
schedulingAddresses: { sched: { '@type': 'SchedulingAddress', uri: 'mailto:a@x.test' } },
|
||||
});
|
||||
});
|
||||
|
||||
it('sends null for cleared fields on update, but omits them on create', () => {
|
||||
const cleared: Partial<ContactCard> = { nicknames: undefined, calendarUri: undefined, schedulingUri: undefined };
|
||||
expect(contactToWire(cleared, 'update')).toEqual({ nicknames: null, calendars: null, schedulingAddresses: null });
|
||||
expect(contactToWire(cleared, 'create')).toEqual({});
|
||||
});
|
||||
|
||||
it('leaves calendar links alone when the update does not mention them', () => {
|
||||
expect(contactToWire({ keywords: { vip: true } }, 'update')).toEqual({ keywords: { vip: true } });
|
||||
});
|
||||
|
||||
it('converts flat vCard addresses to components and SOURCE to a directory entry', () => {
|
||||
const wire = contactToWire({
|
||||
addresses: { a0: { street: 'Main St 1', locality: 'Town', country: 'DE', fullAddress: 'Main St 1, Town', contexts: { work: true } } },
|
||||
source: 'https://x.test/card.vcf',
|
||||
}, 'create');
|
||||
expect(wire.addresses).toEqual({
|
||||
a0: {
|
||||
components: [
|
||||
{ kind: 'name', value: 'Main St 1' },
|
||||
{ kind: 'locality', value: 'Town' },
|
||||
{ kind: 'country', value: 'DE' },
|
||||
],
|
||||
isOrdered: true,
|
||||
defaultSeparator: ', ',
|
||||
full: 'Main St 1, Town',
|
||||
contexts: { work: true },
|
||||
},
|
||||
});
|
||||
expect(wire.directories).toEqual({ source: { '@type': 'Directory', kind: 'entry', uri: 'https://x.test/card.vcf' } });
|
||||
});
|
||||
});
|
||||
|
||||
describe('contactFromWire', () => {
|
||||
it('fills the flat URI fields from the server card', () => {
|
||||
const card = contactFromWire({
|
||||
id: 'c1',
|
||||
addressBookIds: {},
|
||||
calendars: { k: { kind: 'calendar', uri: 'https://x.test/cal' }, f: { kind: 'freeBusy', uri: 'https://x.test/fb' } },
|
||||
schedulingAddresses: { s: { uri: 'mailto:a@x.test' } },
|
||||
directories: { d: { kind: 'entry', uri: 'https://x.test/card.vcf' } },
|
||||
});
|
||||
expect(card).toMatchObject({
|
||||
calendarUri: 'https://x.test/cal',
|
||||
freeBusyUri: 'https://x.test/fb',
|
||||
schedulingUri: 'mailto:a@x.test',
|
||||
source: 'https://x.test/card.vcf',
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -391,7 +391,7 @@ export interface IJMAPClient {
|
||||
createAddressBook(name: string): Promise<AddressBook>;
|
||||
updateAddressBook(addressBookId: string, updates: Partial<AddressBook>, targetAccountId?: string): Promise<void>;
|
||||
setDefaultAddressBook(addressBookId: string, targetAccountId?: string): Promise<void>;
|
||||
deleteAddressBook(addressBookId: string, targetAccountId?: string): Promise<void>;
|
||||
deleteAddressBook(addressBookId: string, targetAccountId?: string, options?: { removeContents?: boolean }): Promise<void>;
|
||||
getContacts(addressBookId?: string, options?: { throwOnError?: boolean }): Promise<ContactCard[]>;
|
||||
getAllContacts(): Promise<ContactCard[]>;
|
||||
getContact(contactId: string, accountId?: string): Promise<ContactCard | null>;
|
||||
|
||||
+20
-11
@@ -14,6 +14,7 @@ import { findTasksOnlyCalendarIds, isTaskLikeObject, type ScannedCalendarObject
|
||||
import { DEFAULT_CALENDAR_COMPONENTS, mkCalendarCollection, newCalendarCollectionName } from "@/lib/webdav/calendar-collection";
|
||||
import { sanitizeDisplayName, splitMailbox } from "@/lib/rfc5322-mailbox";
|
||||
import { decodeFileNodeName } from "./filenode-name";
|
||||
import { contactFromWire, contactToWire } from "./contact-wire";
|
||||
import { getEffectiveTimeZone } from "@/lib/timezone";
|
||||
import { buildEmailSort, compareEmails, hasKeywordLevels, type KeywordSortPolarity, type SortLevel } from "@/lib/message-list-order";
|
||||
|
||||
@@ -5310,10 +5311,20 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
}
|
||||
|
||||
async deleteAddressBook(addressBookId: string, targetAccountId?: string): Promise<void> {
|
||||
async deleteAddressBook(
|
||||
addressBookId: string,
|
||||
targetAccountId?: string,
|
||||
options?: { removeContents?: boolean },
|
||||
): Promise<void> {
|
||||
const accountId = targetAccountId || this.getContactsAccountId();
|
||||
// Without onDestroyRemoveContents a book that still holds cards is
|
||||
// refused with `addressBookHasContents` (RFC 9610 §2.3).
|
||||
const response = await this.request([
|
||||
["AddressBook/set", { accountId, destroy: [addressBookId] }, "0"],
|
||||
["AddressBook/set", {
|
||||
accountId,
|
||||
destroy: [addressBookId],
|
||||
...(options?.removeContents ? { onDestroyRemoveContents: true } : {}),
|
||||
}, "0"],
|
||||
], this.contactUsing());
|
||||
|
||||
const result = response.methodResponses?.[0]?.[1];
|
||||
@@ -5675,7 +5686,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
if (response.methodResponses?.[0]?.[0] === "ContactCard/get") {
|
||||
const list = (response.methodResponses[0][1].list || []) as ContactCard[];
|
||||
allContacts.push(...list);
|
||||
allContacts.push(...list.map(contactFromWire));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5741,8 +5752,8 @@ export class JMAPClient implements IJMAPClient {
|
||||
], this.contactUsing());
|
||||
|
||||
if (response.methodResponses?.[0]?.[0] === "ContactCard/get") {
|
||||
const list = response.methodResponses[0][1].list || [];
|
||||
return list[0] || null;
|
||||
const list = (response.methodResponses[0][1].list || []) as ContactCard[];
|
||||
return list[0] ? contactFromWire(list[0]) : null;
|
||||
}
|
||||
return null;
|
||||
} catch (error) {
|
||||
@@ -5762,8 +5773,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
}
|
||||
|
||||
// Strip shared-only fields before sending to JMAP
|
||||
const { originalId: _oid, accountId: _aid, accountName: _an, isShared: _is, ...contactData } = contact as ContactCard;
|
||||
const contactData = contactToWire(contact, 'create');
|
||||
|
||||
const response = await this.request([
|
||||
["ContactCard/set", {
|
||||
@@ -5772,7 +5782,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
"new-contact": {
|
||||
...contactData,
|
||||
// Stalwart stores the card without one if omitted (#644)
|
||||
uid: contactData.uid || `urn:uuid:${generateUUID()}`,
|
||||
uid: contact.uid || `urn:uuid:${generateUUID()}`,
|
||||
addressBookIds,
|
||||
}
|
||||
}
|
||||
@@ -5800,8 +5810,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
async updateContact(contactId: string, updates: Partial<ContactCard>, targetAccountId?: string): Promise<void> {
|
||||
const accountId = targetAccountId || this.getContactsAccountId();
|
||||
|
||||
// Strip shared-only fields before sending to JMAP
|
||||
const { originalId: _oid, accountId: _aid, accountName: _an, isShared: _is, ...cleanUpdates } = updates as ContactCard;
|
||||
const cleanUpdates = contactToWire(updates, 'update');
|
||||
|
||||
const response = await this.request([
|
||||
["ContactCard/set", {
|
||||
@@ -5873,7 +5882,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
if (response.methodResponses?.[1]?.[0] === "ContactCard/get") {
|
||||
const rawContacts = (response.methodResponses[1][1].list || []) as ContactCard[];
|
||||
const contacts = rawContacts.map((contact) => ({
|
||||
const contacts = rawContacts.map(contactFromWire).map((contact) => ({
|
||||
...contact,
|
||||
id: isPrimary ? contact.id : `${accountId}:${contact.id}`,
|
||||
originalId: contact.id,
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
import type { AddressComponent, ContactAddress, ContactCard } from './types';
|
||||
|
||||
/**
|
||||
* Translation between the ContactCard shape the UI works with and the RFC 9553
|
||||
* card the server stores.
|
||||
*
|
||||
* ContactCard/set rejects a whole card over a single property it does not
|
||||
* know ("Invalid property."), so client-side fields must never reach it, and
|
||||
* the UI's flat conveniences are written as their JSContact equivalents.
|
||||
*/
|
||||
|
||||
/** Fields the client adds to cards it lists; not part of RFC 9553. */
|
||||
const CLIENT_ONLY_KEYS = [
|
||||
'originalId', 'accountId', 'accountName', 'isShared', 'localAccountId',
|
||||
] as const;
|
||||
|
||||
/** vCard CALURI / FBURL / CALADRURI, kept flat in the UI. */
|
||||
const URI_KEYS = ['calendarUri', 'freeBusyUri', 'schedulingUri', 'source'] as const;
|
||||
|
||||
const LEGACY_ADDRESS_FIELDS: Array<[keyof ContactAddress, AddressComponent['kind']]> = [
|
||||
['street', 'name'],
|
||||
['locality', 'locality'],
|
||||
['region', 'region'],
|
||||
['postcode', 'postcode'],
|
||||
['country', 'country'],
|
||||
];
|
||||
|
||||
/** vCard imports store flat fields; RFC 9553 wants `components`. */
|
||||
function addressToWire(address: ContactAddress): Record<string, unknown> {
|
||||
const { street: _s, locality: _l, region: _r, postcode: _p, country: _c, fullAddress, ...rest } = address;
|
||||
const out: Record<string, unknown> = { ...rest };
|
||||
if (!address.components?.length) {
|
||||
const components = LEGACY_ADDRESS_FIELDS
|
||||
.map(([field, kind]) => ({ kind, value: (address[field] as string | undefined)?.trim() ?? '' }))
|
||||
.filter(c => c.value);
|
||||
if (components.length) {
|
||||
out.components = components;
|
||||
out.isOrdered = true;
|
||||
out.defaultSeparator = ', ';
|
||||
}
|
||||
}
|
||||
if (fullAddress && !address.full) out.full = fullAddress;
|
||||
return out;
|
||||
}
|
||||
|
||||
type Mode = 'create' | 'update';
|
||||
|
||||
/**
|
||||
* The card as ContactCard/set expects it. On `update`, a key that is present
|
||||
* with the value `undefined` means "the user cleared it" and is sent as
|
||||
* `null`: JSON drops undefined, and an omitted property keeps its old value.
|
||||
*/
|
||||
export function contactToWire(card: Partial<ContactCard>, mode: Mode): Record<string, unknown> {
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [key, value] of Object.entries(card)) {
|
||||
if ((CLIENT_ONLY_KEYS as readonly string[]).includes(key)) continue;
|
||||
if ((URI_KEYS as readonly string[]).includes(key)) continue;
|
||||
if (value === undefined) {
|
||||
if (mode === 'update') out[key] = null;
|
||||
continue;
|
||||
}
|
||||
out[key] = value;
|
||||
}
|
||||
|
||||
if (card.addresses) {
|
||||
out.addresses = Object.fromEntries(
|
||||
Object.entries(card.addresses).map(([id, a]) => [id, addressToWire(a)]),
|
||||
);
|
||||
}
|
||||
|
||||
// Rebuild the calendar links only when the caller manages them, so an
|
||||
// update that does not mention them leaves the server's entries alone.
|
||||
if ('calendarUri' in card || 'freeBusyUri' in card) {
|
||||
const calendars: Record<string, unknown> = {};
|
||||
if (card.calendarUri) calendars.cal = { '@type': 'Calendar', kind: 'calendar', uri: card.calendarUri };
|
||||
if (card.freeBusyUri) calendars.fb = { '@type': 'Calendar', kind: 'freeBusy', uri: card.freeBusyUri };
|
||||
if (Object.keys(calendars).length) out.calendars = calendars;
|
||||
else if (mode === 'update') out.calendars = null;
|
||||
}
|
||||
if ('schedulingUri' in card) {
|
||||
if (card.schedulingUri) out.schedulingAddresses = { sched: { '@type': 'SchedulingAddress', uri: card.schedulingUri } };
|
||||
else if (mode === 'update') out.schedulingAddresses = null;
|
||||
}
|
||||
if (card.source) {
|
||||
// vCard SOURCE is a JSContact directory entry (RFC 9553 §2.6.2).
|
||||
const directories = { ...(card.directories ?? {}) } as Record<string, unknown>;
|
||||
directories.source = { '@type': 'Directory', kind: 'entry', uri: card.source };
|
||||
out.directories = directories;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Fill the UI's flat URI fields from the RFC 9553 properties. */
|
||||
export function contactFromWire<T extends ContactCard>(card: T): T {
|
||||
const calendars = Object.values(card.calendars ?? {});
|
||||
const calendarUri = card.calendarUri ?? calendars.find(c => c?.kind === 'calendar')?.uri;
|
||||
const freeBusyUri = card.freeBusyUri ?? calendars.find(c => c?.kind === 'freeBusy')?.uri;
|
||||
const schedulingUri = card.schedulingUri ?? Object.values(card.schedulingAddresses ?? {})[0]?.uri;
|
||||
const source = card.source ?? Object.values(card.directories ?? {}).find(d => d?.kind === 'entry')?.uri;
|
||||
if (!calendarUri && !freeBusyUri && !schedulingUri && !source) return card;
|
||||
return {
|
||||
...card,
|
||||
...(calendarUri ? { calendarUri } : {}),
|
||||
...(freeBusyUri ? { freeBusyUri } : {}),
|
||||
...(schedulingUri ? { schedulingUri } : {}),
|
||||
...(source ? { source } : {}),
|
||||
};
|
||||
}
|
||||
@@ -312,6 +312,11 @@ export interface ContactCard {
|
||||
grammaticalGender?: string;
|
||||
pronouns?: Record<string, { pronouns: string; pref?: number; contexts?: Record<string, boolean> }>;
|
||||
};
|
||||
calendars?: Record<string, { uri: string; kind?: 'calendar' | 'freeBusy'; mediaType?: string; contexts?: Record<string, boolean> }>;
|
||||
schedulingAddresses?: Record<string, { uri: string; contexts?: Record<string, boolean>; pref?: number }>;
|
||||
// Client-side flat views of `calendars`, `schedulingAddresses` and the
|
||||
// `directories` entry (vCard CALURI / FBURL / CALADRURI / SOURCE). They are
|
||||
// not RFC 9553 properties; lib/jmap/contact-wire.ts maps them.
|
||||
calendarUri?: string;
|
||||
schedulingUri?: string;
|
||||
freeBusyUri?: string;
|
||||
|
||||
@@ -1098,7 +1098,8 @@ export const useContactStore = create<ContactStore>()(
|
||||
const originalId = addressBook.originalId || stripLocalAccountPrefix(addressBook.id, addressBook.localAccountId);
|
||||
const accountId = addressBook.isShared ? addressBook.accountId : undefined;
|
||||
client = resolveAccountClient(client, addressBook.localAccountId);
|
||||
await client.deleteAddressBook(originalId, accountId);
|
||||
// The confirm dialog tells the user the contacts go with the book.
|
||||
await client.deleteAddressBook(originalId, accountId, { removeContents: true });
|
||||
set((state) => ({
|
||||
addressBooks: state.addressBooks.filter(b => b.id !== addressBook.id),
|
||||
contacts: state.contacts.filter(c => !c.addressBookIds?.[addressBook.id]),
|
||||
|
||||
Reference in New Issue
Block a user