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

entity: add new entity-listener tag #25

Closed
wants to merge 1 commit into from

Conversation

beuss
Copy link
Contributor

@beuss beuss commented Apr 9, 2018

Allow entities to declare listeners that'll be invoked during lifecycle.
This allows to perform consistency checks in a more reliable way than
overriding repository save() method as listeners are called even in case
of cascading persist.
Annotation has been modified to allow avoiding imports since for
@EntityListeners we need to pass .class which doesn't play well with
autoimport. Alternative would have been to import and append '.class' to
annotation param by the mean of another boolean parameter.

@axeloradmin
Copy link

We already have plan for this feature. However, it was not implemented because of one limitation of listeners (we can't use entity manager inside them). Still it can be useful for pure computation/validation. We'll consider it for next 5.1 or 5.2 release.

BTW, would you please sign the CLA?

@beuss
Copy link
Contributor Author

beuss commented Apr 11, 2018

Hi,
Yes, the limitation is quite annoying but as you said, it can still be useful for basic consistency checks.
The CLA has been signed and sent long time ago (25/10/2017) and sent to [email protected] as requested
Regards

@axeloradmin
Copy link

Would you please rebase this patch on dev branch? We'll integrate it in 5.1 release.

@beuss beuss changed the base branch from wip to dev December 4, 2018 16:41
@beuss
Copy link
Contributor Author

beuss commented Dec 4, 2018

@axeloradmin Done :-)

Copy link

@axeloradmin axeloradmin left a comment

Choose a reason for hiding this comment

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

See the comments and make the changes accordingly.

@beuss
Copy link
Contributor Author

beuss commented Dec 11, 2018

@axelor all requesdt changes have been taken into account

@axeloradmin
Copy link

Would you please make the required change listeners += it.'@class'.text() and amend the commit? We are planing to merge this for 5.1.0 soon.

@beuss
Copy link
Contributor Author

beuss commented Jan 3, 2019

Would you please make the required change listeners += it.'@class'.text() and amend the commit? We are planing to merge this for 5.1.0 soon.

Hi

Can't see what you're talking about, for me all changes (including the listenerClass to class rename) have been applied. Can you be more specific? (sorry if it's obvious)

Regards

@axeloradmin
Copy link

Change line 209 to listeners += it.'@class'.text(). There is no need to append .class.

Allow entities to declare listeners that'll be invoked during lifecycle.
This allows to perform consistency checks in a more reliable way than
overriding repository save() method as listeners are called even in case
of cascading persist.
Annotation has been modified to allow avoiding imports since for
@EntityListeners we need to pass .class which doesn't play well with
autoimport. Alternative would have been to import and append '.class' to
annotation param by the mean of another boolean parameter.
@beuss
Copy link
Contributor Author

beuss commented Jan 30, 2019

Change line 209 to listeners += it.'@class'.text(). There is no need to append .class.

Done

@axeloradmin
Copy link

The patch is cherry picked and amended with few changes. It's now included in dev branch. Thanks!

@beuss beuss deleted the entity-listeners branch February 4, 2019 13:39
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