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

Bug during overrides in configuration #32

Open
xavren opened this issue Nov 2, 2016 · 13 comments
Open

Bug during overrides in configuration #32

xavren opened this issue Nov 2, 2016 · 13 comments

Comments

@xavren
Copy link

xavren commented Nov 2, 2016

Hello,

I was trying the new sylius 1.0.0@alpha and tried to disable a state and transitions but it seems that the way that configuration is parsed does not respect overrides and merge all together...

I created test and made a PR, tell me if it seems ok for you and i can create the code to respect this spec

#31

@winzou
Copy link
Owner

winzou commented Nov 4, 2016

Hi,

To disable a state you can do the following:

winzou_state_machine:
    an_extended_machine:
        states:
            a_state_to_disable: "::disabled"

Is this what you are looking for?

It's handled here: https://github.com/winzou/StateMachineBundle/blob/master/DependencyInjection/winzouStateMachineExtension.php#L69

@c-revillo
Copy link

Hello, i also experiencing this problem with sylius, now with the beta. i exposed my problem here Sylius/Sylius#7095
There i've been told the solution to modify my appKernel.php a little bit and indeed it works.

And as i said there, the other thing that worked was to put everything on app/config/config.yml and mofiy the Configuration for the State Machine Bundle a little bit.

Tried the ::disabled thing too, but no luck...

@winzou
Copy link
Owner

winzou commented Jan 13, 2017

There are indeed different ways of merging symfony configuration. In your case you want to override all the states with your own array of states. But others will want to be able to just add one state, without being forced to rewrite the whole array of states. Both use cases cannot be handled at the same time :)

For your use case, you must use the ::disabled trick, which should be working. Can you try again and if it is still not working, come back here with more details?

@c-revillo
Copy link

c-revillo commented Jan 13, 2017

Sure, here's the thing. i'm trying to override the states that are defined in this sylius file https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/CoreBundle/Resources/config/app/state_machine/sylius_order_checkout.yml

As this means SyliusCoreBundle i added my own app/Resources/SyliusCoreBundle/Resources/config/app/state_machine/sylius_order_checkout.yml.

There i copied everything and added the ::disabled in one of the states like

winzou_state_machine:
    sylius_order_checkout:
        class: "%sylius.model.order.class%"
        property_path: checkoutState
        graph: sylius_order_checkout
        state_machine_class: "%sylius.state_machine.class%"
        states:
            cart: ~
            addressed: ~
            shipping_selected: '::disabled'
            payment_skipped: ~
            payment_selected: ~
            completed: ~

Now, if try debug the winzou_state_machine and look for that part i get...
php bin/console debug:config winzou_state_machine and i can see

winzou_state_machine:
    sylius_order_checkout:
        class: Sylius\Component\Core\Model\Order
        property_path: checkoutState
        graph: sylius_order_checkout
        state_machine_class: Sylius\Component\Resource\StateMachine\StateMachine
        states:
            cart: null
            addressed: null
            shipping_selected: '::disabled'
            payment_skipped: null
            payment_selected: null
            completed: null

The disabled is there... but my question is... does this means that the state has been really disabled? looking to the sylius documentation it looks like that state shouldn't appear in the output of the debug command at all...
http://docs.sylius.org/en/latest/cookbook/checkout.html

Thanks, btw!

@winzou
Copy link
Owner

winzou commented Jan 18, 2017

Can you try to execute this command debug:winzou:state-machine? Thanks

@c-revillo
Copy link

Ok, so, having the following in my app/Resources/SyliusCoreBundle/config/app/state_machine/sylius_order_checkout.yml

winzou_state_machine:
    sylius_order_checkout:
        states:
            shipping_selected: '::disabled'

executing the command if prompts me what state machine i want to know about and selecting the sylius_order_checkout i get this

