Fix some PHP8.1 deprecation warnings occurred while running tests #18314
Conversation
sgiehl
added
Needs Review
|
|
||
| // Update minimum only if it is set | ||
| if ($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] !== false) { | ||
| if ($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] !== false && $newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] !== null) { |
There was a problem hiding this comment.
Seems this one actually fixes some kind of bug, that was never recognized.
Before when an event value was null, which imho means it is not set, it was indirectly casted to int (caused by the round) and thus always resulted in 0. So the minimum event value was incorrectly reported as 0.
This can be seen in the expected test results I have updated. In the Fixture we are actually not setting 0 as event value, so imho this should be a valid change
|
I'll put this into review. There might still be some PHP 8.1 issues around. But might be good to already merge this before releasing an RC. Will continue searching for additional PHP 8.1 issues the coming days and create a new PR if I find any. |
|
|
||
| // Update minimum only if it is set | ||
| if ($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] !== false) { | ||
| if ($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] !== false && $newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] !== null) { |
| @@ -71,7 +71,7 @@ public function getPrettyTimeFromSeconds($numberOfSeconds, $displayTimeAsSentenc | |||
| } else { | |||
| $time = sprintf(Piwik::translate('Intl_NDays'), $days) . " " . sprintf("%02s", $hours) . ':' . sprintf("%02s", $minutes) . ':' . sprintf("%02s", $seconds); | |||
| } | |||
| $centiSeconds = ($numberOfSeconds * 100) % 100; | |||
| $centiSeconds = intval($numberOfSeconds * 100) % 100; | |||
There was a problem hiding this comment.
note to myself, this avoids Deprecated: Implicit conversion from float xxxxx to int loses precision. There was no rounding happening before either https://3v4l.org/OK980 so all good
| @@ -298,7 +298,7 @@ public static function safe_unserialize($string, $allowedClasses = [], $rethrow | |||
| { | |||
| try { | |||
| // phpcs:ignore Generic.PHP.ForbiddenFunctions | |||
| return unserialize($string, ['allowed_classes' => empty($allowedClasses) ? false : $allowedClasses]); | |||
| return unserialize($string ?? '', ['allowed_classes' => empty($allowedClasses) ? false : $allowedClasses]); | |||
There was a problem hiding this comment.
note to myself: it'll return always false which it also did before https://3v4l.org/qC9u3
|
@tsteur thanks for the review. I've applied some changes. |
There was a problem hiding this comment.
@sgiehl 👍 checked with https://3v4l.org/dAKrC and works
Be good to merge once tests pass (unless they fail for another reason)
Description:
Tried running the tests locally and a couple of deprecation warnings showed up. Not sure if they would occur normally. But should be good to fix them nevertheless.
refs #17686
Review