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.

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

[Form] Make sure to collect child forms created on *_SET_DATA events #33999

Merged
merged 1 commit into from Oct 26, 2019

Conversation

@yceruto
Copy link
Member

yceruto commented Oct 16, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #29291
License MIT
Doc PR -

See reproducer provided by @WubbleWobble https://github.com/WubbleWobble/symfony-issue-29291.

@stof

This comment has been minimized.

Copy link
Member

stof commented Oct 16, 2019

wouldn't this have a bad side-effect if the call to setData happens after creating the form rather than passing the data to the form factory directly ?

@yceruto

This comment has been minimized.

Copy link
Member Author

yceruto commented Oct 16, 2019

@stof in that case $this->defaultDataSet is already true and setData() wouldn't be called again.

@yceruto

This comment has been minimized.

Copy link
Member Author

yceruto commented Oct 16, 2019

#29291 (comment)
I think this is related to the fact that the fields are added in a PRE_SUBMIT listener. Maybe extractConfiguration is not called properly for dynamic fields, and so the data is incomplete.

I've also checked the case when dynamic child forms are added on PRE_SUBMIT event and they are correctly collected thanks to this code:

if (!isset($this->dataByForm[$hash])) {
// field was created by form event
$this->collectConfiguration($form);
$this->collectDefaultData($form);
}

@yceruto yceruto force-pushed the yceruto:form_data_collector_34 branch from 093d50e to f8e779a Oct 16, 2019
@yceruto

This comment has been minimized.

Copy link
Member Author

yceruto commented Oct 16, 2019

I've changed the solution consistent with collectSubmittedData() approach when the form hash doesn't exist.

@OskarStark

This comment has been minimized.

Copy link
Contributor

OskarStark commented Oct 17, 2019

Is it possible to add a testcase?

@yceruto yceruto force-pushed the yceruto:form_data_collector_34 branch 2 times, most recently from 703f79e to 3d5f12b Oct 17, 2019
@xabbuh
xabbuh approved these changes Oct 18, 2019
@yceruto yceruto force-pushed the yceruto:form_data_collector_34 branch from 3d5f12b to 50efc1a Oct 23, 2019
yceruto added a commit that referenced this pull request Oct 26, 2019
…ATA events (yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] Make sure to collect child forms created on *_SET_DATA events

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #29291
| License       | MIT
| Doc PR        | -

See reproducer provided by @WubbleWobble https://github.com/WubbleWobble/symfony-issue-29291.

Commits
-------

50efc1a Make sure to collect child forms created on *_SET_DATA events
@yceruto yceruto merged commit 50efc1a into symfony:3.4 Oct 26, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@yceruto yceruto deleted the yceruto:form_data_collector_34 branch Oct 26, 2019
This was referenced Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.