-
Notifications
You must be signed in to change notification settings - Fork 381
Use the new actor provider interface #500
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
Conversation
0a42b16
to
df53613
Compare
df53613
to
1415e20
Compare
Thanks @mbabker |
@stof, just wondering if there's any ETA on the next release? Thanks so far :) |
I still have 1 change I want to do to be compatible with the new way of doing things (regarding the way the locale is handled). My plan is to do it this weekend (hopefully) and then do a release. |
<call method="setActorProvider"> | ||
<argument type="service" id="stof_doctrine_extensions.tool.actor_provider" /> | ||
</call> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the feeling that adding this, in conjunction with the LoggableListener::getUsername
implementation, creates a BC break: even if one calls LoggableListener::setUsername
manually, the fact that the actor is always injected will make the actor takes precedence over the value injected by the user in every scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of our projects was affected by it. It took a while to debug, but fortunately, the change was made quickly by creating a new ActorProvider and set the User there.
Follows doctrine-extensions/DoctrineExtensions#2914 and the 3.19 release
Replaces #499
Fixes #498
Probably implicitly fixes some other issues as well