8000 [Form] getParent() does not receive correct options array by Burgov · Pull Request #3793 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Form] getParent() does not receive correct options array #3793

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 1 commit into from

Conversation

Burgov
Copy link
Contributor
@Burgov Burgov commented Apr 5, 2012

This PR is based on PR #3789. I wrote two tests which show exactly what functionality was broken by PR #3290. Unfortunately one of them (testChildDefaultOptionIsPassedToChildsGetParentMethod) is still broken after PR #3789.

The default options of children or even the type itself are still not passed to the getParent method, forcing users to pass these options to the factory every time they use the type. A more specific use case is when you create a type which has "choice" as it's parent. When in its default options it defines "expanded"=>true and "multiple"=>true, the getParent() of the "choice" type should always return "form", otherwise a dataMapper will never be assigned and the checkbox list shows empty, even if some values are "true".

At this moment the getParent method reads as follows:

public function getParent(array $options)
{
    return isset($options['expanded']) && $options['expanded'] ? 'form' : 'field';
}

If the option was not passed to the factory (e.g. from the controller), it seems to just assume the form is not "expanded", which sounds wrong to me.

Hopefully this test can help fix the problem!

8000
$type = new Fixtures\FooChildType();

$builder = $this->factory->createNamedBuilder($type, 'foo_child');
$this->assertEquals(3, count($builder->getTypes()));
Copy link
Contributor

Choose a reason for hiding this comment

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

assertCount() should be used here, and below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@webmozart
Copy link
Contributor

(copied from the previous PR for reference)

Thank you for the PR. Unfortunately there is no way this could ever work. In order to determine the specific options used to instantiate a form, you'd have to know the complete inheritance tree. So you cannot know the options when building the tree. I'm starting to think that this whole dynamic inheritance thing is flawed.

@Burgov
Copy link
Contributor Author
Burgov commented Apr 19, 2012

fixed by #3878

@Burgov Burgov closed this Apr 19, 2012
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.

3 participants
0