8000 [Form] Fix constraints could be null if not set by DZunke · Pull Request #16897 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Form] Fix constraints could be null if not set #16897

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.

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

Closed
wants to merge 4 commits into from
Closed

[Form] Fix constraints could be null if not set #16897

wants to merge 4 commits into from

Conversation

DZunke
Copy link
Contributor
@DZunke DZunke commented Dec 8, 2015
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

If the form options has no key for constraints the default should be an empty array to be ignored by the loop in the FormValidator. The default without this fix is null and foreach will throw an error.

The "Bug" also still exists in master-Branch.

If the form options has no key for constraints the default should be an empty array to be ignored by the loop.
@jakzal
Copy link
Contributor
jakzal commented Dec 10, 2015

Can you add a test case please?

@jakzal jakzal added the Form label Dec 10, 2015
@DZunke
Copy link
Contributor Author
DZunke commented Dec 10, 2015

@jakzal hope the test is ok. needed to get my own FormBuilder Instance cause the default "getBuilder" method used by all other tests has default options including the constraints index.

@@ -123,6 +123,19 @@ public function testDontValidateIfParentWithoutCascadeValidation()
$this->assertNoViolation();
}

public function testNotExistingConstraintIndex()
{
$object = $this->getMock('\stdClass');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you see any advantage of creating a dummy double for stdClass rather than simply creating the object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. I'd only copied cause of the lack of knowledge in writing tests. Thanks for the Hint!

@@ -123,6 +123,17 @@ public function testDontValidateIfParentWithoutCascadeValidation()
$this->assertNoViolation();
}

public function testNotExistingConstraintIndex()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we call it testMissingConstraintIndex()?

@fabpot
Copy link
Member
fabpot commented Jan 25, 2016

Thank you @DZunke.

fabpot added a commit that referenced this pull request Jan 25, 2016
This PR was squashed before being merged into the 2.3 branch (closes #16897).

Discussion
----------

[Form] Fix constraints could be null if not set

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

If the form options has no key for constraints the default should be an empty array to be ignored by the loop in the FormValidator. The default without this fix is ```null``` and foreach will throw an error.

The "Bug" also still exists in master-Branch.

Commits
-------

f80e0eb [Form] Fix constraints could be null if not set
@fabpot fabpot closed this Jan 25, 2016
@fabpot fabpot mentioned this pull request Feb 3, 2016
This was referenced Feb 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0