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 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
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
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
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
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
Copy link
Contributor

Is it possible to add a testcase?

@yceruto yceruto force-pushed the form_data_collector_34 branch 2 times, most recently from 703f79e to 3d5f12b Compare October 17, 2019 19:05
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
@yceruto yceruto deleted the form_data_collector_34 branch October 26, 2019 10:33
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
Development

Successfully merging this pull request may close these issues.

None yet

5 participants