fix: Sort by "last modified" (issue #5158)

src/components/ContactsList.vue

@@ -365,24 +365,7 @@ export default {
 			if (index === -1) {
 				return
 			}
-
-			const scroller = this.$refs.scroller
-			const scrollerBoundingRect = scroller.$el.getBoundingClientRect()
-			const item = document.getElementById(key.slice(0, -2))
-			const itemBoundingRect = item?.getBoundingClientRect()
-
-			// Try to scroll the item fully into view
-			if (!item || itemBoundingRect.y < scrollerBoundingRect.y) {
-				// Item is above the current scroll window (or partly overlapping)
-				scroller.scrollToIndex(index)
-			} else if (item) {
-				const itemHeight = scroller.getItemSize(index)
-				const pos = itemBoundingRect.y + itemHeight - (this.$el.offsetHeight + 50)
-				if (pos > 0) {
-					// Item is below the current scroll window (or partly overlapping)
-					scroller.scrollTo(scroller.scrollOffset + pos)
-				}
-			}
+			this.$refs.scroller.scrollToIndex(index, { align: 'nearest' })
 		},
 
 		/**

src/models/contact.js

@@ -8,7 +8,7 @@ import b64toBlob from 'b64-to-blob'
 import { Buffer } from 'buffer'
 import ICAL from 'ical.js'
 import { v4 as uuid } from 'uuid'
-import { shallowRef, unref } from 'vue'
+import { shallowRef, toRaw, unref } from 'vue'
 import logger from '../services/logger.js'
 import updateDesignSet from '../services/updateDesignSet.js'
 import store from '../store/index.js'
@@ -26,7 +26,7 @@ function isEmpty(value) {
 export const ContactKindProperties = ['KIND', 'X-ADDRESSBOOKSERVER-KIND']
 
 export const MinimalContactProperties = [
-	'EMAIL', 'UID', 'TEL', 'CATEGORIES', 'FN', 'ORG', 'N', 'X-PHONETIC-FIRST-NAME', 'X-PHONETIC-LAST-NAME', 'X-MANAGERSNAME', 'TITLE', 'NOTE', 'RELATED',
+	'EMAIL', 'UID', 'TEL', 'CATEGORIES', 'FN', 'ORG', 'N', 'X-PHONETIC-FIRST-NAME', 'X-PHONETIC-LAST-NAME', 'X-MANAGERSNAME', 'TITLE', 'NOTE', 'RELATED', 'REV',
 ].concat(ContactKindProperties)
 
 export function generateContactKey(uid, addressbookId) {
@@ -68,17 +68,6 @@ export default class Contact {
 			logger.info('This contact did not have a proper uid. Setting a new one for ', { contact: this })
 			this.vCard.addPropertyWithValue('uid', uuid())
 		}
-
-		// if no rev set, init one
-		if (!this.vCard.hasProperty('rev')) {
-			const version = this.vCard.getFirstPropertyValue('version')
-			if (version === '4.0') {
-				this.vCard.addPropertyWithValue('rev', ICAL.Time.fromJSDate(new Date(), true))
-			}
-			if (version === '3.0') {
-				this.vCard.addPropertyWithValue('rev', ICAL.VCardTime.fromDateAndOrTimeString(new Date().toISOString(), 'date-time'))
-			}
-		}
 	}
 
 	get vCard() {
@@ -196,7 +185,11 @@ export default class Contact {
 	 * @memberof Contact
 	 */
 	get rev() {
-		return this.vCard.getFirstPropertyValue('rev')
+		try {
+			return this.vCard.getFirstPropertyValue('rev')
+		} catch {
+			return null
+		}
 	}
 
 	/**
@@ -209,6 +202,39 @@ export default class Contact {
 		this.vCard.updatePropertyWithValue('rev', rev)
 	}
 
+	/**
+	 * REV as a unix timestamp (seconds), or null if missing or unparseable.
+	 * Reads the raw jCal value instead of going through ICAL.Time, which
+	 * throws on malformed REV values and broke sorting (see issue #5158).
+	 *
+	 * @readonly
+	 * @memberof Contact
+	 */
+	get revTimestamp() {
+		const raw = toRaw(this).vCard.getFirstProperty('rev')?.jCal?.[3]
+		if (typeof raw !== 'string') {
+			return null
+		}
+		// The jCal value may be basic ISO 8601 ("20260312T192500Z"), extended
+		// ("2026-03-12T19:25:00Z"), or carry misplaced separators from ical.js
+		// reformatting a REV with missing seconds ("2026-03-12T19::25Z").
+		// Split off the zone before stripping separators so the sign of a
+		// negative UTC offset survives, then rebuild the extended format for
+		// Date.parse. Seconds default to 00, a missing zone is treated as UTC.
+		const zoneMatch = raw.match(/(Z|[+-]\d{2}:?\d{2})$/)
+		const zone = zoneMatch?.[1].replace(':', '') ?? 'Z'
+		const datetime = zoneMatch ? raw.slice(0, -zoneMatch[1].length) : raw
+		const match = datetime.replace(/[-:]/g, '')
+			.match(/^(\d{4})(\d{2})(\d{2})T(\d{2})(\d{2})(\d{2})?$/)
+		if (!match) {
+			return null
+		}
+		const [, year, month, day, hour, minute, second = '00'] = match
+		const offset = zone.replace(/^([+-]\d{2})/, '$1:')
+		const timestamp = Date.parse(`${year}-${month}-${day}T${hour}:${minute}:${second}${offset}`)
+		return Number.isNaN(timestamp) ? null : Math.floor(timestamp / 1000)
+	}
+
 	/**
 	 * Return the key
 	 *

src/services/checks/index.js

@@ -5,12 +5,10 @@
 
 import badGenderType from './badGenderType.js'
 import duplicateTypes from './duplicateTypes.js'
-import invalidREV from './invalidREV.js'
 import missingFN from './missingFN.js'
 
 export default [
 	badGenderType,
 	duplicateTypes,
-	invalidREV,
 	missingFN,
 ]

src/store/contacts.js

@@ -5,6 +5,7 @@
 
 import { showError } from '@nextcloud/dialogs'
 import ICAL from 'ical.js'
+import { toRaw } from 'vue'
 import Contact from '../models/contact.js'
 import logger from '../services/logger.js'
 import validate from '../services/validate.js'
@@ -27,38 +28,48 @@ ICAL.design.vcard3.param.type.multiValueSeparateDQuote = true
 ICAL.design.vcard.param.type.multiValueSeparateDQuote = true
 
 function sortData(a, b) {
-	const nameA = typeof a.value === 'string'
-		? a.value.toUpperCase() // ignore upper and lowercase
-		: a.value.toUnixTime() // only other sorting we support is a vCardTime
-	const nameB = typeof b.value === 'string'
-		? b.value.toUpperCase() // ignore upper and lowercase
-		: b.value.toUnixTime() // only other sorting we support is a vCardTime
-
-	const score = nameA.localeCompare
-		? nameA.localeCompare(nameB)
-		: nameB - nameA
-	// if equal, fallback to the key
-	return score !== 0
-		? score
-		: a.key.localeCompare(b.key)
-}
-
-function sortByFavoriteAndName(a, b) {
 	// favorites always on top
 	if (a.favorite !== b.favorite) {
 		return a.favorite ? -1 : 1
 	}
-	// alphabetical within each group
-	if (!a.value && !b.value) {
-		return 0
+
+	const nameA = typeof a.value === 'string' ? a.value.toUpperCase() : a.value
+	const nameB = typeof b.value === 'string' ? b.value.toUpperCase() : b.value
+
+	// Push null/undefined values to the end
+	if (nameA === null && nameB === null) {
+		return a.key.localeCompare(b.key)
 	}
-	if (!a.value) {
+	if (nameA === null) {
 		return 1
 	}
-	if (!b.value) {
+	if (nameB === null) {
 		return -1
 	}
-	return a.value.localeCompare(b.value)
+
+	const score = typeof nameA === 'string'
+		? nameA.localeCompare(nameB)
+		: nameB - nameA // descending: newest first
+	return score !== 0 ? score : a.key.localeCompare(b.key)
+}
+
+function extractSortValue(contact, orderKey) {
+	if (orderKey === 'rev') {
+		return contact.revTimestamp
+	}
+	const val = contact[orderKey]
+	if (val === null || val === undefined) {
+		return null
+	}
+	if (typeof val === 'string') {
+		return val
+	}
+	// ical.js methods don't work through Vue's reactive proxy; unwrap first.
+	try {
+		return toRaw(val).toUnixTime()
+	} catch {
+		return null
+	}
 }
 
 const state = {
@@ -113,10 +124,10 @@ const mutations = {
 			.filter((c) => c.kind !== 'group')
 			.map((c) => ({
 				key: c.key,
-				value: (c[state.orderKey] || '').toString().toLowerCase(),
+				value: extractSortValue(c, state.orderKey),
 				favorite: c.favorite || false,
 			}))
-			.sort(sortByFavoriteAndName)
+			.sort(sortData)
 	},
 	/**
 	 * Delete a contact from the global contacts list
@@ -147,8 +158,8 @@ const mutations = {
 
 			const sortedContact = {
 				key: contact.key,
-				value: (contact[state.orderKey] || '').toString().toLowerCase(),
-				favorite: contact.favorite,
+				value: extractSortValue(contact, state.orderKey),
+				favorite: contact.favorite || false,
 			}
 
 			// Not using sort, splice has far better performances
@@ -187,9 +198,11 @@ const mutations = {
 	 * Update a contact
 	 *
 	 * @param {object} state the store data
-	 * @param {Contact} contact the contact to update
+	 * @param {object} payload destructuring object
+	 * @param {Contact} payload.contact the contact to update
+	 * @param {boolean} [payload.skipSort] skip the re-sort after updating (default false)
 	 */
-	updateContact(state, contact) {
+	updateContact(state, { contact, skipSort = false }) {
 		if (state.contacts[contact.key] && contact instanceof Contact) {
 			const existingFavorite = state.contacts[contact.key].dav?.favorite || false
 			state.contacts[contact.key].updateContact(contact.jCal)
@@ -199,21 +212,21 @@ const mutations = {
 				state.contacts[contact.key].dav.favorite = existingFavorite
 			}
 
-			const sortedContact = state.sortedContacts.find((search) => search.key === contact.key)
+			if (!skipSort) {
+				const sortedContact = state.sortedContacts.find((search) => search.key === contact.key)
 
-			if (!sortedContact) {
-				logger.warn('sortedContact not found for', { key: contact.key })
-				return
-			}
-
-			const hasValueChanged = sortedContact.value !== contact[state.orderKey]
-			const hasFavoriteChanged = sortedContact.favorite !== (state.contacts[contact.key].dav?.favorite || false)
-
-			if (hasValueChanged || hasFavoriteChanged) {
-				sortedContact.value = contact[state.orderKey]
-				sortedContact.favorite = state.contacts[contact.key].dav?.favorite || false
+				if (!sortedContact) {
+					logger.warn('sortedContact not found for', { key: contact.key })
+					return
+				}
 
-				state.sortedContacts.sort(sortByFavoriteAndName)
+				const newValue = extractSortValue(contact, state.orderKey)
+				const newFavorite = state.contacts[contact.key].dav?.favorite || false
+				if (sortedContact.value !== newValue || sortedContact.favorite !== newFavorite) {
+					sortedContact.value = newValue
+					sortedContact.favorite = newFavorite
+					state.sortedContacts.sort(sortData)
+				}
 			}
 		} else {
 			logger.error('Error while replacing the following contact', { contact })
@@ -250,7 +263,7 @@ const mutations = {
 			// Update sorted contacts list, replace at exact same position
 			const index = state.sortedContacts.findIndex((search) => search.key === oldKey)
 			state.sortedContacts[index].key = newContact.key
-			state.sortedContacts[index].value = newContact[state.orderKey]
+			state.sortedContacts[index].value = extractSortValue(newContact, state.orderKey)
 		} else {
 			logger.error('Error while replacing the addressbook of following contact', { contact })
 		}
@@ -287,10 +300,10 @@ const mutations = {
 			.filter((contact) => contact.kind !== 'group')
 			.map((contact) => ({
 				key: contact.key,
-				value: contact[state.orderKey],
+				value: extractSortValue(contact, state.orderKey),
 				favorite: contact.favorite || false,
 			}))
-			.sort(sortByFavoriteAndName)
+			.sort(sortData)
 	},
 
 	/**
@@ -442,7 +455,7 @@ const actions = {
 			try {
 				await contact.dav.update()
 				// all clear, let's update the store
-				context.commit('updateContact', contact)
+				context.commit('updateContact', { contact })
 			} catch (error) {
 				logger.error(error)
 
@@ -485,7 +498,10 @@ const actions = {
 				const newContact = new Contact(davObject.data, contact.addressbook)
 				newContact.dav = davObject
 				newContact.dav.favorite = savedFavorite
-				context.commit('updateContact', newContact)
+				// skipSort: opening a contact must not visibly reorder the list.
+				// The server's REV rarely differs from the cached one here; if it
+				// does, the next mutation will re-sort.
+				context.commit('updateContact', { contact: newContact, skipSort: true })
 			})
 			.catch((error) => { throw error })
 	},