fix(contacts): use ical.js group parameter for custom labels

src/components/ContactDetails/ContactDetailsProperty.vue

@@ -131,11 +131,6 @@ export default {
 		 * @return {string}
 		 */
 		propName() {
-			// ! is this a ITEMXX.XXX property??
-			if (this.propGroup[1]) {
-				return this.propGroup[1]
-			}
-
 			return this.property.name
 		},
 
@@ -193,22 +188,27 @@ export default {
 		},
 
 		/**
-		 * Return the id and type of a property group
-		 * e.g ITEMXX.tel => ['ITEMXX', 'tel']
+		 * Return the group of the property if any
+		 * e.g NEXTCLOUD1.TEL => 'nextcloud1'
 		 *
-		 * @return {Array}
+		 * @return {string|undefined}
 		 */
-		propGroup() {
-			return this.property.name.split('.')
+		propGroupName() {
+			return this.property.getParameter('group')
 		},
 
 		/**
 		 * Return the associated X-ABLABEL if any
 		 *
-		 * @return {ICAL.Property}
+		 * @return {ICAL.Property|undefined}
 		 */
 		propLabel() {
-			return this.localContact.vCard.getFirstProperty(`${this.propGroup[0]}.x-ablabel`)
+			if (!this.propGroupName) {
+				return undefined
+			}
+			return this.localContact.vCard
+				.getAllProperties('x-ablabel')
+				.find((label) => label.getParameter('group') === this.propGroupName)
 		},
 
 		/**
@@ -272,18 +272,21 @@ export default {
 				} else {
 					// ical.js take types as arrays
 					this.type = data.id.split(',')
+
 					// only one can coexist
-					this.localContact.vCard.removeProperty(`${this.propGroup[0]}.x-ablabel`)
-
-					// checking if there is any other property in this group
-					const groups = this.localContact.jCal[1]
-						.map((prop) => prop[0])
-						.filter((name) => name.startsWith(`${this.propGroup[0]}.`))
-					if (groups.length === 1) {
-						// then this prop is the latest of its group
-						// -> converting back to simple prop
-						// eslint-disable-next-line vue/no-mutating-props
-						this.property.jCal[0] = this.propGroup[1]
+					if (this.propLabel) {
+						this.localContact.vCard.removeProperty(this.propLabel)
+					}
+
+					if (this.propGroupName) {
+						// checking if there is any other property in this group
+						const members = this.localContact.jCal[1]
+							.filter((prop) => prop[1].group === this.propGroupName)
+						if (members.length === 1) {
+							// then this prop is the latest of its group
+							// -> converting back to simple prop
+							this.property.removeParameter('group')
+						}
 					}
 				}
 			},

src/mixins/PropertyMixin.js

@@ -4,7 +4,6 @@
  */
 import debounce from 'debounce'
 import Contact from '../models/contact.js'
-import { setPropertyAlias } from '../services/updateDesignSet.js'
 
 export default {
 	props: {
@@ -122,29 +121,34 @@ export default {
 		}, 500),
 
 		createLabel(label) {
-			let propGroup = this.property.name
-			if (!this.property.name.startsWith('nextcloud')) {
-				propGroup = `nextcloud${this.getNcGroupCount() + 1}.${this.property.name}`
-				this.property.jCal[0] = propGroup
+			let group = this.property.getParameter('group')
+			if (!group) {
+				group = `nextcloud${this.getNcGroupCount() + 1}`
+				this.property.setParameter('group', group)
 			}
-			const group = propGroup.split('.')[0]
-			const name = propGroup.split('.')[1]
 
-			this.localContact.vCard.addPropertyWithValue(`${group}.x-ablabel`, label.name)
-
-			// force update the main design sets
-			setPropertyAlias(name, propGroup)
+			const labelProperty = this.localContact.vCard
+				.getAllProperties('x-ablabel')
+				.find((property) => property.getParameter('group') === group)
+			if (labelProperty) {
+				labelProperty.setValue(label.name)
+			} else {
+				this.localContact.vCard
+					.addPropertyWithValue('x-ablabel', label.name)
+					.setParameter('group', group)
+			}
 
 			this.$emit('update')
 		},
 
 		getNcGroupCount() {
-			const props = this.localContact.jCal[1]
-				.map((prop) => prop[0].split('.')[0]) // itemxxx.adr => itemxxx
-				.filter((name) => name.startsWith('nextcloud')) // filter nextcloudxxx.adr
-				.map((prop) => parseInt(prop.split('nextcloud')[1])) // nextcloudxxx => xxx
-			return props.length > 0
-				? Math.max.apply(null, props) // get max iteration of nextcloud grouped props
+			const counts = this.localContact.jCal[1]
+				.map((prop) => prop[1].group) // property group, e.g. nextcloudxxx
+				.filter((group) => group && group.startsWith('nextcloud'))
+				.map((group) => parseInt(group.slice('nextcloud'.length))) // nextcloudxxx => xxx
+				.filter((count) => !isNaN(count))
+			return counts.length > 0
+				? Math.max.apply(null, counts) // get max iteration of nextcloud grouped props
 				: 0
 		},
 	},

src/models/contact.js

@@ -41,14 +41,12 @@ export default class Contact {
 			throw new Error('Invalid vCard')
 		}
 
-		let jCal = ICAL.parse(vcard)
+		const jCal = ICAL.parse(vcard)
 		if (jCal[0] !== 'vcard') {
 			throw new Error('Only one contact is allowed in the vcard data')
 		}
 
-		if (updateDesignSet(jCal)) {
-			jCal = ICAL.parse(vcard)
-		}
+		updateDesignSet()
 
 		this.jCal = jCal
 		this.addressbook = addressbook

src/services/updateDesignSet.js

@@ -4,88 +4,12 @@
  */
 import ICAL from 'ical.js'
 
-/**
- * Some clients group properties by naming them something like 'ITEM1.URL'.
- * These should be treated the same as their original (i.e. 'URL' in this
- * example), so we iterate through the vCard to find these properties and
- * add them to the ical.js design set.
- *
- * { @link https://github.com/nextcloud/contacts/issues/42 }
- *
- * @param {Array} vCard The ical.js vCard
- * @return {boolean} Whether or not the design set has been altered.
- */
-function addGroupedProperties(vCard) {
-	let madeChanges = false
-	vCard[1].forEach((prop) => {
-		const propGroup = prop[0].split('.')
-
-		// if this is a grouped property, update the designSet
-		if (propGroup.length === 2) {
-			madeChanges = setPropertyAlias(propGroup[1], prop[0])
-		}
-	})
-	return madeChanges
-}
-
 /**
  * Fixes misbehaviour with TYPE quotes and separated commas
  * Seems to have been introduced with https://github.com/mozilla-comm/ical.js/pull/387
- *
- * @return {boolean} Whether or not the design set has been altered.
  */
-function setTypeMultiValueSeparateDQuote() {
-	if (
-		!ICAL.design.vcard.param.type
-		|| ICAL.design.vcard.param.type.multiValueSeparateDQuote !== false
-		|| !ICAL.design.vcard3.param.type
-		|| ICAL.design.vcard3.param.type.multiValueSeparateDQuote !== false
-	) {
-		// https://github.com/mozilla-comm/ical.js/blob/ba8e2522ffd30ffbe65197a96a487689d6e6e9a1/lib/ical/stringify.js#L121
-		ICAL.design.vcard.param.type.multiValueSeparateDQuote = false
-		ICAL.design.vcard3.param.type.multiValueSeparateDQuote = false
-
-		return true
-	}
-
-	return false
-}
-
-/**
-/**
- * Check whether the ical.js design sets need updating (and if so, do it)
- *
- * @param {Array} vCard The ical.js vCard
- * @return {boolean} Whether or not the design set has been altered.
- */
-export default function(vCard) {
-	let madeChanges = false
-
-	madeChanges |= setTypeMultiValueSeparateDQuote()
-	madeChanges |= addGroupedProperties(vCard)
-
-	return madeChanges
-}
-
-/**
- * @param {string} original Name of the property whose settings should be copied
- * @param {string} alias Name of the new property
- * @return {boolean} Whether or not the design set has been altered.
- */
-export function setPropertyAlias(original, alias) {
-	let madeChanges = false
-	original = original.toLowerCase()
-	alias = alias.toLowerCase()
-
-	if (ICAL.design.vcard.property[original]) {
-		ICAL.design.vcard.property[alias] = ICAL.design.vcard.property[original]
-		madeChanges = true
-	}
-
-	if (ICAL.design.vcard3.property[original]) {
-		ICAL.design.vcard3.property[alias] = ICAL.design.vcard3.property[original]
-		madeChanges = true
-	}
-
-	return madeChanges
+export default function updateDesignSet() {
+	// https://github.com/mozilla-comm/ical.js/blob/ba8e2522ffd30ffbe65197a96a487689d6e6e9a1/lib/ical/stringify.js#L121
+	ICAL.design.vcard.param.type.multiValueSeparateDQuote = false
+	ICAL.design.vcard3.param.type.multiValueSeparateDQuote = false
 }

tests/javascript/mixins/propertyMixin.test.js

@@ -0,0 +1,96 @@
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import PropertyMixin from '../../../src/mixins/PropertyMixin'
+import Contact from '../../../src/models/contact'
+
+const { createLabel, getNcGroupCount } = PropertyMixin.methods
+
+const buildContext = (contact, property) => ({
+	property,
+	localContact: contact,
+	getNcGroupCount,
+	$emit: jest.fn(),
+})
+
+describe('createLabel', () => {
+
+	let contact
+
+	beforeEach(() => {
+		contact = new Contact(`
+			BEGIN:VCARD
+			VERSION:3.0
+			UID:123456789-123465-123456-123456789
+			FN:Test contact
+			TEL;TYPE=WORK,VOICE:+4343849384
+			NEXTCLOUD1.TEL;TYPE=HOME,VOICE:+32382938
+			NEXTCLOUD1.X-ABLABEL:Old label
+			END:VCARD`.replace(/\t/gmi, '')
+		)
+	})
+
+	test('groups an ungrouped property and adds its label', () => {
+		const property = contact.vCard.getAllProperties('tel')
+			.find((prop) => !prop.getParameter('group'))
+		const context = buildContext(contact, property)
+
+		createLabel.call(context, { name: 'My label' })
+
+		expect(property.getParameter('group')).toEqual('nextcloud2')
+		const label = contact.vCard.getAllProperties('x-ablabel')
+			.find((prop) => prop.getParameter('group') === 'nextcloud2')
+		expect(label.getFirstValue()).toEqual('My label')
+		expect(context.$emit).toHaveBeenCalledWith('update')
+
+		const vcard = contact.toStringStripQuotes()
+		expect(vcard).toContain('NEXTCLOUD2.TEL;TYPE=WORK,VOICE:+4343849384')
+		expect(vcard).toContain('NEXTCLOUD2.X-ABLABEL:My label')
+	})
+
+	test('updates the label of an already grouped property', () => {
+		const property = contact.vCard.getAllProperties('tel')
+			.find((prop) => prop.getParameter('group') === 'nextcloud1')
+		const context = buildContext(contact, property)
+
+		createLabel.call(context, { name: 'New label' })
+
+		expect(property.getParameter('group')).toEqual('nextcloud1')
+		const labels = contact.vCard.getAllProperties('x-ablabel')
+		expect(labels.length).toEqual(1)
+		expect(labels[0].getFirstValue()).toEqual('New label')
+	})
+})
+
+describe('getNcGroupCount', () => {
+
+	test('returns the highest nextcloud group number', () => {
+		const contact = new Contact(`
+			BEGIN:VCARD
+			VERSION:3.0
+			UID:123456789-123465-123456-123456789
+			FN:Test contact
+			ITEM1.TEL:+1
+			NEXTCLOUD3.TEL:+2
+			NEXTCLOUD7.EMAIL:mail@example.com
+			END:VCARD`.replace(/\t/gmi, '')
+		)
+
+		expect(getNcGroupCount.call({ localContact: contact })).toEqual(7)
+	})
+
+	test('returns 0 when there is no nextcloud group', () => {
+		const contact = new Contact(`
+			BEGIN:VCARD
+			VERSION:3.0
+			UID:123456789-123465-123456-123456789
+			FN:Test contact
+			TEL:+1
+			END:VCARD`.replace(/\t/gmi, '')
+		)
+
+		expect(getNcGroupCount.call({ localContact: contact })).toEqual(0)
+	})
+})

tests/javascript/models/contact.test.js

@@ -60,3 +60,37 @@ describe('Test stripping quotes from TYPE', () => {
 	})
 
 })
+
+describe('Grouped properties (custom labels)', () => {
+
+	const vcard = `
+		BEGIN:VCARD
+		VERSION:3.0
+		UID:123456789-123465-123456-123456789
+		FN:Test contact
+		NEXTCLOUD1.TEL;TYPE=HOME,VOICE:+32382938
+		NEXTCLOUD1.X-ABLABEL:Custom label
+		END:VCARD`.replace(/\t/gmi, '')
+
+	test('grouped properties parse into the group parameter form', () => {
+		const contact = new Contact(vcard)
+
+		const tel = contact.vCard.getFirstProperty('tel')
+		expect(tel.getParameter('group')).toEqual('nextcloud1')
+		expect(tel.getFirstValue()).toEqual('+32382938')
+
+		const label = contact.vCard.getFirstProperty('x-ablabel')
+		expect(label.getParameter('group')).toEqual('nextcloud1')
+		expect(label.getFirstValue()).toEqual('Custom label')
+	})
+
+	test('grouped properties survive a serialization round trip', () => {
+		const contact = new Contact(vcard)
+
+		const telLine = getPropertyLines('NEXTCLOUD1.TEL', contact.toStringStripQuotes())[0]
+		expect(telLine).toStrictEqual('NEXTCLOUD1.TEL;TYPE=HOME,VOICE:+32382938')
+
+		const labelLine = getPropertyLines('NEXTCLOUD1.X-ABLABEL', contact.toStringStripQuotes())[0]
+		expect(labelLine).toStrictEqual('NEXTCLOUD1.X-ABLABEL:Custom label')
+	})
+})