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

[Resource] Fixed issue with mapping inheritance and mapped superclass (Gedmo) #3627

Merged
merged 2 commits into from
Nov 25, 2015

Conversation

pamil
Copy link
Contributor

@pamil pamil commented Nov 25, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets #1363, #3621, #1438, #478
License MIT

Probably fixes all bugs related to Gedmo DoctrineExtensions and mapping inheritance :)

@pamil pamil changed the title [Resource] Fixed issue with mapping inheritance and mapped superclass [Resource] Fixed issue with mapping inheritance and mapped superclass (Gedmo) Nov 25, 2015
pjedrzejewski pushed a commit that referenced this pull request Nov 25, 2015
[Resource] Fixed issue with mapping inheritance and mapped superclass (Gedmo)
@pjedrzejewski pjedrzejewski merged commit 0e0b33b into Sylius:master Nov 25, 2015
@pjedrzejewski
Copy link
Member

Thanks Kamil!

@pamil pamil deleted the gedmo-inheritance branch November 25, 2015 20:07
@inssein
Copy link
Contributor

inssein commented Nov 25, 2015

@pamil sweet! I will definitely be checking this out once I get home.

From taking a quick look, I can't seem to make sense of it easily. How does loading of the metadata when using cache get fixed by changing the priority of the event?

@pamil
Copy link
Contributor Author

pamil commented Nov 25, 2015

@inssein SyliusResourceBundle does some really weird things with mapping (allowing mapped superclasses to have relationships and ids, etc.). As far as I know, it happened only while metadata cache was enabled and when the initial class metadata cache was created not in main application, but in some command (I'm not 100% sure here).

Anyway, it resulted in the loadClassMetadata listeners being in the order like that:

  • SluggableListener
  • TreeListener
  • LoadORMResourcesListener
  • TimestampableListener
  • SoftDeletableListener

This is the reason we had those problems with slugs and trees, but not with much more widely used timestampable and softdeletable behaviours.

If you take a look at Gedmo's ExtensionMetadataFactory::getExtensionMetadata($meta), you'll see this code at the beginning:

if ($meta->isMappedSuperclass) {
    return; // ignore mappedSuperclasses for now
}

So our mapped superclasses were transformed into entities just after the first two Gedmo listeners were called and their metadata was empty at that moment :)

@inssein
Copy link
Contributor

inssein commented Nov 25, 2015

@pamil For me (#3621) Timestampable also wasn't working.

@inssein inssein mentioned this pull request Nov 26, 2015
@pamil
Copy link
Contributor Author

pamil commented Nov 26, 2015

@inssein It depends on how your application looked like, with ResourceBundle listeners guaranteed to be on top, it should work everywhere :)

flajos added a commit to artkonekt/Sylius that referenced this pull request Jul 21, 2017
ORM metadata caching did not contain empty data for the Gedmo (loggable) extension causing that some crucial informations were not logged (ie. changes in the Payment entity). Solution was ported from Sylius#3627, there you can find more information about the problem and the solution.
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