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

Support ORM's embeddables #108

Open
wants to merge 2 commits into
base: 1.3.x
Choose a base branch
from
Open

Conversation

malarzm
Copy link

@malarzm malarzm commented Jan 27, 2020

Today at work I've realized there's currently no rules for ORM's embeddables so here's an attempt to change that :) I'll remove WIP once I'm done with my goals, for now I'm making a PR for early feedback (or to learn that somebody else is already working on this).

Goals:

  • Check if property's type agrees with ORM's knowledge
  • Check columns specified in embeddables just like for entities

@@ -64,9 +68,6 @@ public function processNode(Node $node, Scope $scope): array
}

$className = $class->getName();
if ($objectManager->getMetadataFactory()->isTransient($className)) {
Copy link
Author

Choose a reason for hiding this comment

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

I'm trying to figure out internally why the embeddables are considered transient, but removal of this check causes no test failures

Copy link
Author

Choose a reason for hiding this comment

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

Also classes not managed by Doctrine will have an exception thrown a bit later while trying to get their metadata and will return early from the rule with no errors, so I believe the intent is preserved

Copy link
Author

Choose a reason for hiding this comment

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

I've created an issue in ORM but I think we can continue without waiting for its resolution doctrine/orm#8006

@malarzm malarzm changed the title [WIP] Support ORM's embeddables Support ORM's embeddables Jan 28, 2020
@malarzm
Copy link
Author

malarzm commented Jan 30, 2020

PR is now ready :)

@malarzm malarzm requested a review from lookyman February 7, 2020 12:30
@malarzm
Copy link
Author

malarzm commented Feb 24, 2020

Anybody? @lookyman @ondrejmirtes ?

@ondrejmirtes
Copy link
Member

Hi, sorry for keeping you waiting, I'll look into this when I have the time.

@enumag
Copy link
Contributor

enumag commented Nov 17, 2021

Any news on this one? It would be helpful imo. :-)

@kissifrot
Copy link

Looks like this PR has conflicts, but it would be helpful to have it merged :)

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.

4 participants