Loosen the validation on setting country in payment propertyBag to handle buggy code

[mattwire]

Overview

See https://lab.civicrm.org/dev/core/-/issues/3918

Before

Crash if bad country data passed in.

After

Logged in bad country data passed in.

Technical Details

Comments

Alternative to #24894

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@mattwire is this a good idea? It affects setBillingCountry which was OK - whereas we just want setCountry to be more lenient

[eileenmcnaughton]

The good thing is the unit test fail flags where to add a new unit test :-)

[eileenmcnaughton]

I guess the flip side to my thoughts about it making setBillingCountry more lenient is that in general when I tried to use PropertyBag I found it was probably a bit too strict to be easy to use (mostly cos of the handling of empty/null) - so maybe logging is enough

[artfulrobot]

(SCRAP THIS)

I think logging this stuff is useful to search out places where the data is being incorrectly supplied, however this passes the problem on down the line - it means we accept whatever junk passed in as billingCountry, so we will output the same junk too, which likely leads to multiple implementations of fixes for this problem by the consumers of the propertybag.

Would it be too much to, as Eileen suggests do

if is_string and isISO3166ok: accept

elseif !empty && (is_int || preg_match('/^\d+$/')) && idMatchesCountryId: lookup and map to the ISO3166 code. Log warning

elseif !empty && is_string && matches country name: lookup and map to the ISO3166 code. Log warning.

else store NULL (as what we have cannot be valid) and log warning

EDIT: I've put my preferred aproach at #24927

[mattwire]

Closing in favour of #24927