Revived #265: [WIP] Mapping support for Embeddables (VOs). #547
Conversation
…tion after some valuable comments.
Conflicts: doctrine-mapping.xsd lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php lib/Doctrine/ORM/Mapping/MappingException.php lib/Doctrine/ORM/Persisters/BasicEntityPersister.php lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php lib/Doctrine/ORM/Tools/SchemaTool.php tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.User.php tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml
|
Hello, thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link: |
| @@ -554,6 +578,6 @@ protected function getDriver() | |||
| */ | |||
| protected function isEntity(ClassMetadataInterface $class) | |||
| { | |||
| return isset($class->isMappedSuperclass) && $class->isMappedSuperclass === false; | |||
| return isset($class->isMappedSuperclass) && $class->isMappedSuperclass === false && isset($class->isEmbeddable) && $class->isEmbeddable; | |||
There was a problem hiding this comment.
This is wrong. Uou are sayign that a class is an entity only if it is embeddable. It does not make sense
There was a problem hiding this comment.
Nice catch! That solved a lot of the errors already
|
@Burgov was this rebased? |
| private function gatherEmbeddedsSql($class, $table, $schema) | ||
| { | ||
| foreach ($class->embeddedMappings as $embeddedFieldMapping) { | ||
| \Doctrine\Common\Util\Debug::dump($embeddedFieldMapping, 6); |
|
@Ocramius i merged the old branch onto master, instead of rebasing. I hope that's not too big of a problem? Otherwise is there a way to use the merge commit to rebase it? |
|
@Burgov well, you can rebase now then... That will already be quite a nightmare after so much time. You could probably squash the commits before doing that |
|
@Ocramius i'm not sure I'm following you? This PR is up to date with master, but that's thanks to the merge commit rather than a rebase action. I'm not sure how I can rebase this without redoing what I've been doing all day :) |
|
@Burgov sorry, I guess I just need to regain some focus :) my bad |
|
Just spent some more time trying to find out what is going wrong, but I just can't get the hang of it. My guess is that everything will be fixed when the test below will be fixed. However, after debugging and looking at the old PR's code, I cannot find out exactly where and how the embedded mapping is supposed to be added to the $fieldMapping property |
|
@Burgov are by any chance failing a postgresql tests? |
it appears not to be the case |
|
@Burgov I might be barking up the wrong tree, but the above looks quite like doctrine/dbal#221, that's why I asked if the fail only on postgresql |
|
@Burgov i have this feature on my todo list very high and work on it, probably for 2.5. I am not sure opening this pull request here makes sense just to have this open. We are aware of this feature request, but its the most complicated feature addition of all time. |
|
@beberlei it's good to know you're working on it (I had no way of knowing that though). Is there some kind of roadmap or an publicly visible branch for this feature? |
|
Very interested in this feature as well; +1 for publicly visible branch as soon there is something to see/test :) |
|
Killer feature. Looking forward for updates! |
I decided to merge #265 onto master in order to revive the branch. We're going to be in desperate need for something similar to this soon.
However, I'm getting a lot of errors by PHPUnit, so I clearly missed something. Can anyone tell me what needs to happen to get this stable again?