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

src/components/ContactsList.vue

@@ -363,24 +363,7 @@ export default {
 			if (index === -1) {
 				return
 			}
-
-			const scroller = this.$refs.scroller
-			const scrollerBoundingRect = scroller.$el.getBoundingClientRect()
-			const item = this.$el.querySelector('#' + 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 updateDesignSet from '../services/updateDesignSet.js'
 import store from '../store/index.js'
 
@@ -25,7 +25,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 default class Contact {
@@ -63,17 +63,6 @@ export default class Contact {
 			console.info('This contact did not have a proper uid. Setting a new one for ', 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() {
@@ -178,7 +167,11 @@ export default class Contact {
 	 * @memberof Contact
 	 */
 	get rev() {
-		return this.vCard.getFirstPropertyValue('rev')
+		try {
+			return this.vCard.getFirstPropertyValue('rev')
+		} catch {
+			return null
+		}
 	}
 
 	/**
@@ -191,6 +184,41 @@ export default class Contact {
 		this.vCard.updatePropertyWithValue('rev', rev)
 	}
 
+	/**
+	 * REV as a unix timestamp (seconds), or null if missing/unparseable.
+	 * Bypasses ICAL.Time parsing so malformed-but-salvageable basic-ISO
+	 * strings ("20260312T192500Z", "T1925Z", trailing "T19:25Z") still sort.
+	 *
+	 * @readonly
+	 * @memberof Contact
+	 */
+	get revTimestamp() {
+		try {
+			const prop = toRaw(this).vCard.getFirstProperty('rev')
+			if (!prop) {
+				return null
+			}
+			const raw = prop.jCal[3]
+			if (!raw || typeof raw !== 'string') {
+				return null
+			}
+			let s = raw
+			if (/^\d{8}T/.test(s)) {
+				s = s.replace(/^(\d{4})(\d{2})(\d{2})T/, '$1-$2-$3T')
+			}
+			if (/T\d{6}/.test(s)) {
+				s = s.replace(/T(\d{2})(\d{2})(\d{2})/, 'T$1:$2:$3')
+			} else if (/T\d{4}[Z+-]/.test(s)) {
+				s = s.replace(/T(\d{2})(\d{2})([Z+-])/, 'T$1:$2:00$3')
+			}
+			s = s.replace(/T(\d{2}:\d{2}):?Z$/, 'T$1:00Z')
+			const ts = Date.parse(s)
+			return isNaN(ts) ? null : Math.floor(ts / 1000)
+		} catch {
+			return null
+		}
+	}
+
 	/**
 	 * 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 validate from '../services/validate.js'
 
@@ -26,20 +27,43 @@ 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
+	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 (nameA === null) {
+		return 1
+	}
+	if (nameB === null) {
+		return -1
+	}
+
+	const score = typeof nameA === 'string'
 		? nameA.localeCompare(nameB)
-		: nameB - nameA
-	// if equal, fallback to the key
-	return score !== 0
-		? score
-		: a.key.localeCompare(b.key)
+		: 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 = {
@@ -99,7 +123,7 @@ const mutations = {
 
 			const sortedContact = {
 				key: contact.key,
-				value: contact[state.orderKey],
+				value: extractSortValue(contact, state.orderKey),
 			}
 
 			// Not using sort, splice has far better performances
@@ -130,21 +154,25 @@ 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) {
 			// replace contact object data
 			state.contacts[contact.key].updateContact(contact.jCal)
-			const sortedContact = state.sortedContacts.find((search) => search.key === contact.key)
-
-			// has the sort key changed for this contact ?
-			const hasChanged = sortedContact.value !== contact[state.orderKey]
-			if (hasChanged) {
-				// then update the new data
-				sortedContact.value = contact[state.orderKey]
-				// and then we sort again
-				state.sortedContacts.sort(sortData)
+			if (!skipSort) {
+				const sortedContact = state.sortedContacts.find((search) => search.key === contact.key)
+
+				// has the sort key changed for this contact ?
+				const newValue = extractSortValue(contact, state.orderKey)
+				if (sortedContact.value !== newValue) {
+					// then update the new data
+					sortedContact.value = newValue
+					// and then we sort again
+					state.sortedContacts.sort(sortData)
+				}
 			}
 		} else {
 			console.error('Error while replacing the following contact', contact)
@@ -181,7 +209,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 {
 			console.error('Error while replacing the addressbook of following contact', contact)
 		}
@@ -217,7 +245,7 @@ const mutations = {
 		state.sortedContacts = Object.values(state.contacts)
 			// exclude groups
 			.filter((contact) => contact.kind !== 'group')
-			.map((contact) => { return { key: contact.key, value: contact[state.orderKey] } })
+			.map((contact) => { return { key: contact.key, value: extractSortValue(contact, state.orderKey) } })
 			.sort(sortData)
 	},
 
@@ -343,7 +371,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) {
 				console.error(error)
 
@@ -377,8 +405,12 @@ const actions = {
 		}
 		return contact.dav.fetchCompleteData(forceReFetch)
 			.then(() => {
-				const newContact = new Contact(contact.dav.data, contact.addressbook)
-				context.commit('updateContact', newContact)
+				const vcardData = contact.dav.data
+				const newContact = new Contact(vcardData, contact.addressbook)
+				// 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 })
 	},