EditCustomerOptionsAction not compatible for Sylius >= 1.9

[Loocos]

Hi there,

I just noticied during my test than there's a missing update on the EditCustomerOptionsAction controller 👍

$this->eventDispatcher->dispatch( 'brille24.order_item.pre_update', new ResourceControllerEvent($orderItem) ); should become $this->eventDispatcher->dispatch( new ResourceControllerEvent($orderItem), 'brille24.order_item.pre_update' );

Same for : $this->eventDispatcher->dispatch( 'brille24.order_item.post_update', new ResourceControllerEvent($orderItem) ); Should become : $this->eventDispatcher->dispatch( new ResourceControllerEvent($orderItem), 'brille24.order_item.post_update' );

Thanks!

[seizan8]

Correct. It seems like this needs refactoring.

However, I did not figure out where that Action is linked. Am I wrong or is the link to it never used in the plugin? If I try to open the URL manually I get a different error btw. "brille24.sylius_customer_options_plugin.controller.edit_customer_options_action" has no container set, did you forget to define it as a service subscriber?

To me it seems like this is an old Action that is not used anymore. What is it supposed to do? Update orderItems? That's done by the Brille24\SyliusCustomerOptionsPlugin\Services\CustomerOptionRecalculator, which doesn't need/use that action.

[Loocos]

@seizan8 Yeah I'm getting this error too 'has no container set, did you forget to define it as a service subscriber?', looks like we need to make an alias in our app since sf5

The action is on order show, on each item, which allow you to watch all the options done by the user :

Then, what should be done on my/your side ?

[Loocos]

Watch this file : https://github.com/Brille24/SyliusCustomOptionsPlugin/blob/master/tests/Application/templates/bundles/SyliusAdminBundle/Order/Show/Summary/_item.html.twig On line 25, there's the concerned action button

[mamazu]

Hey thanks for noticing this. Oddly we are currently running this plugin in our Sylius 1.10 (Symfony 4) build and it still works. (Maybe we do have some more overrides though).

However you are completely correct that with Symfony 5 and the change in the Event Dispatcher this isn't possible anymore. Would you mind creating a pull request for it?

[seizan8]

Since Symfony 5.0 dispatcher expects an object as first argument for the dispatch method. You could still pass the name first with v4.4. Using the name first is deprecated since 4.3 though.

https://github.com/symfony/symfony/blob/5.0/src/Symfony/Component/EventDispatcher/EventDispatcher.php

[Loocos]

So I can make a PR to fix it for Symfony 5. Should we handle the case for Symfony 4 ?

[seizan8]

A PR would be nice. The event can simply be passed first and the name after. This works with symfony ^4.3 too. Maybe also add "symfony/event-dispatcher": "^4.3 || ^5.0", to the composer.json

[Loocos]

Alright! Gonna do it