From 7174e3be6c7eff0199f08b2bdb710b4df81dc227 Mon Sep 17 00:00:00 2001 From: Jeppe B <2jepp9350@gmail.com> Date: Mon, 10 Aug 2026 09:21:56 +0200 Subject: [PATCH] fix(api): coerce string booleans before persisting module_config bool values (#359) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../MiniMaxEnabledSetVariableValueTest.php | 103 ++++++++++++++++++ .../app/traits/module_config_variable_t.php | 9 +- 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 services/nginx/app/tests/Unit/MiniMax/MiniMaxEnabledSetVariableValueTest.php diff --git a/services/nginx/app/tests/Unit/MiniMax/MiniMaxEnabledSetVariableValueTest.php b/services/nginx/app/tests/Unit/MiniMax/MiniMaxEnabledSetVariableValueTest.php new file mode 100644 index 00000000..5f88c07e --- /dev/null +++ b/services/nginx/app/tests/Unit/MiniMax/MiniMaxEnabledSetVariableValueTest.php @@ -0,0 +1,103 @@ + */ + public array $updates = []; + + /** @var array */ + 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); +}); diff --git a/services/nginx/app/traits/module_config_variable_t.php b/services/nginx/app/traits/module_config_variable_t.php index bfed8146..93999b44 100644 --- a/services/nginx/app/traits/module_config_variable_t.php +++ b/services/nginx/app/traits/module_config_variable_t.php @@ -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