Update priority of FosUser\EventListener #1065
Conversation
|
Great! Thank you very much for finding and fixing the problem. |
|
Can you rebase on 2.0 also? Thanks! |
|
I added a unit test, but it feels a bit ugly (especially the path mapping to load the configs), let me know if there is a better way. |
|
IMO a Behat (integration) test will be easier to do and more useful. |
|
Isn't this enough to test the priority? IMO, the less behat tests we have the better. |
|
But we need to be sure that this feature actually works :) Testing the priority is not enough (proof: this PR) |
|
The integration is already tested separately, so IMO the priority is the only thing that still needs to be tested because that is what is broken. So I have kept the unit test to ensure the priority is set correctly, and added another step to the integration test to ensure the password hashing works when creating users |
|
Rebase done |
| $this->assertGreaterThan( | ||
| $viewListener->getTag('kernel.event_listener')[0]['priority'], | ||
| $fosListener->getTag('kernel.event_listener')[0]['priority'], | ||
| "api_platform.fos_user.event_listener priority needs to be greater than that of api_platform.listener.view.serialize" |
There was a problem hiding this comment.
Needs simple quotes, lint failed :)
|
Thanks @pierredup! |
Update priority of FosUser\EventListener
Since #597, the
ApiPlatform\Core\EventListener\SerializerListener(formerlySerializerViewListener) class changed priorty from 10 to 16.The
ApiPlatform\Core\Bridge\FosUser\EventListenerclass has a priority of 11, which means it now runs after theSerializerListener. This causes the controller result to be already serialized, which fails the checks in the FOSUser listener.This PR updates the priority of
ApiPlatform\Core\Bridge\FosUser\EventListenerto 24, so that it can run before theSerializerListenerclass.