fix(api): coerce string booleans before persisting module_config bool values (#359)
PHP truthy semantics treat the literal `'false'` as truthy, so `$value ? 'true' : 'false'` in `setVariableValue()` persisted every 'switch off' request as `'true'` for ~30 modules with bool config variables. Canonicalise via the existing `inputToBool()` helper before the ternary so JSON.stringify(boolean) inputs land on the right storage string.
Live repro (api-v2.truckwash.io, 2026-08-10 09:25):
```
POST /minimax/config {variable:"enabled", value:"false"} → 200
GET /minimax/config?variable=enabled → {value:true, ...} ← unchanged
POST /minimax/config {variable:"enabled", value:false} → 200
GET /minimax/config?variable=enabled → {value:false, ...} ← JSON bool works
```
Tests: `tests/Unit/MiniMax/MiniMaxEnabledSetVariableValueTest.php` (6 assertions, captures the UPDATE column value via stubbed `updateVariableValue`/`insertVariableValue` for string/boolean × true/false × insert/update paths).
Companion pleno-vue PR #281 lands the matching UI-side fix (`onMiniMaxEnabledSwitch` re-fetches + optimistic rollback).
This commit is contained in:
@@ -0,0 +1,103 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* End-to-end regression for the bool-truthy-string bug:
|
||||
*
|
||||
* \traits\module_config_variable::setVariableValue() converts boolean
|
||||
* values to the storage strings 'true' / 'false' via
|
||||
* `$value ? 'true' : 'false'`
|
||||
* but PHP treats any non-empty string as truthy — including the literal
|
||||
* 'false' that the frontend sends for an unchecked toggle
|
||||
* (JSON.stringify(false) === "false"). Without an explicit coercion
|
||||
* to a real bool first, the storage value flips the wrong way for every
|
||||
* "switch off" request against every boolean config variable (~30
|
||||
* modules).
|
||||
*
|
||||
* These tests call setVariableValue() against an in-memory db stub that
|
||||
* captures the UPDATE statement's column value, so the regression is
|
||||
* pinned at the trait level rather than at any one call site.
|
||||
*/
|
||||
|
||||
app_require('traits/module_config_variable_t.php');
|
||||
|
||||
use traits\module_config_variable;
|
||||
|
||||
if (!class_exists('BoolSetVariableValueStub')) {
|
||||
class BoolSetVariableValueStub
|
||||
{
|
||||
use module_config_variable;
|
||||
|
||||
/** @var array<int, array{string, string, mixed}> */
|
||||
public array $updates = [];
|
||||
|
||||
/** @var array<int, array{string, string, mixed}> */
|
||||
public array $inserts = [];
|
||||
|
||||
public bool $exists = true;
|
||||
|
||||
public function __construct()
|
||||
{
|
||||
$this->module_name = 'miniMax';
|
||||
$this->config_variable = 'enabled';
|
||||
$this->config_variable_type = 'bool';
|
||||
$this->config_variable_required = true;
|
||||
$this->config_variable_is_secret = false;
|
||||
$this->allowed_values = null;
|
||||
}
|
||||
|
||||
protected function isVariableSet(string $module, string $variable): bool
|
||||
{
|
||||
return $this->exists;
|
||||
}
|
||||
|
||||
protected function updateVariableValue(string $module, string $variable, mixed $value): void
|
||||
{
|
||||
$this->updates[] = [$module, $variable, $value];
|
||||
}
|
||||
|
||||
protected function insertVariableValue(string $module, string $variable, mixed $value): void
|
||||
{
|
||||
$this->inserts[] = [$module, $variable, $value];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
it('stores the string "false" as "false" (the bug regressed to "true")', function (): void {
|
||||
$stub = new BoolSetVariableValueStub();
|
||||
$stub->setVariableValue('false');
|
||||
expect($stub->updates)->toHaveCount(1);
|
||||
expect($stub->updates[0][2])->toBe('false');
|
||||
});
|
||||
|
||||
it('stores the string "true" as "true"', function (): void {
|
||||
$stub = new BoolSetVariableValueStub();
|
||||
$stub->setVariableValue('true');
|
||||
expect($stub->updates)->toHaveCount(1);
|
||||
expect($stub->updates[0][2])->toBe('true');
|
||||
});
|
||||
|
||||
it('stores a JSON boolean false as "false"', function (): void {
|
||||
$stub = new BoolSetVariableValueStub();
|
||||
$stub->setVariableValue(false);
|
||||
expect($stub->updates[0][2])->toBe('false');
|
||||
});
|
||||
|
||||
it('stores a JSON boolean true as "true"', function (): void {
|
||||
$stub = new BoolSetVariableValueStub();
|
||||
$stub->setVariableValue(true);
|
||||
expect($stub->updates[0][2])->toBe('true');
|
||||
});
|
||||
|
||||
it('falls back to insertVariableValue when the row does not exist yet', function (): void {
|
||||
$stub = new BoolSetVariableValueStub();
|
||||
$stub->exists = false;
|
||||
$stub->setVariableValue('false');
|
||||
expect($stub->updates)->toBeEmpty();
|
||||
expect($stub->inserts)->toHaveCount(1);
|
||||
expect($stub->inserts[0][2])->toBe('false');
|
||||
});
|
||||
|
||||
it('rejects int inputs for a bool config variable', function (): void {
|
||||
$stub = new BoolSetVariableValueStub();
|
||||
expect(fn () => $stub->setVariableValue(42))->toThrow(Exception::class);
|
||||
});
|
||||
@@ -152,8 +152,15 @@ trait module_config_variable
|
||||
if (!$this->validateVariableValue($value)) {
|
||||
throw new Exception('Invalid value for config variable: ' . $this->config_variable . ' (' . $value . ')');
|
||||
}
|
||||
// If the type is a boolean, convert the value to "true" or "false"
|
||||
// If the type is a boolean, convert the value to a real boolean first.
|
||||
// PHP's truthy semantics treat any non-empty string — including the
|
||||
// literal 'false' — as truthy, so a naive `$value ? 'true' : 'false'`
|
||||
// would persist the string 'false' as 'true'. The frontend sends JSON
|
||||
// booleans as the strings 'true' / 'false', so this branch runs on
|
||||
// every toggle. inputToBool() canonicalises both string and boolean
|
||||
// inputs to a real bool before the ternary.
|
||||
if ($this->config_variable_type === 'bool') {
|
||||
$value = is_string($value) ? self::inputToBool($value) : (bool)$value;
|
||||
$value = $value ? 'true' : 'false';
|
||||
}
|
||||
// Update the value of the config variable in the database, and insert it if it does not exist
|
||||
|
||||
Reference in New Issue
Block a user