From 25ed64f1279c26fb09f72dff4247f899fa9d808f Mon Sep 17 00:00:00 2001 From: Mason Dechaineux Date: Mon, 2 Dec 2024 10:46:32 +1000 Subject: [PATCH 1/2] FIX eagerLoad crash with nonterminal hasOne relation --- src/ORM/DataList.php | 14 ++ tests/php/ORM/DataListEagerLoadingTest.php | 130 ++++++++++++++---- .../EagerLoading/EagerLoadObject.php | 1 + .../MixedBackwardsHasManyEagerLoadObject.php | 25 ++++ .../MixedBackwardsHasOneEagerLoadObject.php | 19 +++ .../MixedBackwardsManyManyEagerLoadObject.php | 16 +++ 6 files changed, 181 insertions(+), 24 deletions(-) create mode 100644 tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasManyEagerLoadObject.php create mode 100644 tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasOneEagerLoadObject.php create mode 100644 tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsManyManyEagerLoadObject.php diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index 72ccacd7113..43824002391 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -1055,7 +1055,14 @@ private function fetchEagerLoadRelations(Query $query): void $parentIDs = $topLevelIDs; $parentRelationData = $query; $chainToDate = []; + $polymorphicEncountered = false; foreach (explode('.', $relationChain) as $relationName) { + if ($polymorphicEncountered) { + $polymorphicRelation = $chainToDate[array_key_last($chainToDate)]; + throw new InvalidArgumentException( + "Invalid relation passed to eagerLoad() - $relationChain. Further nested relations are not supported after polymorphic has_one relation $polymorphicRelation." + ); + } /** @var Query|array $parentRelationData */ $chainToDate[] = $relationName; list( @@ -1074,6 +1081,9 @@ private function fetchEagerLoadRelations(Query $query): void $relationName, $relationType ); + if ($relationComponent['joinClass']) { + $polymorphicEncountered = true; + } break; case 'belongs_to': list($parentRelationData, $parentIDs) = $this->fetchEagerLoadBelongsTo( @@ -1205,6 +1215,10 @@ private function fetchEagerLoadHasOne( // into the has_one components - DataObject does that for us in getComponent() without any extra // db calls. + // fetchEagerLoadRelations expects these to be flat arrays if the relation is not polymorphic + if (!$hasOneClassField) { + return [$fetchedRecords, $fetchedIDs[$relationDataClass] ?? []]; + } return [$fetchedRecords, $fetchedIDs]; } diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php index 13c2460c8f8..ebfcdae2fb3 100644 --- a/tests/php/ORM/DataListEagerLoadingTest.php +++ b/tests/php/ORM/DataListEagerLoadingTest.php @@ -36,6 +36,9 @@ use SilverStripe\ORM\Tests\DataListTest\EagerLoading\BelongsManyManyEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\BelongsManyManySubEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\BelongsManyManySubSubEagerLoadObject; +use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedBackwardsHasManyEagerLoadObject; +use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedBackwardsHasOneEagerLoadObject; +use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedBackwardsManyManyEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedHasManyEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedHasOneEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedManyManyEagerLoadObject; @@ -76,6 +79,9 @@ public static function getExtraDataObjects() MixedHasManyEagerLoadObject::class, MixedHasOneEagerLoadObject::class, MixedManyManyEagerLoadObject::class, + MixedBackwardsHasOneEagerLoadObject::class, + MixedBackwardsHasManyEagerLoadObject::class, + MixedBackwardsManyManyEagerLoadObject::class, ]; } @@ -189,154 +195,175 @@ public function provideEagerLoadRelations(): array [ 'iden' => 'lazy-load', 'eagerLoad' => [], - 'expected' => 83 + 'expected' => 91 ], [ 'iden' => 'has-one-a', 'eagerLoad' => [ 'HasOneEagerLoadObject', ], - 'expected' => 82 + 'expected' => 90 ], [ 'iden' => 'has-one-b', 'eagerLoad' => [ 'HasOneEagerLoadObject.HasOneSubEagerLoadObject', ], - 'expected' => 81 + 'expected' => 89 ], [ 'iden' => 'has-one-c', 'eagerLoad' => [ 'HasOneEagerLoadObject.HasOneSubEagerLoadObject.HasOneSubSubEagerLoadObject', ], - 'expected' => 80 + 'expected' => 88 ], [ 'iden' => 'belongs-to-a', 'eagerLoad' => [ 'BelongsToEagerLoadObject', ], - 'expected' => 82 + 'expected' => 90 ], [ 'iden' => 'belongs-to-b', 'eagerLoad' => [ 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject', ], - 'expected' => 81 + 'expected' => 89 ], [ 'iden' => 'belongs-to-c', 'eagerLoad' => [ 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject.BelongsToSubSubEagerLoadObject', ], - 'expected' => 80 + 'expected' => 88 ], [ 'iden' => 'has-many-a', 'eagerLoad' => [ 'HasManyEagerLoadObjects', ], - 'expected' => 82 + 'expected' => 90 ], [ 'iden' => 'has-many-b', 'eagerLoad' => [ 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects', ], - 'expected' => 79 + 'expected' => 87 ], [ 'iden' => 'has-many-c', 'eagerLoad' => [ 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects.HasManySubSubEagerLoadObjects', ], - 'expected' => 72 + 'expected' => 80 ], [ 'iden' => 'many-many-a', 'eagerLoad' => [ 'ManyManyEagerLoadObjects', ], - 'expected' => 83 // same number as lazy-load, though without an INNER JOIN + 'expected' => 91 // same number as lazy-load, though without an INNER JOIN ], [ 'iden' => 'many-many-b', 'eagerLoad' => [ 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects', ], - 'expected' => 81 + 'expected' => 89 ], [ 'iden' => 'many-many-c', 'eagerLoad' => [ 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects.ManyManySubSubEagerLoadObjects', ], - 'expected' => 75 + 'expected' => 83 ], [ 'iden' => 'many-many-through-a', 'eagerLoad' => [ 'ManyManyThroughEagerLoadObjects', ], - 'expected' => 83 + 'expected' => 91 ], [ 'iden' => 'many-many-through-b', 'eagerLoad' => [ 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects', ], - 'expected' => 81 + 'expected' => 89 ], [ 'iden' => 'many-many-through-c', 'eagerLoad' => [ 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects.ManyManyThroughSubSubEagerLoadObjects', ], - 'expected' => 75 + 'expected' => 83 ], [ 'iden' => 'belongs-many-many-a', 'eagerLoad' => [ 'BelongsManyManyEagerLoadObjects', ], - 'expected' => 83 + 'expected' => 91 ], [ 'iden' => 'belongs-many-many-b', 'eagerLoad' => [ 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects', ], - 'expected' => 81 + 'expected' => 89 ], [ 'iden' => 'belongs-many-many-c', 'eagerLoad' => [ 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects.BelongsManyManySubSubEagerLoadObjects', ], - 'expected' => 75 + 'expected' => 83 ], [ 'iden' => 'mixed-a', 'eagerLoad' => [ 'MixedManyManyEagerLoadObjects', ], - 'expected' => 83 + 'expected' => 91 ], [ 'iden' => 'mixed-b', 'eagerLoad' => [ 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects', ], - 'expected' => 80 + 'expected' => 88 ], [ 'iden' => 'mixed-c', 'eagerLoad' => [ 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', ], - 'expected' => 73 + 'expected' => 81 + ], + [ + 'iden' => 'mixed-back-a', + 'eagerLoad' => [ + 'MixedBackwardsHasOneEagerLoadObject', + ], + 'expected' => 90 + ], + [ + 'iden' => 'mixed-back-b', + 'eagerLoad' => [ + 'MixedBackwardsHasOneEagerLoadObject.MixedBackwardsHasManyEagerLoadObjects', + ], + 'expected' => 89 + ], + [ + 'iden' => 'mixed-back-c', + 'eagerLoad' => [ + 'MixedBackwardsHasOneEagerLoadObject.MixedBackwardsHasManyEagerLoadObjects.MixedBackwardsManyManyEagerLoadObjects', + ], + 'expected' => 87 ], [ 'iden' => 'duplicates', @@ -349,7 +376,7 @@ public function provideEagerLoadRelations(): array 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects', 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', ], - 'expected' => 73 + 'expected' => 81 ], [ 'iden' => 'all', @@ -361,8 +388,9 @@ public function provideEagerLoadRelations(): array 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects.ManyManyThroughSubSubEagerLoadObjects', 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects.BelongsManyManySubSubEagerLoadObjects', 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', + 'MixedBackwardsHasOneEagerLoadObject.MixedBackwardsHasManyEagerLoadObjects.MixedBackwardsManyManyEagerLoadObjects', ], - 'expected' => 32 + 'expected' => 36 ], ]; } @@ -443,6 +471,13 @@ private function expectedEagerLoadRelations(): array 'mixedHasOneObj 0 1 0 1', 'mixedHasManyObj 0 1 1', 'mixedHasOneObj 0 1 1 1', + 'mixedBackwardsHasOneObj 0', + 'mixedBackwardsHasManyObj 0 0', + 'mixedBackwardsManyManyObj 0 0 0', + 'mixedBackwardsManyManyObj 0 0 1', + 'mixedBackwardsHasManyObj 0 1', + 'mixedBackwardsManyManyObj 0 1 0', + 'mixedBackwardsManyManyObj 0 1 1', 'obj 1', 'hasOneObj 1', 'hasOneSubObj 1', @@ -516,6 +551,13 @@ private function expectedEagerLoadRelations(): array 'mixedHasOneObj 1 1 0 1', 'mixedHasManyObj 1 1 1', 'mixedHasOneObj 1 1 1 1', + 'mixedBackwardsHasOneObj 1', + 'mixedBackwardsHasManyObj 1 0', + 'mixedBackwardsManyManyObj 1 0 0', + 'mixedBackwardsManyManyObj 1 0 1', + 'mixedBackwardsHasManyObj 1 1', + 'mixedBackwardsManyManyObj 1 1 0', + 'mixedBackwardsManyManyObj 1 1 1', ]; } @@ -670,6 +712,25 @@ private function createEagerLoadData( } } } + $mixedBackwardsHasOneObj = new MixedBackwardsHasOneEagerLoadObject(); + $mixedBackwardsHasOneObj->Title = "mixedBackwardsHasOneObj $i"; + $mixedBackwardsHasOneObjID = $mixedBackwardsHasOneObj->write(); + + $obj->MixedBackwardsHasOneEagerLoadObjectID = $mixedBackwardsHasOneObjID; + $obj->write(); + + for ($j = 0; $j < $numLevel2Records; $j++) { + $mixedBackwardsHasManyObj = new MixedBackwardsHasManyEagerLoadObject(); + $mixedBackwardsHasManyObj->Title = "mixedBackwardsHasManyObj $i $j"; + $mixedBackwardsHasManyObj->MixedBackwardsHasOneEagerLoadObjectID = $mixedBackwardsHasOneObjID; + $mixedBackwardsHasManyObjID = $mixedBackwardsHasManyObj->write(); + $mixedBackwardsHasOneObj->MixedBackwardsHasManyEagerLoadObjects()->add($mixedBackwardsHasManyObj); + for ($k = 0; $k < $numLevel3Records; $k++) { + $mixedBackwardsManyManyObj = new MixedBackwardsManyManyEagerLoadObject(); + $mixedBackwardsManyManyObj->Title = "mixedBackwardsManyManyObj $i $j $k"; + $mixedBackwardsHasManyObj->MixedBackwardsManyManyEagerLoadObjects()->add($mixedBackwardsManyManyObj); + } + } } } @@ -748,6 +809,16 @@ private function iterateEagerLoadData(DataList $dataList, int $chunks = 0): arra $results[] = $mixedHasManyObj->MixedHasOneEagerLoadObject()->Title; } } + $mixedBackwardsHasOneObj = $obj->MixedBackwardsHasOneEagerLoadObject(); + if ($mixedBackwardsHasOneObj) { + $results[] = $mixedBackwardsHasOneObj->Title; + foreach ($mixedBackwardsHasOneObj->MixedBackwardsHasManyEagerLoadObjects() as $mixedBackwardsHasManyObj) { + $results[] = $mixedBackwardsHasManyObj->Title; + foreach ($mixedBackwardsHasManyObj->MixedBackwardsManyManyEagerLoadObjects() as $mixedBackwardsManyManyObj) { + $results[] = $mixedBackwardsManyManyObj->Title; + } + } + } } $selectCount = $this->stopCountingSelectQueries(); } finally { @@ -1716,6 +1787,17 @@ public function testPolymorphEagerLoading(): void $this->validateMultipleAppearance($items, 4, EagerLoadObject::get()->eagerLoad('HasOnePolymorphObject'), 'HasOnePolymorphObject'); } + /** + * Tests that attempting to eager load a sub relation to a polymorphic relation will throw an exception. + */ + public function testEagerLoadingSubRelationToPolymorphicException(): void + { + $items = $this->providePolymorphHasOne(); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid relation passed to eagerLoad() - HasOnePolymorphObject.ManyManySubEagerLoadObjects. Further nested relations are not supported after polymorphic has_one relation HasOnePolymorphObject."); + EagerLoadObject::get()->eagerLoad("HasOnePolymorphObject.ManyManySubEagerLoadObjects")->toArray(); + } + protected function providePolymorphHasOne(): array { $subA = new HasOneEagerLoadObject(); diff --git a/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php b/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php index 568d715204d..c0022b5dd86 100644 --- a/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php +++ b/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php @@ -16,6 +16,7 @@ class EagerLoadObject extends DataObject implements TestOnly private static $has_one = [ 'HasOneEagerLoadObject' => HasOneEagerLoadObject::class, 'HasOnePolymorphObject' => DataObject::class, + 'MixedBackwardsHasOneEagerLoadObject' => MixedBackwardsHasOneEagerLoadObject::class, ]; private static $belongs_to = [ diff --git a/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasManyEagerLoadObject.php b/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasManyEagerLoadObject.php new file mode 100644 index 00000000000..e134ec060fe --- /dev/null +++ b/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasManyEagerLoadObject.php @@ -0,0 +1,25 @@ + 'Varchar' + ]; + + private static $has_one = [ + 'MixedBackwardsHasOneEagerLoadObject' => MixedBackwardsHasOneEagerLoadObject::class + ]; + + + private static $many_many = [ + 'MixedBackwardsManyManyEagerLoadObjects' => MixedBackwardsManyManyEagerLoadObject::class + ]; +} diff --git a/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasOneEagerLoadObject.php b/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasOneEagerLoadObject.php new file mode 100644 index 00000000000..fb3bb485585 --- /dev/null +++ b/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsHasOneEagerLoadObject.php @@ -0,0 +1,19 @@ + 'Varchar' + ]; + + private static $has_many = [ + 'MixedBackwardsHasManyEagerLoadObjects' => MixedBackwardsHasManyEagerLoadObject::class + ]; +} diff --git a/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsManyManyEagerLoadObject.php b/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsManyManyEagerLoadObject.php new file mode 100644 index 00000000000..e8ba574798b --- /dev/null +++ b/tests/php/ORM/DataListTest/EagerLoading/MixedBackwardsManyManyEagerLoadObject.php @@ -0,0 +1,16 @@ + 'Varchar' + ]; +} From f51b4f7c39a02c93cf4cc72c77b229397b04350c Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Tue, 17 Dec 2024 18:42:22 +1300 Subject: [PATCH 2/2] API Use validate method instead of validationResult --- src/Forms/Form.php | 20 +++++++++++++++----- src/Forms/FormRequestHandler.php | 2 +- tests/php/Forms/CompositeValidatorTest.php | 6 +++--- tests/php/Forms/FieldsValidatorTest.php | 2 +- tests/php/Forms/FileFieldTest.php | 8 ++++---- tests/php/Forms/FormFieldTest.php | 4 ++-- tests/php/Forms/FormSchemaTest.php | 2 +- tests/php/Forms/FormTest.php | 2 +- tests/php/Forms/GridField/GridFieldTest.php | 4 ++-- tests/php/Forms/OptionsetFieldTest.php | 2 +- tests/php/Forms/ValidatorTest.php | 4 ++-- 11 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 69151c12145..ce78802151e 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -20,6 +20,7 @@ use SilverStripe\View\AttributesHTML; use SilverStripe\View\SSViewer; use SilverStripe\View\ViewableData; +use SilverStripe\Dev\Deprecation; /** * Base class for all forms. @@ -1243,6 +1244,18 @@ public function getLegend() return $this->legend; } + /** + * Alias of validate() for backwards compatibility. + * + * @return ValidationResult + * @deprecated 5.4.0 Use validate() instead + */ + public function validationResult() + { + Deprecation::notice('5.4.0', 'Use validate() instead'); + return $this->validate(); + } + /** * Processing that occurs before a form is executed. * @@ -1254,13 +1267,10 @@ public function getLegend() * * Triggered through {@link httpSubmission()}. * - * * Note that CSRF protection takes place in {@link httpSubmission()}, * if it fails the form data will never reach this method. - * - * @return ValidationResult - */ - public function validationResult() + */ + public function validate(): ValidationResult { // Automatically pass if there is no validator, or the clicked button is exempt // Note: Soft support here for validation with absent request handler diff --git a/src/Forms/FormRequestHandler.php b/src/Forms/FormRequestHandler.php index 1071f92aba8..69deb991bf2 100644 --- a/src/Forms/FormRequestHandler.php +++ b/src/Forms/FormRequestHandler.php @@ -217,7 +217,7 @@ public function httpSubmission($request) // Action handlers may throw ValidationExceptions. try { // Or we can use the Validator attached to the form - $result = $this->form->validationResult(); + $result = $this->form->validate(); if (!$result->isValid()) { return $this->getValidationErrorResponse($result); } diff --git a/tests/php/Forms/CompositeValidatorTest.php b/tests/php/Forms/CompositeValidatorTest.php index bcd79a41caf..422935a7156 100644 --- a/tests/php/Forms/CompositeValidatorTest.php +++ b/tests/php/Forms/CompositeValidatorTest.php @@ -165,7 +165,7 @@ public function testValidate(): void // Put data into the form so the validator can pull it back out again $form->loadDataFrom($data); - $result = $form->validationResult(); + $result = $form->validate(); $this->assertFalse($result->isValid()); $this->assertCount(2, $result->getMessages()); } @@ -187,14 +187,14 @@ public function testRemoveValidation(): void // Put data into the form so the validator can pull it back out again $form->loadDataFrom($data); - $result = $form->validationResult(); + $result = $form->validate(); $this->assertFalse($result->isValid()); $this->assertCount(1, $result->getMessages()); // Make sure it doesn't fail after removing validation AND has no errors (since calling validate should // reset errors) $compositeValidator->removeValidation(); - $result = $form->validationResult(); + $result = $form->validate(); $this->assertTrue($result->isValid()); $this->assertEmpty($result->getMessages()); } diff --git a/tests/php/Forms/FieldsValidatorTest.php b/tests/php/Forms/FieldsValidatorTest.php index 8c958bcd348..b24005e7448 100644 --- a/tests/php/Forms/FieldsValidatorTest.php +++ b/tests/php/Forms/FieldsValidatorTest.php @@ -67,7 +67,7 @@ public function testValidation(array $values, bool $isValid) } $form = new Form(null, 'testForm', $fieldList, new FieldList([/* no actions */]), new FieldsValidator()); - $result = $form->validationResult(); + $result = $form->validate(); $this->assertSame($isValid, $result->isValid()); $messages = $result->getMessages(); if ($isValid) { diff --git a/tests/php/Forms/FileFieldTest.php b/tests/php/Forms/FileFieldTest.php index 18e397d30f5..2bf65a5e5d0 100644 --- a/tests/php/Forms/FileFieldTest.php +++ b/tests/php/Forms/FileFieldTest.php @@ -37,7 +37,7 @@ public function testUploadRequiredFile() ]; $fileField->setValue($fileFieldValue); - $this->assertTrue($form->validationResult()->isValid()); + $this->assertTrue($form->validate()->isValid()); } /** @@ -141,7 +141,7 @@ public function testUploadMissingRequiredFile() $fileField->setValue($fileFieldValue); $this->assertFalse( - $form->validationResult()->isValid(), + $form->validate()->isValid(), 'An error occurred when uploading a file, but the validator returned true' ); @@ -150,7 +150,7 @@ public function testUploadMissingRequiredFile() $fileField->setValue($fileFieldValue); $this->assertFalse( - $form->validationResult()->isValid(), + $form->validate()->isValid(), 'An empty array was passed as parameter for an uploaded file, but the validator returned true' ); @@ -159,7 +159,7 @@ public function testUploadMissingRequiredFile() $fileField->setValue($fileFieldValue); $this->assertFalse( - $form->validationResult()->isValid(), + $form->validate()->isValid(), 'A null value was passed as parameter for an uploaded file, but the validator returned true' ); } diff --git a/tests/php/Forms/FormFieldTest.php b/tests/php/Forms/FormFieldTest.php index 07d6c126884..d19b0148e7f 100644 --- a/tests/php/Forms/FormFieldTest.php +++ b/tests/php/Forms/FormFieldTest.php @@ -469,7 +469,7 @@ public function testGetSchemaStateWithFormValidation() $field = new FormField('MyField', 'My Field'); $validator = new RequiredFields('MyField'); $form = new Form(null, 'TestForm', new FieldList($field), new FieldList(), $validator); - $form->validationResult(); + $form->validate(); $schema = $field->getSchemaState(); $this->assertEquals( '"My Field" is required', @@ -498,7 +498,7 @@ public function testValidationExtensionHooks() // Ensure messages set via updateValidationResult() propagate through to form fields after validation $form = new Form(null, 'TestForm', new FieldList($field), new FieldList(), new RequiredFields()); - $form->validationResult(); + $form->validate(); $schema = $field->getSchemaState(); $this->assertEquals( 'A test error message', diff --git a/tests/php/Forms/FormSchemaTest.php b/tests/php/Forms/FormSchemaTest.php index c032f3a5372..d4f05bcc3f2 100644 --- a/tests/php/Forms/FormSchemaTest.php +++ b/tests/php/Forms/FormSchemaTest.php @@ -150,7 +150,7 @@ public function testGetStateWithFieldValidationErrors() 'Title' => null, ] ); - $this->assertFalse($form->validationResult()->isValid()); + $this->assertFalse($form->validate()->isValid()); $formSchema = new FormSchema(); $expected = [ 'id' => 'Form_TestForm', diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index 8b97293443a..05ad1f2055f 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -404,7 +404,7 @@ public function testLookupFieldDisabledSaving() $form->saveInto($object); $playersIds = $object->Players()->getIDList(); - $this->assertTrue($form->validationResult()->isValid()); + $this->assertTrue($form->validate()->isValid()); $this->assertEquals( $playersIds, [], diff --git a/tests/php/Forms/GridField/GridFieldTest.php b/tests/php/Forms/GridField/GridFieldTest.php index 642460aa5ac..859e9b54432 100644 --- a/tests/php/Forms/GridField/GridFieldTest.php +++ b/tests/php/Forms/GridField/GridFieldTest.php @@ -637,7 +637,7 @@ public function testValidationMessageInOutput() $form = new Form(null, "testForm", $fieldList, new FieldList(), $validator); // A form that fails validation should display the validation error in the FieldHolder output. - $form->validationResult(); + $form->validate(); $gridfieldOutput = $gridField->FieldHolder(); $this->assertStringContainsString('

error

', $gridfieldOutput); @@ -647,7 +647,7 @@ public function testValidationMessageInOutput() // A form that passes validation should not display a validation error in the FieldHolder output. $form->setValidator(new RequiredFields()); - $form->validationResult(); + $form->validate(); $gridfieldOutput = $gridField->FieldHolder(); $this->assertStringNotContainsString('

', $gridfieldOutput); } diff --git a/tests/php/Forms/OptionsetFieldTest.php b/tests/php/Forms/OptionsetFieldTest.php index 34142ec2973..e49b2020712 100644 --- a/tests/php/Forms/OptionsetFieldTest.php +++ b/tests/php/Forms/OptionsetFieldTest.php @@ -61,7 +61,7 @@ public function testValidation() $this->assertTrue($field->validate($validator)); // ... but should not pass "RequiredFields" validation - $this->assertFalse($form->validationResult()->isValid()); + $this->assertFalse($form->validate()->isValid()); //disabled items shouldn't validate $field->setDisabledItems(['Five']); diff --git a/tests/php/Forms/ValidatorTest.php b/tests/php/Forms/ValidatorTest.php index 941094b438f..56659a07797 100644 --- a/tests/php/Forms/ValidatorTest.php +++ b/tests/php/Forms/ValidatorTest.php @@ -45,13 +45,13 @@ public function testRemoveValidation() $form->setValidator($validator); // Setup validator now that we've got our form. $form->loadDataFrom($data); // Put data into the form so the validator can pull it back out again. - $result = $form->validationResult(); + $result = $form->validate(); $this->assertFalse($result->isValid()); $this->assertCount(1, $result->getMessages()); // Make sure it doesn't fail after removing validation AND has no errors (since calling validate should reset errors). $validator->removeValidation(); - $result = $form->validationResult(); + $result = $form->validate(); $this->assertTrue($result->isValid()); $this->assertEmpty($result->getMessages()); }