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

Full embedded support #14

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pink6440
Copy link

Here is a first try to overrided embedded property class in the same way as for entities.

In addition to previous PR about light embedded support :

  • i had create another listener with a higher priority

The purpose of this listener is to replace the mappings of fields related to embedded properties.

I had take the assumption that properties of an embeddable class are "simple" and won't be overloaded in the child class.

This was referenced May 28, 2019
Copy link
Owner

@joschi127 joschi127 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!
Thank you very much for your effort. This looks very good.

I've left a few minor and easy to fix comments.

It's hard for me to check this and test it myself without building an example project. And it's quite complex what you had to add. Would it be possible for you to also add a simple test case for this? See the Tests/Functional/src for other tests. Maybe you can easily add one similar to those?

.gitconfig Outdated Show resolved Hide resolved
EventListener/LoadORMMetadataSubscriber.php Outdated Show resolved Hide resolved
EventListener/LoadORMMetadataSubscriber.php Outdated Show resolved Hide resolved
EventListener/LoadORMMetadataSubscriber.php Outdated Show resolved Hide resolved
@pink6440
Copy link
Author

I still have to finish the test.

@pink6440
Copy link
Author

I had a test on the existing pass on user ;
Just asserting that the overrided $adresse property is of type EmbeddedAdressV2.

@pink6440
Copy link
Author

@joschi127 hi ! I'm not sure about this : are my last commits visible to you ? (ones with the fix for the typos and the ones with the tests)

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.

2 participants