Allow API4 match to match an empty value

[mattwire]

Overview

The new matching functionality on X::save is nice but I found an issue that I find counterintuitive / confusing.

For example if you have some data to import and some has an external_identifier then you might match on first_name,last_name,external_identifier but if external_identifier is empty it won't match because it does external_identifier = '' and that's not the same as external_identifier IS NULL

ie. https://github.com/civicrm/civicrm-core/blob/master/Civi/Api4/Generic/AbstractSaveAction.php#L145

@colemanw said: Maybe patch that line to use the IS EMPTY operator if $val === ''?

Pending test to be added here:

civicrm-core/tests/phpunit/api/v4/Action/SaveTest.php

Line 45 in db11224

->setMatch(['first_name', 'external_identifier'])

Before

Matching on an empty string doesn't match.

Eg. match on: first_name='bob'; external_identifier=''; creates new record even if that exists.

After

Matching on an empty string matches.

Eg. match on: first_name='bob'; external_identifier=''; updates existing record.

Technical Details

This is particularly noticeable when doing imports.

Comments

@colemanw I've added a test now.

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

[colemanw]

Thanks @mattwire - Jenkins says you have to add spaces around the "<" on line 56 of the test.

[colemanw]

@mattwire your new test didn't pass

[mattwire]

@colemanw This seems to be a setup/teardown issue. When the test is run by itself it passes every time. But when run as a suite (eg. api/v4/Action) it fails suggesting that something external is interfering with the test?

[colemanw]

@mattwire I think the match criteria isn't unique enough, and running the test when there's other data in the database, you get more than one match for a contact with first name "Abc" and a blank external identifier.

[colemanw]

That works. Typically in tests requiring unique names I use something like $lastName = uniqid(__FUNCTION__);. That's pretty bulletproof even when running on a database with lots of existing data.

[highfalutin]

Thank you for this work, @mattwire and @colemanw! I found myself running into this exact issue just a couple days ago.