You have just selected: sylius_order_checkout
+--------------------+
| Configured States: |
+--------------------+
| cart               |
| addressed          |
| shipping_selected  |
| payment_skipped    |
| payment_selected   |
| completed          |
+--------------------+
+-----------------+-------------------+-------------------+
| Transition      | From(s)           | To                |
+-----------------+-------------------+-------------------+
| address         | cart              | addressed         |
|                 | addressed         |                   |
|                 | shipping_selected |                   |
|                 | payment_selected  |                   |
|                 | payment_skipped   |                   |
+-----------------+-------------------+-------------------+
| select_shipping | addressed         | shipping_selected |
|                 | shipping_selected |                   |
|                 | payment_selected  |                   |
|                 | payment_skipped   |                   |
+-----------------+-------------------+-------------------+
| skip_payment    | shipping_selected | payment_skipped   |
+-----------------+-------------------+-------------------+
| select_payment  | payment_selected  | payment_selected  |
|                 | shipping_selected |                   |
+-----------------+-------------------+-------------------+
| complete        | payment_selected  | completed         |
|                 | payment_skipped   |                   |
+-----------------+-------------------+-------------------+

Is this the expected behaviour? Thanks.

@AndreasA
Copy link

AndreasA commented May 10, 2019

Disabling states works for me this way. However, for some reason the first step (also Sylius) cart always seems to be gone too.
Furthermore, why not just stateName: false or something like this instead of ::disabled ?

I think the issue is this line:
https://github.com/winzou/StateMachineBundle/blob/master/DependencyInjection/winzouStateMachineExtension.php#L71

As far as I know array_search returns FALSE and not NULL if they entry was not found. Tbh. I don't even understand what this line is supposed to do?

@diimpp
Copy link

diimpp commented May 27, 2019

How does one overrides whole states tree in Symfony4, where bundle configuration override was removed? Is it not possible anymore?

@AndreasA
Copy link

AndreasA commented May 27, 2019

Well, You can still extend it using packages which basically allows one to disable existing transitions / states like mentioned above in this thread.

Of course, it can be done using a CompilerPass or using PrependExtensionInterface

However, according to Sylius documentation it should work by placing the file in src/Resources like src/Resources/SyliusCoreBundle/config/app/state_machine/sylius_order_checkout.yml.

See: https://docs.sylius.com/en/1.4/cookbook/shop/checkout.html

It could be that this is Sylius specific though, I haven't tried it with just Symfony4.

However, I think it is the change to the application's kernel here that ensures this works:
https://github.com/Sylius/Sylius/blob/1.4/src/Kernel.php#L93

However, I think it could also work by using the build method of the kernel or configureContainer instead of overriding the ContainerLoader and explicitly targeting the necessary files.

@diimpp
Copy link

diimpp commented May 27, 2019

@AndreasA thank you, Andreas. It's good to know it still works.

@AndreasA
Copy link

Yes. But as mentioned, if you are not using Sylius, you might have to make some changes but I guess it should just be (as mentioned above) this overridden method: https://github.com/Sylius/Sylius/blob/1.4/src/Kernel.php#L93

It could probably be done differently too but as long as it works :)

@Geolim4
Copy link

Geolim4 commented Jun 7, 2022

Hello, any news on this issue ?
I'm forced to re-declare state-machine by myself to handle customs transitions from/to.

@chipsmaster
Copy link

Hi, my workaround is to override winzou\Bundle\StateMachineBundle\DependencyInjection\Configuration class by composer autloader and add ->performNoDeepMerging() to the "from" node so that we can overwrite it completely :

        $configNode
            ->arrayNode('transitions')
                ->useAttributeAsKey('name')
                ->prototype('array')
                    ->children()
                        ->arrayNode('from')
                            ->performNoDeepMerging()
                            ->prototype('scalar')->end()
                        ->end()
                        ->scalarNode('to')->end()
                    ->end()
                ->end()
            ->end()
        ;

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

No branches or pull requests

7 participants