Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Make validator param nullable #11512

Merged
merged 2 commits into from
Dec 16, 2024
Merged

API Make validator param nullable #11512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1ae139
API Make validator param nullable
emteknetnz Dec 16, 2024
bfe6f94
MNT Fix unit test
emteknetnz Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Forms/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ class Form extends ModelData implements HasRequestHandler
protected $name;

/**
* @var Validator
* @var null|Validator
*/
protected $validator;
protected $validator = null;

/**
* @see setValidationResponseCallback()
Expand Down Expand Up @@ -635,7 +635,7 @@ public function transform(FormTransformation $trans)

/**
* Get the {@link Validator} attached to this form.
* @return Validator
* @return null|Validator
*/
public function getValidator()
{
Expand All @@ -644,10 +644,10 @@ public function getValidator()

/**
* Set the {@link Validator} on this form.
* @param Validator $validator
* @param null|Validator $validator
* @return $this
*/
public function setValidator(Validator $validator)
public function setValidator(?Validator $validator)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to have this param nullable, as there is no default and you should be able to 'unset' any existing validators, as there is no longer a need to always have a validator now that the Form will loop form fields and call validate() on them

{
if ($validator) {
$this->validator = $validator;
Expand Down
6 changes: 2 additions & 4 deletions tests/php/Forms/DropdownFieldTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ public function testEmptySourceDoesntBlockValidation()
$this->assertTrue($field->validate()->isValid());
}

public function provideGetDefaultValue(): array
public static function provideGetDefaultValue(): array
{
return [
[
Expand Down Expand Up @@ -648,9 +648,7 @@ public function provideGetDefaultValue(): array
];
}

/**
* @dataProvider provideGetDefaultValue
*/
#[DataProvider('provideGetDefaultValue')]
public function testGetDefaultValue(mixed $value, bool $hasEmptyDefault, mixed $expected): void
{
$field = new DropdownField('MyField', source: ['one' => 'one', 'two' => 'two', '3' => 'three']);
Expand Down
4 changes: 2 additions & 2 deletions tests/php/Forms/TreeDropdownFieldTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,9 @@ public function testTreeSearchUsingSubObject()
$cssPath = 'ul.tree li#selector-TestTree-' . $subObject1->ID . ' li#selector-TestTree-' . $subObject1ChildB->ID . ' a';
$firstResult = $parser->getBySelector($cssPath);
$this->assertEquals(
$subObject1ChildB->Name,
$subObject1ChildB->Title,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test used to pass by accident. The fixtures in TreeDropdownFieldTest.yml have a Title property, not a Name property

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be interesting to know what we changed that stopped that from working, but you're right that it shouldn't have been.

(string)$firstResult[0],
$subObject1ChildB->Name . ' is found, nested under ' . $subObject1->Name
$subObject1ChildB->Title . ' is found, nested under ' . $subObject1->Title
);

// other objects which don't contain the keyword 'SubObject' are not returned in search results
Expand Down
Loading