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

DocumentChoiceList supports grouping choices #59

Closed
wants to merge 1 commit into from

Conversation

biozshock
Copy link
Contributor

Hi!
Added possibility to make optgroups with document field type.
Basically it's symfony/symfony#2464 but adopted for ODM.

@stof stof mentioned this pull request Dec 19, 2011
@jwage
Copy link
Member

jwage commented Dec 22, 2011

rebase? 👍

@stof
Copy link
Member

stof commented Dec 22, 2011

@jwage a far better solution would be to use the code from the bridge as we made it more generic with @beberlei.

There is only one issue to make it fully generic but the only way to solve it efficiently is to add a method in the Common interface (looping over all properties to find the identifier is silly). The method is already implemented in the ORM but not yet part of the interface. See my comment on symfony/symfony#2942
Implementing the method in the ODM would make it possible to refactor the form integration by using only 2 classes in the bundle: a Symfony\BridgeDoctrine\Form\ChoiceList\EntityLoaderInterfaceimplementation for the MongoDB QueryBuilder and theDocumentTypeextendingDoctrineTypeto create it. There would be one BC break for the users: the option used to change the manager name when using a non-default one would beeminstead ofdocument_manager` as @beberlei has not made it configurable.

@jwage
Copy link
Member

jwage commented Dec 22, 2011

Lets wait then until the other blockers are resolved.

@stof
Copy link
Member

stof commented Dec 22, 2011

@jwage the only blocker to do this is to implement the method in the ODM (adding it in the interface is needed to be sure that all Doctrine project have it but does not forbid to implement it in the ODM). I can provide the PR for the ODM and then the bundle if you want.

@jwage
Copy link
Member

jwage commented Dec 22, 2011

Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants