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

Doctrine embedded entity 'make:entity --regenerate' bugfix #164

Merged
merged 1 commit into from
May 2, 2018

Conversation

sadikoff
Copy link
Contributor

Closes #160

@sadikoff sadikoff changed the title [WIP] Doctrine embedded entity 'make:entity --regenerate' bugfix Doctrine embedded entity 'make:entity --regenerate' bugfix Apr 30, 2018
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

Really cool! This, obviously, will fix the error. But, it will NOT actually generate the embeddable class, correct? The user will need to create this? (I know this exactly how we discussed it should work initially, I'm just checking).

If so, we need to do 1 more thing, and we have 2 options:
A) Warn the user that the embeddable has not been generated
B) Generate the embeddable

(B) MIGHT be easy, as embeddables are almost EXACTLY entity classes, just with a different annotation, no repository class and no id). But, it might not be easy :p.

$metadata = $this->doctrineHelper->getMetadata($classOrNamespace);
} catch (MappingException $mappingException) {
$metadata = $this->doctrineHelper->getMetadata($classOrNamespace, true);
}
Copy link
Member

Choose a reason for hiding this comment

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

Yea, that's a very practical idea.


$operations[$embeddedClasses[$fieldName]] = $this->createClassManipulator($embeddedClasses[$fieldName]);

$manipulator->addEmbeddedEntity($fieldName, $mapping['class']);
Copy link
Member

Choose a reason for hiding this comment

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

- $manipulator->addEmbeddedEntity($fieldName, $mapping['class']);
+ $manipulator->addEmbeddedEntity($fieldName, $className);

}

$this->addGetter($propertyName, $typeHint, false);
$this->addSetter($propertyName, $typeHint, false);
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious. Is a setter "normal" or wanted for an embeddable? Or is it more common to mutate it (e.g. $myEntity->getEmbeddableField()->setSomeFieldOnEmbeddable('bar'))? Or, both?

/**
* @ORM\Embeddable()
*/
class Recept
Copy link
Member

Choose a reason for hiding this comment

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

Recipe :) Fun example, btw

@weaverryan weaverryan force-pushed the doctrine-embeddables branch from 07d2162 to 0e36480 Compare May 2, 2018 13:20
@weaverryan
Copy link
Member

Thank you @sadikoff - awesome fix!

@weaverryan weaverryan merged commit 0e36480 into symfony:master May 2, 2018
weaverryan added a commit that referenced this pull request May 2, 2018
…sadikoff)

This PR was squashed before being merged into the 1.0-dev branch (closes #164).

Discussion
----------

Doctrine embedded entity 'make:entity --regenerate' bugfix

Closes #160

Commits
-------

0e36480 Doctrine embedded entity 'make:entity --regenerate' bugfix
@welcoMattic
Copy link
Member

Thanks @sadikoff 👍

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.

3 participants