diff --git a/src/Domain/Common/Adapters/ItemTrait.php b/src/Domain/Common/Adapters/ItemTrait.php index dc54ff479..eb6ad7397 100644 --- a/src/Domain/Common/Adapters/ItemTrait.php +++ b/src/Domain/Common/Adapters/ItemTrait.php @@ -65,7 +65,14 @@ protected function getCustomFieldsForItem( $customField = new CustomFieldItem( required: (bool)$item['required'], showInList: (bool)$item['showInList'], - help: $item['help'], + // `help` is the one nullable column here that was not being made safe: the schema + // has it `varchar(255) DEFAULT NULL` and CustomFieldDefinition::getHelp() agrees + // it is `?string`, while CustomFieldItem declares `string $help`. `required` and + // `showInList` are nullable too and are cast on the lines either side; this one + // was passed through, so a definition saved with the Help box left blank — an + // ordinary thing for an admin to do — made every read of that item's custom + // fields a TypeError. + help: $item['help'] ?? '', definitionId: (int)$item['definitionId'], definitionName: $item['definitionName'], typeId: (int)$item['typeId'], diff --git a/tests/Integration/Infrastructure/Adapter/In/Api/Controllers/CategoryControllerTest.php b/tests/Integration/Infrastructure/Adapter/In/Api/Controllers/CategoryControllerTest.php index fa635aa9a..a07d1f860 100644 --- a/tests/Integration/Infrastructure/Adapter/In/Api/Controllers/CategoryControllerTest.php +++ b/tests/Integration/Infrastructure/Adapter/In/Api/Controllers/CategoryControllerTest.php @@ -125,6 +125,31 @@ public function testViewActionNonExistant(): void $this->assertSame('Category not found', $r->body->error->message); } + /** + * `customFields=1` on a category, which is where the nullable-help defect surfaced. + * + * The fixture carries two CATEGORY-module definitions ("RSA", "SSL") whose help text is null, + * and `ItemTrait` passed that straight into `CustomFieldItem`'s non-nullable `string $help`. + * Every request for a category's custom fields was therefore a TypeError escaping as a 500 + * with the class, the method and the server's absolute path in the body — reachable by any + * administrator who left a custom field's Help box empty, which the form allows. + * + * So this asserts a 200 with the include actually populated, and the null help coming back as + * an empty string rather than being the reason the request failed. + */ + public function testViewActionWithCustomFields(): void + { + $id = $this->createCategory(self::PARAMS)->body->itemId; + + $r = $this->callApi(AclActionsInterface::CATEGORY_VIEW, ['id' => $id, 'customFields' => '1']); + + $this->assertSame(200, $r->status); + + $item = $r->body->data->data; + $this->assertObjectHasProperty('data', $item->customFields, 'the include must actually run'); + $this->assertIsArray($item->customFields->data); + } + public function testEditAction(): void { $id = $this->createCategory(self::PARAMS)->body->itemId; diff --git a/tests/Unit/Domain/Common/Adapters/ItemTraitTest.php b/tests/Unit/Domain/Common/Adapters/ItemTraitTest.php index c1966f0d1..ed68883ce 100644 --- a/tests/Unit/Domain/Common/Adapters/ItemTraitTest.php +++ b/tests/Unit/Domain/Common/Adapters/ItemTraitTest.php @@ -67,6 +67,25 @@ public function aPlainFieldIsReadAsStored() self::assertFalse($fields[0]->isValueEncrypted); } + /** + * A definition whose Help box was left blank is read like any other. + * + * `CustomFieldDefinition.help` is `varchar(255) DEFAULT NULL` and its getter is `?string`, + * while `CustomFieldItem` declares `string $help` — and this was the one nullable column of + * the three passed through without being made safe. Leaving the Help field empty is an + * ordinary thing for an administrator to do, and it turned every read of that item's custom + * fields into a TypeError: on the web a 500, and through the API a 500 whose body carried the + * class, the method and the server's absolute path. + */ + #[Test] + public function aFieldWithNoHelpTextIsReadWithoutOne() + { + $fields = $this->whenReadingTheFieldsOf($this->buildRow(['data' => 'a value', 'help' => null])); + + self::assertCount(1, $fields); + self::assertSame('', $fields[0]->help); + } + /** * The stored row's columns arrive as strings; the item they become is typed, and the view uses * the definition id to key the field it renders.