Fix inconsistencies in duplicate retrieval

[eileenmcnaughton]

Overview

Alternative to
Fix dedupe bug whereby the result of passing in criteria with no matches is to have it ignored
#15158
and

Performance improvement on dedupe find

#15153

as on further digging I realised that making the code more sensible would address both

Before

$pairs = $this->callAPISuccess('Dedupe', 'getduplicates', [
'rule_group_id' => 1,
'criteria' => ['contact' => ['id' => ['>' => 1000000000]]],
])
returns results if no contacts in the DB match the criteria

In addition the above query runs without a limit even when one is specified.

After

Above only returns results if contacts in the DB match the criteria.

The search_limit is respected when retrieving the contacts to match

Technical Details

Dedupe.getduplicates and duplicates retrieval in general respect 2 types of limit

 $pairs = $this->callAPISuccess('Dedupe', 'getduplicates', [
      'rule_group_id' => 1,
      'options' => ['limit' => 1],
     'criteria' => ['contact' => ['first_name' => 'Bob]],
    ])

Will fill the prevnext_cache with all pairs of contacts where one of the contacts match the criteria - ie first if finds all Bobs, then it fills the catch with all matches found for any of those Bobs given the rule. The api call returns ONLY the first pair due to limit => 1

 $pairs = $this->callAPISuccess('Dedupe', 'getduplicates', [
      'rule_group_id' => 1,
      'search_limit'  => 1,
     'criteria' => ['contact' => ['first_name' => 'Bob]],
    ])

Will find the first bob (search_limit is 1) and fill the prevnext_cache with all matches for that Bob and return as many pairs as it found

Comments

@pfigel another

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

test this please

[pfigel]

This one looks like the cleanest fix, 👍

(CiviCRM Review Template WORD-1.2)

• General standards
  • (r-explain) PASS
  • (r-user) PASS: No UI-visible impact in core
  • (r-doc) PASS
  • (r-run) PASS: Tested various combinations with/without limit/criteria. Confirmed core UI is still functional (search, merge, filtering results)
• Developer standards
  • (r-tech) PASS:
    • Signature of CRM_Dedupe_Finder::dupes changed; all calls in core are compatible, universe shows no incompatible calls either
    • Change where non-matching criteria causes criteria to be ignored technically changes the contract but is a bugfix that restores sanity and fixes performance
  • (r-code) PASS
  • (r-maint) PASS: Adds some tests
  • (r-test) PASS

[eileenmcnaughton]

Thanks @pfigel I'm sure this caused you some brain pain too :-) But the code is inching towards sanity