Bugfix/main/minor fixes #9033
Bugfix/main/minor fixes #9033
Conversation
jonasraoni
force-pushed
the
bugfix/main/minor-fixes
branch
from
60de5b2 to
58f7582
Compare
classes/controllers/grid/feature/selectableItems/SelectableItemsFeature.php
Outdated
Show resolved
Hide resolved
|
@jonasraoni, I've started going through this but I'm still finding it hard to review as it mixes small-but-important changes in with uncontroversial changes (e.g. spelling corrections). I'd suggest two next steps:
|
|
@asmecher I still didn't review these PRs, I've just shared to let you take a first-look 😁 |
|
After the latest type hints, I think the code must be almost ready to run with the |
|
I'd like to set aside a bit of time after 3.4.0 is released to work through some of the strict mode warnings/errors, but I'm pretty sure strict mode is going to be broken until after the next LTS release (looking like 3.5.0 at this point), when we can formally remove some of the backwards compatibility stuff that was introduced with this cycle. |
jonasraoni
force-pushed
the
bugfix/main/minor-fixes
branch
3 times, most recently
from
65a99d2 to
7f21cf1
Compare
| @@ -330,7 +330,7 @@ public function convertFromDB($value, $type, $nullable = false) | |||
| return (float) $value; | |||
| case 'object': | |||
| case 'array': | |||
| $decodedValue = json_decode($value, true); | |||
| $decodedValue = json_decode($value ?? ($type === 'array' ? '[]' : '{}'), true); | |||
There was a problem hiding this comment.
AFAICR I got this from a warning in a user log.
A not null field is attempting to convert a null value. I thought it's better to return an empty array/object than false.
|
@asmecher Done! I've created some new issues. From this PR the only item that might be worth to review is the one below, the rest + things that I didn't push, I'll leave for post-3.4: As a reminder, the other issues that I've met before must be unsolved yet, see: #8845 (as I wrote in the description, the PRs on the issue are not covering anything that I wrote in the issue description). |
jonasraoni
force-pushed
the
bugfix/main/minor-fixes
branch
2 times, most recently
from
9d20241 to
d788e76
Compare
| // FIXME: pkp/pkp-lib#6250 Remove after 3.3.x upgrade code is removed (see also pkp/pkp-lib#5772) | ||
| if (!is_null($decodedValue)) { | ||
| return $decodedValue; | ||
| return $type === 'array' ? array_values((array) $decodedValue) : (object) $decodedValue; |
There was a problem hiding this comment.
For here and below, the database might be holding a valid JSON, with an unexpected value.
For example, if we encode the array [1 => 0] using the $type array, it will be stored as a JSON object.
There are already some known cases out in the wild.
jonasraoni
force-pushed
the
bugfix/main/minor-fixes
branch
from
d788e76 to
2c01bea
Compare
| @@ -80,6 +80,8 @@ class DAO extends EntityDAO | |||
|
|
|||
| /** | |||
| * Construct a new User object. | |||
| * | |||
| * @return User | |||
There was a problem hiding this comment.
Since I think this PR is most suitable for main, I'd rather we just started adding first-class type hints.
No description provided.