Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Removing some extra details from #6444#6600
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Removing some extra details from #6444#6600
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| choice_value | ||
| ~~~~~~~~~~~~ | ||
| In the ``EntityType``, this defaults to the ``id`` of the entity, if it can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A question: does"the id of the entity" refer explicitly to a property calledid or does it refer to the primary key value, whatever the property is called (and it works even for composite primary keys)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it refers to the PK, but I believe none of this works the same with composite PK's, but I'm not sure exactly what happens on that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It works at the moment the identifier is in a single column (any PK or association), seehttps://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php#L112 for more details.
xabbuh commentedMay 23, 2016
👍 |
| If the id cannot be read, for BC, the component checks if the class implements | ||
| ``__toString()`` and will use an incremental integer otherwise. | ||
| In the ``EntityType``, this is overridden to use the ``id`` by default. When the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks@weaverryan, I agree with those steps. However this information inchoice_name is quite important and should remain in a "note" or "caution" block IMHO.
Because as you said in your description, using this option prevents the optimization when loading entities.
Also, saying that the__toString method or integers (as strings) should be kept since in fact, selecting choices (empty_data, API, ...) really depends on this setting.
Note that there is some work in progress (and planned to be) where feedbacks would be very welcome:#6265 and#6446.
Besides that 👍
wouterj commentedJul 2, 2016
Ping@weaverryan Status: needs work |
This PR was merged into the 2.7 branch.Discussion----------Removing some extra details from#6444See#6444 - I wanted to accomplish 2 things:1) Ideally remove needing to duplicate the option - i.e. *try* to re-use the existing `.inc` file, as long as things remain clear. Also, the original `.inc` actually explain what the point of the option is.2) Removing some extra low-level details that I'm not sure are understandable.The only thing I wasn't sure about was related to `choice_values`. If I'm using an API, and I want the user to be able to submit some string (e.g. the `username` for a User instead of the id), is this possible by setting this to `username`? Or will that mess up how the entities are queried?Commits-------2336e88 Trying to remove some duplication and some extra details
See#6444 - I wanted to accomplish 2 things:
Ideally remove needing to duplicate the option - i.e.try to re-use the existing
.incfile, as long as things remain clear. Also, the original.incactually explain what the point of the option is.Removing some extra low-level details that I'm not sure are understandable.
The only thing I wasn't sure about was related to
choice_values. If I'm using an API, and I want the user to be able to submit some string (e.g. theusernamefor a User instead of the id), is this possible by setting this tousername? Or will that mess up how the entities are queried?