APIv4 - Fix CONTAINS operator to work with more types of serialized fields #26362
Conversation
|
(Standard links)
|
| switch ($field['serialize'] ?? NULL) { | ||
|
|
||
| case \CRM_Core_DAO::SERIALIZE_JSON: | ||
| $operator = 'LIKE'; | ||
| $value = '%"' . $value . '"%'; |
There was a problem hiding this comment.
MIN_INSTALL_MYSQL_VER is now 5.7, so maybe it makes sense to fix this while we're here? Would be good to improve the JSON search, since the current version has some obvious shortcomings.
There was a problem hiding this comment.
Thanks for the suggestion! I looked at that as well, and tried a few ways of searching JSON but couldn't get it working just right so I thought I'd go ahead and put up this PR without that so at least this can get merged.
| @@ -592,8 +592,13 @@ protected function createSQLClause($fieldAlias, $operator, $value, $field, int $ | |||
| } | |||
| return $sql ? implode(' AND ', $sql) : NULL; | |||
| } | |||
|
|
|||
| // The CONTAINS operator matches a substring for strings. For arrays & serialized fields, | |||
| // it only matches a complete (not partial) string within the array. | |||
There was a problem hiding this comment.
Maybe note that we don't match complete strings for PHP serialized?
There was a problem hiding this comment.
I'll be there's a way to support that type of serialization too! Probably something like:
case \CRM_Core_DAO::SERIALIZE_PHP:
$operator = 'LIKE';
$value = '%' . serialize($value) . '%';
break;
But I haven't tested that. Best to merge this PR first.
There was a problem hiding this comment.
Sure, I assumed it was a don't bother to support it because there aren't many of them situation.
|
Thanks for reviewing @larssandergreen - are you satisfied with this PR getting merged as-is before iterating further on it? |
|
Code looks good, I will give it an r-run in a bit. |
|
No issues on r-run, good to merge from my perspective. |
Overview
Fixes a bug in APIv4 where the CONTAINS operator would return false-positives for certain types of serialized fields.
Before
Given a group with parents [12, 35, 40] the CONTAINS match would return TRUE for
12(correctly) but also return TRUE for1(incorrectly matching one of the digits instead of the complete number)After
Fixed.