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] Fix assigning data in PostSetDataEvent and PostSubmitEvent #53381

Merged
merged 1 commit into from Jan 4, 2024

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Jan 3, 2024

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues #53364
License MIT

Ref #51043 where we changed the behavior on 6.4 since we don't assign the data property anymore.

cc @HeahDude

@@ -11,7 +11,6 @@

namespace Symfony\Component\Form\Event;

use Symfony\Component\Form\Exception\BadMethodCallException;
Copy link
Member

Choose a reason for hiding this comment

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

should IMO be added back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not present in PostSubmitEvent.

Copy link
Contributor

@OskarStark OskarStark Jan 3, 2024

Choose a reason for hiding this comment

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

It's not present in PostSubmitEvent.

No, but this is about PostSetDataEvent and it is in the comment for the next major:

// throw new BadMethodCallException('Form data cannot be changed during "form.post_set_data", you should use "form.pre_set_data" instead.');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I mean is that there's the same code in PostSubmitEvent without the import 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I would add the import then 👍

Copy link
Member

Choose a reason for hiding this comment

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

I'd follow what fabbot wants on this one, because this will come up again otherwise.
But we should then be careful when merging up to not remove the use in 7.0

@OskarStark OskarStark changed the title [Form] Fix assigning data in PostSetDataEvent and PostSubmitEvent [Form] Fix assigning data in PostSetDataEvent and PostSubmitEvent Jan 3, 2024
@xabbuh
Copy link
Member

xabbuh commented Jan 4, 2024

Thank you @fancyweb.

@xabbuh xabbuh merged commit 2e73037 into symfony:6.4 Jan 4, 2024
9 checks passed
@fancyweb fancyweb deleted the form/fix-set-data-call branch January 4, 2024 07:52
This was referenced Jan 31, 2024
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