10000 [Form] delete_empty don't work for embedded forms without data_class · Issue #22008 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] delete_empty don't work for embedded forms without data_class #22008

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.

Sign up for GitHub

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

Jump to bottom
Closed
murilolobato opened this issue Mar 15, 2017 · 7 comments
Closed

[Form] delete_empty don't work for embedded forms without data_class #22008

murilolobato opened this issue Mar 15, 2017 · 7 comments
Labels
Bug Form Status: Needs Review Unconfirmed

Comments

@murilolobato
Copy link
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 2.8.18

I'm creating a form, that has a collection of another form. OrderType has a collection of ItemType. Assuming that both have the option data_class = null, empty items are not being removed during form submit. Empty Items are being validated. The expected behavior was that empty items would be removed, as it happens when I create entities for this two types and set them in type's data_class options.

Looking at the source code and other issues, people already have had problem with delete_empty, but this is a new case. I've also found the tests to delete_empty and the scenario I've listed above is not being tested.

Here is the test with data_class defined: https://github.com/symfony/Form/blob/master/Tests/Extension/Core/Type/CollectionTypeTest.php#L145

I'll try to create a test, and attach to this issue, but I can't promise anything.
@murilolobato murilolobato mentioned this issue Mar 15, 2017
Closed
@murilolobato
Copy link
Author

Maybe @peterrehm or @cordoval could help, as they're involved at the creation of delete_empty.

@yceruto
Copy link
Member
yceruto commented Mar 15, 2017 •
edited
Loading

@murilolobato if 'allow_delete' => false then 'delete_empty' => true does not work (see test https://github.com/murilolobato/symfony/blob/ede90e8957195a15223fd5d689b4198801852a56/src/Symfony/Component/Form/Tests/Extension/Core/Type/CollectionTypeTest.php#L142)

Could you try with 'allow_delete' => true also?

Edit: The documentation talks about this too
http://symfony.com/doc/master/reference/forms/types/collection.html#delete-empty

@murilolobato
Copy link
Author
murilolobato commented Mar 15, 2017 •
edited
Loading

@yceruto I've updated the pull request, now with the allow_delete to my test. It still fails, I already knew that it would fail too, beacuse in my real app I've made a bunch of tests and tentatives.

And you touched a nice point, because the test I wrote is a copy from the test above, and in that test there is no allow_delete, but works...

@yceruto
Copy link
Member
yceruto commented Mar 15, 2017

Well, I see that, still with 'allow_delete' => false if it's a new element, then, it should be removed 👍

@HeahDude
Copy link
Contributor

Maybe related to #13601? @murilolobato could the approach of #20496 solve your problem?

@murilolobato
Copy link
Author

In #13601, I think that the functionality proposed by @Koc is not needed in that usecase. But maybe, this functionality could help me, because I would teach to the form how to determine if a item is empty, and if it is empty it would be removed from the collection and everything would work, in theory.

The question that @yceruto made in the pull request is very convenient too, it is worth to take a look.

@murilolobato
Copy link
Author

Closing in favor of #13601

@Koc Koc mentioned this issue Mar 24, 2017
Merged
fabpot added a commit that referenced this issue Jul 26, 2017
@fabpot
feature #20496 [Form] Allow pass filter callback to delete_empty opti…
c0d99d1 
…on. (Koc)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] Allow pass filter callback to delete_empty option.

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #13601, #13940, #22008, #22014
| License       | MIT
| Doc PR        | coming soon

Commits
-------

8630abe [Form] Allow pass filter callback to delete_empty option.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Form Status: Needs Review Unconfirmed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@murilolobato @xabbuh @yceruto @HeahDude @carsonbot
0