Incorporate searchLimit in dedupe cacheKey

[eileenmcnaughton]

Overview

Fixes a bug whereby changing the search limit after a search has been performed does not result in a new search

Before

Altering the limit in the url after the initial retrieval has no effect

This can also be accessed via the api - without this patch running these 2 in sequence will result in the second call 'respecting' the limit from the first

    $pairs = $this->callAPISuccess('Dedupe', 'getduplicates', [
      'rule_group_id' => 1,
      'search_limit' => 500,
    ]);
    $pairs = $this->callAPISuccess('Dedupe', 'getduplicates', [
      'rule_group_id' => 1,
      'search_limit' => 5,
    ]);

After

Search limit is included in the cacheKey so if it changes the cache is refreshed.

Technical Details

When accessing dedupes by the api call or on the dedupe screen it's possible to pass in
a searchLimit param. This works like the group limit in that it limits the number of
contacts for whom a match is sought.

For example if there are 2million contacts in the database
and you have a search limit of 0 then it will look for duplicates for all 2 million. (unset
is the same as 0). If you have a search limit of 1000 it will look for matches for the first
1000 contacts that match the criteria (criteria could be the group or other criteria passed in
via the url although the api is the most obvious way to pass in criteria)

Note there is a separate limit (sometimes called batch limit) that limits results from within
the found matches. If you pass in ['options' => ['limit' => 10]] via the api this will return 10 results if 10 or more exist. However, this limit has no effect on building the cache.

To test the searchLimit it is possible to add limit=5 to the url generated in the
url by findContacts. Without this patch changing the limit once the search has been done
will not alter the results as the limit is not part of the cachekey - this patch
changes that.

Comments

@pfigel

[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).

[pfigel]

One question (see comment), otherwise this LGTM! 👍

(CiviCRM Review Template WORD-1.2)

• General standards
  • (r-explain) PASS
  • (r-user) PASS: Limit being respected and results being refreshed makes sense to users
  • (r-doc) PASS
  • (r-run) PASS: Tested via API & UI
• Developer standards
  • (r-tech) PASS: Signature change of internal code, shows no relevant matches in universe.
  • (r-code) Undecided: See comment
  • (r-maint) PASS: Some existing test coverage
  • (r-test) PASS

[pfigel]

Do these fallbacks make sense? Feels like it should be TRUE or FALSE for check_permissions (not sure which, [] seems to have worked so far but that looks weird) and 0 for search_limit.

[eileenmcnaughton]

Thanks @pfigel I changed search_limit to default to 0 and check_permissions to use !empty($params['check_permissions']) - which is consistent with elsewhere. Adding merge-on-pass