Pass all available relevant fields to Contact.getduplicates on checkMatchingContact #29554
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
b2f080d to
c94ef67
Compare
|
I checked PR site test and I haven't been able to get it to work. I create a supervised rule for individuals with firstname, lastname and a custom field and also a supervised rule for orgs with city and country. Neither of them worked for me. |
|
@francescbassas did you definitely check specifically the Check Matching Contacts button
That is the lookup affected by this change. I just tried adding 'Marital Status' and that did work |
|
test this please |
|
@eileenmcnaughton I defined this Supervised rule (marital status + country) and I think it's not working 🤔
|
|
@francescbassas I suspect country might be not defined the same on the form - if you drop it & just do the custom field does it work? |
|
Using the button "Check for matching contacts".. with just the custom field works, |
…atchingContact I took a look at civicrm#23353 in the hopes of resolving it but it's really tricky code & there were a couple of trade offs in there I wasn't comfortable with 1) losing the faster ajax call 2) letting more worms out than absolutely necessary OTOH that patch is a bit cleverer I think in terms of not double presenting contacts.
c94ef67 to
57f889c
Compare
|
@francescbassas I think the issue with the custom field is deeper in the dedupe code & possibly affects more places - I did a big cleanup on the code & the custom fields are working on the cleaned up version - which I pushed in here. I didn't dig into country but there could be an issue still with that but I propose to leave that out of scope for this PR |
|
Custom field for organization is now working. Only rules that include address fields don't seem to work but I think it's reasonable to remove it from this PR. I took them into account somewhat randomly for testing, not thinking about actual use. Fixing custom fields will already be a big improvement to prevent entry of duplicate contacts (for custom fields used by external identifiers, identity documents, etc.). |
|
@colemanw that close was premature - I've just rebased out the part you merged |
|
@eileenmcnaughton funny I didn't actually close this. I probably just deleted the branch after merging another PR. |
|
I'm gonna merge this & close #23353 based on @francescbassas review Note there is a help text fix in that one that maybe we should bring back to life https://github.com/civicrm/civicrm-core/pull/23353/files#diff-e66fa887a0a943f8db353ae3dfb3923aa94b1e49ba940289eafd674168032082L102 |
Overview
Pass all available relevant fields to Contact.getduplicates on checkMatchingContact - replaces #23353 by @darrick with a narrower fix - also ping @francescbassas
Before
Create a new (default) Supervised Individual dedupe rule. Add First name, last name, phone, adding up to 15 with each having 5
Add a new individual
Add the same individual again - ignore the as-you-type popup - click on 'Check Matching Contact'
After
After hitting Check Matching Contact
Technical Details
I took a look at #23353 in the hopes of resolving it but it's really tricky code & there were a couple of trade offs in there I wasn't comfortable with
OTOH that patch is a bit cleverer I think in terms of not double presenting contacts.
This patch is ultimately very narrow - although there is a long comment block... It still passes the params that were being passed to
getduplicatesbut it also passes additional parameters that it identifies based on the ruleComments