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

Allowed to return null for query_builder #6594

Closed
wants to merge 2 commits into from

Conversation

JonEastman
Copy link
Contributor

Q A
Doc fix? yes
New docs? no
Applies to 2.8
Fixed tickets #5934

| Q             | A
| ------------- | ---
| Doc fix?      |  yes
| New docs?     | no
| Applies to    | 2.8
| Fixed tickets | symfony#5934
@HeahDude
Copy link
Contributor

Thanks, can you please update this line too?

@@ -202,9 +202,9 @@ 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
Member

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, it's not the option value that can be null. Instead, the closure can return null.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wouterj
Copy link
Member

wouterj commented May 21, 2016

Hi @JonEastman! Thanks for your contribution, I've left 2 minor comments.

@JonEastman
Copy link
Contributor Author

@wouterj I made the suggested changes, let me know what you think

@wouterj
Copy link
Member

wouterj commented May 21, 2016

👍 Looks good!

@xabbuh
Copy link
Member

xabbuh commented May 21, 2016

👍

When using a Closure, you will be passed the ``EntityRepository`` of the entity
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 query_builder closure.
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

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

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

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

Choose a reason for hiding this comment

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

Copy link
Contributor

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

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

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

Choose a reason for hiding this comment

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

wouterj added a commit that referenced this pull request May 21, 2016
This PR was merged into the 2.3 branch.

Discussion
----------

Fixed query_builder option

doc fix: 2.3+

ref #6594 (comment)

Commits
-------

45f586b Fixed query_builder option
@@ -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, a Closure or null.
The value of this option can either be a ``QueryBuilder`` object, a Closure or ``null``.
Copy link
Contributor

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

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.

wouterj added a commit that referenced this pull request May 21, 2016
This PR was squashed before being merged into the 2.8 branch (closes #6594).

Discussion
----------

 Allowed to return null for query_builder

| Q             | A
| ------------- | ---
| Doc fix?      |  yes
| New docs?     | no
| Applies to    | 2.8
| Fixed tickets | #5934

Commits
-------

f84a1eb  Allowed to return null for query_builder
@wouterj
Copy link
Member

wouterj commented May 21, 2016

Thanks @JonEastman! I've merged your PR in the documentation and added a little versionadded box indicating that this is a new feature in f64fe19 (fixed the version in a later commit).

@wouterj wouterj closed this May 21, 2016
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.

4 participants