Fix/896 criteria strict comparison #897
Conversation
| @@ -77,7 +80,10 @@ protected function assert($value, $operator, $expected) | |||
| $result = false; | |||
| switch ($operator) { | |||
| case self::OPERAND_EQUAL: | |||
| $result = $value === $expected; | |||
| $result = $value == $expected; | |||
There was a problem hiding this comment.
Can it break the existing widgets when Victoire will be updated? The comparison will be less strict so it may yield different results.
Another solution to keep backward compatibility would be to keep the existing comparison, and add a OPERAND_NOT_STRICTLY_EQUAL, but this is ugly. So your solution is the best we can do.
There was a problem hiding this comment.
It would be safer to convert $expected to true or false when equals to 1 or 0, then do a === comparison.
== comparison seems not sufficient strict because of true == 'false' returning true
There was a problem hiding this comment.
The $value variable isn't always a boolean, we cannot cast it in bool.
I think our problem here is deeper and we need to dig more for a good architecture.
What do we do with this PR ? There isn't any BC break but we don't need this code neither as there is a workaround by using the OPERAND_TRUE instead the OPERAND_EQUAL for now so... we can take our time if you prefer @paulandrieux
|
closing for now as the whole criteria system needs to be refactored and the issue isn't blocking |
Type
Bugfix
Purpose
Fixes #896
And add the
differentoperand which was missing (the opposite of equal).BC Break
NO