Skip to content

Allowed to return null for query_builder #6594

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

Closed
JonEastman wants to merge 2 commits into from
Closed

Allowed to return null for query_builder #6594

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0e3eec
Allowed to return null for query_builder
JonEastman May 21, 2016
cff5c06
Updated null tags & line break corrections
JonEastman May 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions reference/forms/types/entity.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,10 @@ query_builder
Allows you to create a custom query for your choices. See
:ref:`ref-form-entity-query-builder` for an example.

The value of this option can either be a ``QueryBuilder`` object or a Closure.
The value of this option can either be a ``QueryBuilder`` object, a Closure or ``null``.
Copy link
Contributor

@HeahDude HeahDude May 21, 2016

Choose a reason for hiding this comment

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

Needs a rebase here when #6596 is merged in 2.8

Copy link
Member

@wouterj wouterj May 21, 2016

Choose a reason for hiding this comment

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

I'll take care of that while merging.

When using a Closure, you will be passed the ``EntityRepository`` of the entity
as the only argument and should return a ``QueryBuilder``.
as the only argument and should return a ``QueryBuilder``.
If you'd like to display a list of empty entries, you can return ``null`` in the closure.
Copy link
Contributor

@HeahDude HeahDude May 21, 2016

Choose a reason for hiding this comment

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

Unless I'm missing something when null is returned it loads all entities.

Copy link
Member

@xabbuh xabbuh May 21, 2016

Choose a reason for hiding this comment

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

When you set the option to null, but not when return null in the closure, isn't it?

Copy link
Contributor

@HeahDude HeahDude May 21, 2016

Choose a reason for hiding this comment

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

Why that? The only difference is that it's conditional. ?

Copy link
Member

@xabbuh xabbuh May 21, 2016

Choose a reason for hiding this comment

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

I don't think so (see https://github.com/symfony/symfony/pull/13990/files#diff-888009597299c21d535422fd4368bed3R72).

Copy link
Contributor

@HeahDude HeahDude May 21, 2016

Choose a reason for hiding this comment

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

Ok so I've missed the point of that feature, thanks for the enlightening!

Copy link
Contributor

@HeahDude HeahDude May 21, 2016

Choose a reason for hiding this comment

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

@xabbuh Looks like symfony/symfony@317d30b has not made it to the core !? (neither master nor 2.8)

Copy link
Member

@wouterj wouterj May 21, 2016

Choose a reason for hiding this comment

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

Looks like there was something wrong with the commit and it was fixed in symfony/symfony@eb08baa

The tests exists and passes: https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php#L212-L223

Copy link
Contributor

@HeahDude HeahDude May 21, 2016

Choose a reason for hiding this comment

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

see symfony/symfony#13990 (comment)


Overridden Options
------------------
Expand Down