Reading settings with ?: is idiomatic and, for anything numeric, wrong:
$perPage = Settings::i()->myapp_per_page ?: 25;
PHP treats 0, '0' and '' as falsy. If an administrator sets that field to 0 — a perfectly reasonable way to say "none" or "unlimited" — they get 25 instead. The setting appears to do nothing, and no amount of re-saving it helps.
Where this actually bites
- Anything meaning "unlimited", where 0 is the natural way to express it.
- Anything meaning "off", where 0 disables a feature.
- A delay or threshold where 0 means "immediately" or "always".
It is invisible in testing because nobody tests with zero — they test with a sensible number.
The fix
public static function number( mixed $value, int $default ): int
{
if ( $value === NULL or $value === '' or !is_numeric( $value ) )
{
return $default;
}
return (int) $value;
}
Then clamp separately, so the intent stays readable:
$perPage = max( 1, min( 500, static::number( Settings::i()->myapp_per_page, 25 ) ) );
The same trap with ?? and empty arrays
?? has the mirror-image problem: it only catches a key that is missing, not one that is empty. A widget's configuration is an empty array both before it has ever been configured and after being saved with nothing selected, so:
$show = $this->configuration['show'] ?? $defaults; // WRONG: empty array wins
gives you an empty selection and a permanently blank block. Fall through to defaults when the value is empty, not merely absent:
$show = (array) ( $this->configuration['show'] ?? array() );
if ( !$show )
{
$show = $defaults;
}
Verified against
Invision Community 5.0.19. The behaviour is plain PHP truthiness rather than anything Invision-specific, but the widget-configuration case is specific to how blocks store their settings.
Recommended Comments