-
Notifications
You must be signed in to change notification settings - Fork 173
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.
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
Auto merge #2764
base: main
Are you sure you want to change the base?
Auto merge #2764
Conversation
Add checkbox on hover and add a button to delete the selected contacts Signed-off-by: Bastien PROMPSY <bastienprompsy@gmail.com>
Add a button to merge the selected contacts Signed-off-by: Bastien PROMPSY <bastienprompsy@gmail.com>
Add a button auto merge which will merge the possible duplicated contacts verified by the fullname, phone number and email adress Signed-off-by: Bastien PROMPSY <bastienprompsy@gmail.com>
Nice work @MrPompom! :) To prevent mistakes, what do you think of doing it in a way similar to other Contacts apps which have a "Merge duplicates" entry if there are duplicates? Then people can go through the suspected duplicates and double check, instead of possibly merging some correct and some wrong. |
I'm not able to read this from your code, so is this condition an or or an and? In larger organizations there are often people with the same full name. E.g. we had customer issues about this when the share menu wasn't distinguishable and now it also shows the email address. So it's important that the match is based on more than just the fullname. |
First of all, thanks for your contribution. I like the idea but I'm concerned with the UX. Automatically merging contacts involves too much magic. IMO, we should implement some form of dialog/preview similar to what Jan said. |
Hi, I might show something, what could help to identify duplicates ... This software contains a functionality to determine duplicates and "delete" them (move them to the recycle bin). |
} else if (value[3] !== '') { | ||
let include = false | ||
firstContact.jCal[1].forEach(element => { | ||
if (element[0] === value[0] && element[3] === value[3]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where do the magic indeces 0 and 3 come from?
getKey(contact, index) { | ||
if (!this.selected.includes(contact[index].key)) { | ||
this.selected.push(contact[index].key) | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this method is very confusing. why doesn't it return a value? please rename or restructure to make it an actual getter
@@ -163,4 +331,8 @@ export default { | |||
.contacts-list__header { | |||
min-height: 48px; | |||
} | |||
|
|||
.merge-button { | |||
float: right; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use flexbox instead of floats
this.$store.dispatch('deleteContact', { contact: this.contacts[element] }) | ||
} | ||
}) | ||
this.$store.dispatch('updateContact', firstContact) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateContact
doesn't send the change to the server
@MrPompom could you pleaes have a look at the feedback above? |
Linked to the pull request #2756 (comment)
Allow to automatically merge contacts who have the same Fullname, Phone number or Email adress
How it works
When the auto merge button is clicked, all the contacts which have the same Fullname, phone number or email adress will be merged together