-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] DoctrineType, Add widget property to avoid retrieving the whole ChoiceList #3095
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
Conversation
…ving to retrieve the whole table content.
use Doctrine\ORM\QueryBuilder; | ||
use Doctrine\ORM\NoResultException; | ||
|
||
class EntityToArrayTransformer implements DataTransformerInterface |
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.
there is already an EntitiesToArrayTransformer
. Do you really need another transformer here ? And if yes, try to change the name to have more differences in the name to avoid confusion
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.
What about EntityToIdentifierAndPropertyTransformer
?
It is converting a Entity to an array of identifier and property.
This is the exact way I've proposed somewhere. I'll reference the issue later if I haven't closed it already. It only makes sense to be here... Otherwise we should just scrap the whole idea and promote users of sf2 to make their own types for these use cases and use the current entity transformers which is easy to do (as fosuserbundle bundle does I believe). ORMs should be responsible to allow for this functionality. I think the whole new type is kind of pointless as proposed in #1951. I'm on my iPad, but have a look at fosuserbundle and see how they use the existing entities transformer. |
+1 for this way. |
@jstout24 we don't use the existing transformer. We provide a transformer using our UserManagerInterface and allowing to enter a username and getting a user. It is not a generic implementation at all, but one that work whatever backend you sue for the FOSUserBundle storage. |
@stof ah that's right. It's been a while since I looked. I saw someone use it before :p.. Maybe it was lichess or something else. :p |
yeah, this data transformer has originally been added in FOSUserBundle to be used in lichess as ornicar implemented it there first and thought it may be useful to provide it in the bundle directly. |
A quick question.. would using "text" as a widget render a when it should be hidden? or would this end up rendering the hidden field and a text field for autocomplete functionality? |
@jstout24
So, it would possible to have
Some pending questions after Stof's remarks:
|
One more case that should be possible from my experience:
Then we need to specify a custom transformer for property text field -> entity (maybe using a closure). Example: I've got a text field for a person which is filled with autocomplete in the format: I would appreciate if such functionality would be possible with the Entity form type. |
@Tobion Yes. Thanks for this comment. As the transformer convert an entity to an array of identifier and property, I think I could add a case to retrieve an entity from the property if it's defined and if the identifier did not give a valid entity. Something like:
Does it match your case? Regards, |
Well, not exactly. The property does not need to be a database column. So you can not simply search for it. I would rather suggest to allow a closure in this case. And we still need to find a way to add real errors. |
@Tobion Ok I understand, it's a step further. In a first time, I would like to have at least the basic way. Maybe it could be the subject of an other PR in the future. |
@RapotOR killer! |
/** | ||
* Return an entity that is valid choice in the corresponding choice list. | ||
* | ||
* @return Object entity |
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.
should be object
public function getEntity($field, $value) | ||
{ | ||
$alias = $this->queryBuilder->getRootAlias(); | ||
$where = $this->queryBuilder->expr()->eq($alias.'.'.$field, $value); |
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.
DQL injection is possible here. you should not pass the value (which is a user input) using string concatenation. You should use a query parameter here and set the value for the parameter
use Symfony\Component\Form\Util\PropertyPath; | ||
use Doctrine\Common\Collections\Collection; | ||
use Doctrine\Common\Persistence\ObjectManager; | ||
use Doctrine\ORM\NoResultException; |
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.
unused use statement
@bschussek can you give your mind on this PR ? Also note that this PR will conflict with the refactoring done in #3156 |
I think that the idea behind this PR is good. This should be added as a generic option to ChoiceType though. After the refactoring done in #3156 this should be quite easy to implement, without even the need of extra value transformers. The tricky question, as already discussed above, is how to exactly design the field options to be versatile yet understandable. |
@bschussek: Can you confirm that this PR is better than the approach in #1951? |
@fabpot: Not yet, both of them are on my todo list. |
@bschussek ping @RapotOR Please rebase your branch. It conflicts with master because of the move of the tests |
@stof difficult for me at this moment. Btw, there are different concurrent ideas, it could be great to have an official guideline for that.
|
@bschussek @fabpot please give your mind about the preferred way |
New discussion on this in #6602. Can this be closed? |
Closed in favor of #6602. |
I don't mean to be rude, but the last comment is from April 2013 and Symfony 2.5 has not been supported for ages. You should seriously consider upgrading your project to a supported version. |
You can use a custom query to fetch the entities, see EntityType Field in the docs |
Please consider using our support channels for this! Thank you! |
Todo :
The PR gives a different approach of the PR #1951. Please read the discussion.
The same approach is used for DateType (regarding the widget property).
The entity is retrieved from the the hidden (or another type) field contained in the entitype (which is form and not choice anymore in this case).
How it works?
choice
, theChoiceList
is not loaded anymore; very useful for a huge table.property
is given, a second field is addedwidget
value (maybe a retriction to hidden or text?) .The idea is :
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1951, #1946