Skip to content

fix(contacts): create new contacts in the selected address book #5764

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
whoalin1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(contacts): create new contacts in the selected address book #5764

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
23 changes: 22 additions & 1 deletion src/components/ContactDetails.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
<!-- org, title -->
<template #subtitle>
<template v-if="isReadOnly">
<span v-html="formattedSubtitle" />

Check warning on line 54 in src/components/ContactDetails.vue

View workflow job for this annotation

GitHub Actions / NPM lint 

'v-html' directive can lead to XSS attack
</template>
<template v-else>
<NcInputField
Expand Down Expand Up @@ -1264,10 +1264,31 @@
groupNames: this.newGroupsValue,
contact: this.contact,
})

// New contact + different address book: move locally before createVCard
// so Activity credits the final book (create-then-move logged the default).
// See https://github.com/nextcloud/contacts/issues/5757
const pendingAddressBookId = this.newAddressBook
const isNewContact = !this.contact.dav
if (
isNewContact
&& pendingAddressBookId
&& pendingAddressBookId !== this.contact.addressbook.id
) {
const addressbook = this.addressbooks.find((ab) => ab.id === pendingAddressBookId)
if (addressbook) {
this.updateAddressBookAccesses(pendingAddressBookId)
// createVCard uses localContact.addressbook — keep the clone in sync
this.localContact.addressbook = addressbook
await this.moveContactToAddressbook(pendingAddressBookId)

@coderabbitai coderabbitai Bot Oct 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major · Functional Correctness · Heavy lift

Supported by static analysis

Script executed:

grep -n "moveContactToAddressbook\|pendingAddressBook\|selectFirstContactIfNone" -r src | head -50
sed -n '1170,1240p' src/components/ContactDetails.vue

Repository: nextcloud/contacts

Length of output: 3980

Script executed:

grep -n "moveContactToAddressbook" -A60 src/components/ContactDetails.vue | sed -n '1,140p'

Repository: nextcloud/contacts

Length of output: 4809

Script executed:

set -e
printf '%s\n' '--- store move action ---'
sed -n '500,610p' src/store/contacts.js
printf '%s\n' '--- contact creation/update bindings ---'
rg -n -F --glob '*.js' -e 'createVCard' -e 'updateContact' -e 'moveContactToAddressbook' src/store src/components/ContactDetails.vue
printf '%s\n' '--- matching store declarations ---'
grep -n -E '^[[:space:]]*(async )?(createVCard|updateContact|moveContactToAddressbook)|createVCard|updateContact' src/store/contacts.js
printf '%s\n' '--- route selection guard ---'
sed -n '540,635p' src/views/Contacts.vue
printf '%s\n' '--- current versus merge-base save flow ---'
git diff --unified=35 c171dbecb062d38d402da688466180dc8094d75e dee595a9dce995453044d7c760a353c6d2a36f77 -- src/components/ContactDetails.vue src/store/contacts.js

Repository: nextcloud/contacts

Length of output: 9710

Script executed:

set -e
printf '%s\n' '--- addressbook move action ---'
sed -n '535,585p' src/store/addressbooks.js
printf '%s\n' '--- contacts update action ---'
sed -n '400,515p' src/store/contacts.js
printf '%s\n' '--- ContactDetails update and contact binding ---'
sed -n '860,925p' src/components/ContactDetails.vue
rg -n -F --glob 'src/components/ContactDetails.vue' -e 'computed:' -e 'contact()' -e 'mapGetters' -e 'selectedAddressbook' -e 'localContact'
printf '%s\n' '--- ContactDetails component setup/computed region ---'
sed -n '300,470p' src/components/ContactDetails.vue
printf '%s\n' '--- contacts list source ---'
rg -n -F --glob 'src/views/Contacts.vue' -e 'contactsList' -e 'selectedContact' -e 'selectedAddressbook'
sed -n '120,230p' src/views/Contacts.vue

Repository: nextcloud/contacts

Length of output: 21773

Script executed:

set -e
printf '%s\n' '--- ContactDetails computed contact and selection ---'
sed -n '540,835p' src/components/ContactDetails.vue
sed -n '945,1015p' src/components/ContactDetails.vue
sed -n '1100,1140p' src/components/ContactDetails.vue
printf '%s\n' '--- contact/addressbook mutations ---'
sed -n '110,270p' src/store/contacts.js
sed -n '1,130p' src/store/addressbooks.js
printf '%s\n' '--- Contacts list and selected route bindings ---'
sed -n '315,420p' src/views/Contacts.vue

Repository: nextcloud/contacts

Length of output: 22152

Script executed:

set -e
printf '%s\n' '--- new-contact creation route ---'
sed -n '450,555p' src/views/Contacts.vue
rg -n -F --glob 'src/**/*.js' -e 'new Contact' -e 'dispatch('\''addContact' -e 'dispatch("addContact"' -e 'addContact(' -e 'getContact' src/views src/components src/store
printf '%s\n' '--- contact getter and key/addressbook model ---'
rg -n -F --glob 'src/models/contact.js' -e 'get key' -e 'set addressbook' -e 'get addressbook' -e 'key()' -e 'addressbook =' -e 'constructor'
sed -n '1,220p' src/models/contact.js
printf '%s\n' '--- ContactDetails props and route contact key ---'
sed -n '470,545p' src/components/ContactDetails.vue
rg -n -F --glob 'src/store/contacts.js' -e 'getContact' -e 'sortedContacts'

Repository: nextcloud/contacts

Length of output: 12709

Navigate to the destination address book after creating the new contact.

The new contact is already in the store. Contacts.vue dispatches addContact before routing to the edit view, so this.contact resolves to the stored contact in A.

The pre-save move changes that stored contact’s address book to B. Because Contact.key includes the address-book ID, this changes the key from A’s key to B’s key. moveContactToAddressbook then pushes the B key while preserving the current selectedAddressbook route parameter, which is still A.

A’s contactsList excludes the B contact. The route watchers can therefore call selectFirstContactIfNone(), show “Contact not found”, and redirect while createVCard is still pending. The move action does not change selectedAddressbook; it only mutates the store and starts navigation.

Keep the store move required for Activity separate from navigation. Suppress navigation for the pre-save move, then push the B route with the new key after updateContact() completes. Retain the existing post-save behavior for existing contacts.

this.newAddressBook = null
}
}

await this.updateContact()
if (this.newAddressBook && this.newAddressBook !== this.contact.addressbook.id) {
this.updateAddressBookAccesses(this.newAddressBook)
this.moveContactToAddressbook(this.newAddressBook)
await this.moveContactToAddressbook(this.newAddressBook)
this.newAddressBook = null
}
this.editMode = false
Expand Down
Loading