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

Event Consistency #7305

Closed
mbabker opened this issue Jan 20, 2017 · 4 comments · Fixed by #8546
Closed

Event Consistency #7305

mbabker opened this issue Jan 20, 2017 · 4 comments · Fixed by #8546
Labels
RFC Discussions about potential changes or new features.
Milestone

Comments

@mbabker
Copy link
Contributor

mbabker commented Jan 20, 2017

So today I'm adding a feature for my app somewhat unrelated to the Sylius inner workings, but part of what I'm doing has me hooking into the various controller events to catch certain activities. Looking at the base ResourceController, I'm noticing some potential inconsistencies. Any particular reason for this? Ideally I'd suggest things be consistent across the board and an event is always dispatched before handling the view, but this might require a B/C breaking change in Sylius\Bundle\ResourceBundle\Controller\EventDispatcherInterface to allow a null resource for the index action or dispatching the event with the result of the Sylius\Bundle\ResourceBundle\Controller\ResourcesCollectionProviderInterface::get() call which isn't a singular resource.

  • showAction
    • Dispatches a show event before creating the view
  • indexAction
    • No event dispatched
  • createAction
    • pre/post events dispatched when POSTing data
    • No event dispatched for rendering a view (GET request)
  • updateAction
    • pre/post events dispatched when submitting data (POST, PUT, PATCH requests)
    • No event dispatched for rendering a view (GET request)
  • deleteAction
    • pre/post events dispatched
@pjedrzejewski pjedrzejewski added the RFC Discussions about potential changes or new features. label Jan 20, 2017
@ste93cry
Copy link
Contributor

This is somehow related to #7804

@mvar
Copy link
Contributor

mvar commented Mar 22, 2017

I see that updateAction() now supports custom response by setting it to the event (code here).

IMHO, createAction() should follow the same pattern.

@dannyvw
Copy link
Contributor

dannyvw commented May 25, 2017

deleteAction() should also follow the same pattern.

@stefandoorn
Copy link
Contributor

@pjedrzejewski Wouldn't it make sense to fix this before 1.0? I was looking into the 'index' event, but it's not being fired. Would be very helpful to have it I think, especially for plugins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Discussions about potential changes or new features.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants