Skip to content

fix(store): keep new contacts in the sorted index #5708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hamza221 wants to merge 1 commit into
base: main
Choose a base branch
from
+113 −57
Open

fix(store): keep new contacts in the sorted index #5708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 38 additions & 57 deletions src/store/contacts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,33 @@ function sortData(a, b) {
: a.key.localeCompare(b.key)
}

function sortByFavoriteAndName(a, b) {
// favorites always on top
/**
* Favorites first, then by the current order key.
*
* @param {object} a a sorted contacts index entry
* @param {object} b a sorted contacts index entry
* @return {number}
*/
function sortByFavoriteAndData(a, b) {
if (a.favorite !== b.favorite) {
return a.favorite ? -1 : 1
}
// alphabetical within each group
if (!a.value && !b.value) {
return 0
}
if (!a.value) {
return 1
}
if (!b.value) {
return -1
return sortData(a, b)
}

/**
* Build an entry of the sorted contacts index
*
* @param {Contact} contact the contact to index
* @param {string} orderKey the contact property to sort on
* @return {object}
*/
function sortedEntry(contact, orderKey) {
return {
key: contact.key,
value: contact[orderKey] || '',
favorite: contact.favorite || false,
}
return a.value.localeCompare(b.value)
}

const state = {
Expand Down Expand Up @@ -111,12 +122,8 @@ const mutations = {

state.sortedContacts = Object.values(state.contacts)
.filter((c) => c.kind !== 'group')
.map((c) => ({
key: c.key,
value: (c[state.orderKey] || '').toString().toLowerCase(),
favorite: c.favorite || false,
}))
.sort(sortByFavoriteAndName)
.map((c) => sortedEntry(c, state.orderKey))
.sort(sortByFavoriteAndData)
},
/**
* Delete a contact from the global contacts list
Expand Down Expand Up @@ -145,36 +152,15 @@ const mutations = {
if (contact instanceof Contact) {
validate(contact)

const sortedContact = {
key: contact.key,
value: (contact[state.orderKey] || '').toString().toLowerCase(),
favorite: contact.favorite,
}
const sortedContact = sortedEntry(contact, state.orderKey)

// Not using sort, splice has far better performances
// https://jsperf.com/sort-vs-splice-in-array
for (let i = 0, len = state.sortedContacts.length; i < len; i++) {
const other = state.sortedContacts[i]

// favorite comes before non-favorite
const differentFavStatus = other.favorite !== sortedContact.favorite
const otherShouldComeFirst = differentFavStatus && other.favorite
const sameFavAndSortedFirst = !differentFavStatus && sortData(other, sortedContact) >= 0

if (otherShouldComeFirst || sameFavAndSortedFirst) {
continue
}

if (i + 1 === len) {
state.sortedContacts.push(sortedContact)
} else {
state.sortedContacts.splice(i, 0, sortedContact)
}
break
}

if (state.sortedContacts.length === 0) {
const index = state.sortedContacts.findIndex((other) => sortByFavoriteAndData(other, sortedContact) >= 0)
if (index === -1) {
state.sortedContacts.push(sortedContact)
} else {
state.sortedContacts.splice(index, 0, sortedContact)
}

state.contacts[contact.key] = contact
Expand Down Expand Up @@ -206,14 +192,13 @@ const mutations = {
return
}

const hasValueChanged = sortedContact.value !== contact[state.orderKey]
const hasFavoriteChanged = sortedContact.favorite !== (state.contacts[contact.key].dav?.favorite || false)
const updatedEntry = sortedEntry(state.contacts[contact.key], state.orderKey)

if (hasValueChanged || hasFavoriteChanged) {
sortedContact.value = contact[state.orderKey]
sortedContact.favorite = state.contacts[contact.key].dav?.favorite || false
if (sortedContact.value !== updatedEntry.value || sortedContact.favorite !== updatedEntry.favorite) {
sortedContact.value = updatedEntry.value
sortedContact.favorite = updatedEntry.favorite

state.sortedContacts.sort(sortByFavoriteAndName)
state.sortedContacts.sort(sortByFavoriteAndData)
}
} else {
logger.error('Error while replacing the following contact', { contact })
Expand Down Expand Up @@ -250,7 +235,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 = newContact[state.orderKey] || ''
} else {
logger.error('Error while replacing the addressbook of following contact', { contact })
}
Expand Down Expand Up @@ -285,12 +270,8 @@ const mutations = {
sortContacts(state) {
state.sortedContacts = Object.values(state.contacts)
.filter((contact) => contact.kind !== 'group')
.map((contact) => ({
key: contact.key,
value: contact[state.orderKey],
favorite: contact.favorite || false,
}))
.sort(sortByFavoriteAndName)
.map((contact) => sortedEntry(contact, state.orderKey))
.sort(sortByFavoriteAndData)
},

/**
Expand Down
75 changes: 75 additions & 0 deletions tests/javascript/store/contactsMutations.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

// The store index has to be imported first to avoid a circular import issue
import store from '../../../src/store/index.js'
import Contact from '../../../src/models/contact.js'

const addressbook = { id: 'ab1', displayName: 'Address book', enabled: true, contacts: {} }

function contact(fullName) {
const c = new Contact(`BEGIN:VCARD
VERSION:4.0
FN:${fullName}
END:VCARD`, addressbook)
return c
}

describe('addContact mutation keeps every contact in the sorted list', () => {
beforeEach(() => {
store.state.contacts.contacts = {}
store.state.contacts.sortedContacts.splice(0)
})

const keys = () => store.state.contacts.sortedContacts.map((c) => c.value)

test('into an empty list', () => {
store.commit('addContact', contact('Name'))
expect(keys()).toEqual(['Name'])
})

test('when it sorts after the only existing contact', () => {
store.commit('addContact', contact('Alice'))
store.commit('addContact', contact('Name'))
expect(keys()).toEqual(['Alice', 'Name'])
})

test('when it sorts before the only existing contact', () => {
store.commit('addContact', contact('Zoe'))
store.commit('addContact', contact('Name'))
expect(keys()).toEqual(['Name', 'Zoe'])
})

// The reported regression: the only contacts in the store are account
// contacts from another address book, all sorting after the new contact's
// default name, and the new contact never made it into the index.
// https://github.com/nextcloud/contacts/issues/5681
test('when every existing contact sorts before it', () => {
for (let i = 0; i < 300; i++) {
store.commit('addContact', contact(`User00${700 + i} John (elysee)`))
}

const created = contact('Nom')
store.commit('addContact', created)

expect(store.getters.getSortedContacts.findIndex((c) => c.key === created.key)).toBe(0)
})

test('favorites stay on top', () => {
const favorite = contact('Zoe')
favorite.dav = { favorite: true }
store.commit('addContact', contact('Alice'))
store.commit('addContact', favorite)
store.commit('addContact', contact('Name'))
expect(keys()).toEqual(['Zoe', 'Alice', 'Name'])
})

test('when it sorts in the middle', () => {
store.commit('addContact', contact('Alice'))
store.commit('addContact', contact('Zoe'))
store.commit('addContact', contact('Name'))
expect(keys()).toEqual(['Alice', 'Name', 'Zoe'])
})
})
Loading