dev/core#745 Further on_hold fix - it has been suggested the value might be an array like ['']

[eileenmcnaughton]

Overview

This extends the fix to prevent us winding up with a query like on_hold IN ('') or on_hold IN (NULL)

Before

Potential inappropriate filtering on legacy smart groups

After

on_hold filter only applied for numeric values - either as a value or in an array

Technical Details

We seem to have some legacy group variants hitting problems - the data about what they might be is a bit unclear but this would see to ensure that only a non-empty array of numeric values gets to the if clause

Comments

@agileware-fj @seamuslee001 - Seamus felt we were missing some possible variants in #13754 so this should be tighter & still pretty sage

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

[agileware-fj]

@eileenmcnaughton @seamuslee001 In practice, I've not seen any sites where this was an empty string in an array.
That said, I think filtering out a cast array with is_numeric is more elegant.

FYI the on-the-wire fix I did the first time we came across this problem looked like:
$onHoldValue = array_map(function($v){ return (((int) $v) ?: 0); }, (array) $onHoldValue);

[seamuslee001]

Merging as per the positive review by Francis and the fact tests are passing