Skip to content

[Form] isEmpty #13940

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
nino-s opened this issue Mar 16, 2015 · 4 comments
Closed

[Form] isEmpty #13940

nino-s opened this issue Mar 16, 2015 · 4 comments

Comments

@nino-s
Copy link

nino-s commented Mar 16, 2015

The method isEmpty checks the input form data. It checks for simple nullable elements, empty strings, empty arrays and empty traversables. Am i wrong? If i have an object, which consist only of empty collections, nullable fields etc., shouldn't it check such an 'empty' object and return true aswell?

Following use case: I have an embedded collection in an entity form. It's attributes have some asserts (notnull, empty, etc.). Adding and Removing with prototype works perfectly, but if i just add a new entry and enter nothing, it should delete the entry on submit with the flag delete_empty. Actually, it does not, because modelData is an object, therefore not empty and the standard validation errors occur.

Current isEmpty method:

    public function isEmpty()
    {
        foreach ($this->children as $child) {
            if (!$child->isEmpty()) {
                return false;
            }
        }

        return FormUtil::isEmpty($this->modelData) ||
            // arrays, countables
            0 === count($this->modelData) ||
            // traversables that are not countable
            ($this->modelData instanceof \Traversable && 0 === iterator_count($this->modelData));
    }

My idea would be, to additionally check if modelData is an object and if the object has a self made isEmpty method. (A generic version is also possible - all getter)
(is_object($this->modelData) && method_exists($this->modelData, 'isEmpty') && call_user_func(array($this->modelData, 'isEmpty')))

Best regards,
Nino
@wouterj
Copy link
Member

wouterj commented Mar 20, 2015

Hi @nino-s, thank you for such a detailed issue.

The case you describe should be fixed by the foreach ($this->children as $child) loop. If you have a collection of entities, you also have a custom type setting for the prototype form type. Such a form type has field that are bound to the object's properties. If these fields are empty, the object's properties are also empty and $child->isEmpty() will return true.

This is also demostrated in this test case: https://github.com/symfony/Form/blob/master/Tests/Extension/Core/Type/CollectionTypeTest.php#L145-166 (fixtures: AuthorType and Author).

Can you show some more context about how you're using the form component to see what's going wrong?

@nino-s
Copy link
Author

nino-s commented Mar 23, 2015

I dont know what went wrong, but after I cleared the cache complettly, I checked it again and the error was gone.

As I already described, it happened for a simple collection with more than one field (in form type) after I added asserts to each field.
So if someone have the same error in the future, clear the cache :)

@wouterj
Copy link
Member

wouterj commented Mar 23, 2015

Hmm, in that case, I would close this issue and reopen it if the error occurs once more. Probably, there was some old cache causing this behaviour

@jakzal
Copy link
Contributor

jakzal commented Mar 23, 2015

@nino-s feel free to re-open if you manage to reproduce this issue.

@jakzal jakzal closed this as completed Mar 23, 2015
This was referenced Nov 9, 2016
Closed
Merged
This was referenced Mar 15, 2017
Closed
Closed
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
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jakzal @wouterj @nino-s