[WIP] Support UUID as a choice value for DocumentType #236
Conversation
…ument (phpcr_document form type).
…d or not (this also fixes the tests).
|
I have a better solution now, by using the |
|
@dbu Could you give feedback if this is usable? |
| @@ -82,7 +83,7 @@ public function getEntitiesByIds($identifier, array $values) | |||
| })); | |||
|
|
|||
| if (0 == count($values)) { | |||
| return array(); | |||
| return []; | |||
There was a problem hiding this comment.
this is only valid with php 5.4+. lets not change that yet, but when we go 2.0 and drop 5.3 support.
…rt 3.0; Also worked through code comments and made necessary changes. Now extends DoctrineChoiceLoader from the doctrine-bridge. Only changed code is now in there.
…t would still try to use a UUID, but now it will skip it and fall back to using IDs only.
| foreach ($values as $val) { | ||
| $where->same($val, $alias); | ||
| if (UUIDHelper::isUUID($val) && $this->classMetadata) { |
There was a problem hiding this comment.
i would change the order of the checks. and add another check on classMetadata to see if the document is referenceable.
|
Ready (I could rebase it to one commit if you would like to) |
|
yes, please squash the commits into one and then rebase on master (we touched the .travis.yml and composer.json, there will be some conflicts) |
|
@soullivaneuh are you familiar with the ORM EntityType? would you maybe have time to look at this pull request and see if you have an idea how to need less code to achieve this? the problem is that with the anonymous callback functions, we need to copy a lot of code to achieve small changes... |
| private $idReader; | ||
|
|
||
| /** | ||
| * @var null|EntityLoaderInterface |
There was a problem hiding this comment.
EntityLoaderInterface|null
Just a detail, but this will keep code consistency. ;-)
|
@dbu well, I have to admin I don't use DoctrinePHPCRBundle actually. What do you exactly want from me? If I'm not wrong, Doctrine ORM support UUID mapping: http://doctrine-orm.readthedocs.org/projects/doctrine-orm/en/latest/reference/basic-mapping.html#identifier-generation-strategies Would not this be better to propose this PR directly on Symfony Doctrine bridge, especialy for the choice loader? |
|
thanks for the feedback @soullivaneuh . |
I am trying to make the
DocumentTypesupport UUIDs too (useful for API's). It is very hacky in my opinion, maybe someone else with more experience could lead me into the right direction.Related issue #103