After adding the bundle, I am not able to add products with a customer option to the cart

[shochdoerfer]

I installed the bundle as outlined in the Readme file in my Sylius 1.12 instance. I've configured a required customer option of type text, added the customer option group, and assigned the customer option group to one product. The product itself is a simple product with no variants. Even adding variants did not change anything.

When trying to add the product to the cart (providing a proper input value for the customer option), Sylius fails with the following error:

Sylius\Bundle\OrderBundle\Controller\OrderItemController::resolveAddedOrderItem(): Return value must be of type Sylius\Component\Order\Model\OrderItemInterface, bool returned

At first, I thought it was an incompatibility with the Sylius version I am using. I debugged things in depth and found the problem to be the comparison in OrderItemCustomerOptionCapableTrait::equals() The check fails because the call $product->hasCustomerOptions() is true and is returned negated.

Am I missing some configuration?

[mamazu]

Thanks for reporting the problem. This is indeed an issue that has been introduced by a more recent version of Sylius.

What's the problem

Well the problem is that the resolveAddedOrderItem method from Sylius expects that if you add an item to the order that you can find it again. For example: You add a red t-shirt to the cart then going through the list and filtering the list with only red T-Shirt items should return back an array of one item which Sylius returns. However in this case it doesn't because as you correctly analysed the equals method never returns true in this array.

Why do we need to change the equals method in this plugin?

The reason for that is pretty simple. By default two order items are equal if they have the same product which makes sense. If you add a red t-shirt to a red t-shirt then you have two of the same object and Sylius rightfully combines them into 2 x red shirt. However if you bring in customer option then it could be that if one shirt has some writing on it and the other doesn't then they should not be summarized. So the logic was basically: if you have a product that has customer options, they aren't equal.

return ($product instanceof ProductInterface) ? !$product->hasCustomerOptions() : true;

This line above is pretty dense. But what it means is if it's not a ProductInterface from our plugin return true (the bit at the end). And if it is then the return value is !$product->hasCustomerOptions() or in plain English: the product hasn't got customer options. Because as soon as it has customer options they're not equal.

What to do?

The reason why we went for this approach (just saying not equals when the product can have customer options) is that it is the safest way to not loose them to accidental merging. But since that doesn't work anymore we need to come up with a different strategy.

If you have the time it would be great if you could create a pull request with the logic that adds comparing customer option values (which should solve the problem). Or if you are looking for a quick fix just override the equals method from the bundle in your project.

[shochdoerfer]

@mamazu, thanks for the fast response and the in-depth details. That's more or less what I also thought. Also, your conclusion is the same that I had yesterday: The logic needs to compare each and every customer option value.

For my very specific use case, I can easily rely on the default Sylius logic. Let me see if I can find some time in the next weeks to come up with a PR.

[shochdoerfer]

This is a bit more challenging than expected ;) The core issue - as I see it now - is that the main logic is executed in the PostEvent hook, which is too late to have all the needed data available in the equals() method to decide which items are equal.

[mamazu]

What exactly do you mean. Does the order item not yet have the customer option values the customer has entered?

[t-n-y]

I tried this plugin with sylius 1.12 and i got code: 400, message: "Input value "sylius_add_to_cart" contains a non-scalar value." when i add product to cart. Is it the same issue ? Do you have any idea when this bug could be fixed ?

[shochdoerfer]

What exactly do you mean. Does the order item not yet have the customer option values the customer has entered?

Correct, at least that is my current observation. While debugging I was missing the information for the product that is added to the cart. Before your logic is running in the PostEvent several equals() calls are happing that miss this information. I tried to convert the PostEvent into a PreEvent but for some reason that did not fire correctly. Need to dig a bit deeper, I think.

[shochdoerfer]

@mamazu I am slowly making progress. My initial plan of using the PreAdd event was not successful. The event gets triggered but also somehow too late. So I changed my plan and implemented a custom CartItemFactory, which replaces the Sylius default implementation and contains the logic from your AddToCartListener. That seems to work, I now have all the information I need available in the equals() method to do a proper comparison of the configured items.

One minor problem popped up: Doctrine complains about a "not configured to cascade persist operation". Not entirely sure why, it looks like the cascade persist configuration are in place. But maybe I am missing something.

[shochdoerfer]

@mamazu good news! I managed to get a working prototype ready. I'll convert that into a PR and then you can have a look if it makes sense. Will take a few more days since I'll be traveling next week.

[shochdoerfer]

@t-n-y I don't think your issue is similar to mine. At least the error looks different.

[t-n-y]

@shochdoerfer Ok, i ll test with your pr when it s ready and i ll see. thanks for the answer

[shochdoerfer]

@mamazu @t-n-y opened a PR with my changes, tests need to be adapted, just wanted to get general approval first before investing more time on this. Worked fine in my test environment.

[t-n-y]

@shochdoerfer i tried your fork, but when i install it, i got following error : The service "sylius.factory.cart_item" has a dependency on a non-existent service "sylius.custom_factory.order_item.inner"

[adamterepora]

@t-n-y it's enough to replace $addToCart = $request->request->get('sylius_add_to_cart'); with $addToCart = $request->request->all('sylius_add_to_cart'); in ConditionalConstraintValidator.php::getCustomerOptionsFromRequest method. Your problem should be solved.

[t-n-y]

@t-n-y it's enough to replace $addToCart = $request->request->get('sylius_add_to_cart'); with $addToCart = $request->request->all('sylius_add_to_cart'); in ConditionalConstraintValidator.php::getCustomerOptionsFromRequest method. Your problem should be solved.

Thx, do you mean on the fork or the last release ?

[adamterepora]

Ok, I think i figured this out although it took a bit more effort to make this work. I realize it can be done better but it works. Steps to make this work:

1. Override OrderItemController and assign OrderItemOptions to $orderItem (based on "sylius_add_to_cart" form) before form submitting. We have to have those options before checking whether order items are equal.

I made something like this:

...
$form = $this->getFormFactory()->create(
            $configuration->getFormType(),
            $this->createAddToCartCommand($cart, $orderItem),
            $configuration->getFormOptions(),
        );

        // assign customer options if exist
+        $orderItem = $this->assignCustomerOptions($configuration, $orderItem);

        if ($request->isMethod('POST') && $form->handleRequest($request)->isSubmitted() && $form->isValid()) {
....

----
protected function assignCustomerOptions(
        RequestConfiguration $configuration,
        BrilleOrderItemInterface $orderItem
    ) {
        /** @var OrderItemOptionFactory $orderItemOptionFactory */
        $orderItemOptionFactory = $this->get('brille24.factory.order_item_option');

        // would be nice to get those values in different way...
        $params = $configuration->getRequest()->request->all('sylius_add_to_cart');
        if (array_key_exists('customer_options', $params)) {
            $configArr = [];
            foreach ($params['customer_options'] as $optionKey => $optionValue) {
                $orderItemOption = $orderItemOptionFactory->createNewFromStrings($orderItem, $optionKey, $optionValue);
                $configArr[$optionKey] = $orderItemOption;
            }
            $orderItem->setCustomerOptionConfiguration(array_values($configArr));
        }
        return $orderItem;
    }

3. Override OrderItemOptionFactory and inject currently used channel (before form submitting we don't have order related with orderItem, so we need to be sure that factory contains channel. More or less something like this:

services.yaml

    brille24.customer_options_plugin.factory.order_item_option_factory:
        class: App\Factory\OrderItemOptionFactory
        decorates: 'brille24.factory.order_item_option'
        arguments:
            - '@brille24.customer_options_plugin.factory.order_item_option_factory.inner'
            - '@brille24.repository.customer_option'
            - '@brille24.customer_options_plugin.services.customer_option_value_resolver'
            - '@brille24.repository.customer_option_value_price'
            - '@sylius.context.channel'

App\Factory\OrderItemOptionFactory.php

...
$contextChannel = $this->channelContext->getChannel();
/** @var ChannelInterface $channel */
$channel = $order ? $order->getChannel() : $contextChannel;
...

4. OrderItemCustomerOptionCapableTrait.php

   
   public function equals(SyliusOrderItemInterface $item): bool
   {
       // If the product doesn't match for the Sylius implementation then it's not the same.
       if (!parent::equals($item)) {
           return false;
       }
   
       $product = $item instanceof self ? $item->getProduct() : null;
       $hasOpts = $product->hasCustomerOptions();
       if ($product instanceof ProductInterface && !$hasOpts) return true;
   
       // compare passed customer option configurations (its option codes and option value codes)
       // if any of checked values differs, then it's different product
   
       $conf = $this->getCustomerOptionConfigurationAsSimpleArray();
       $conf2 = $item->getCustomerOptionConfigurationAsSimpleArray();
   
       if ($conf && !$conf2) return false;
   
       // maybe we should also compare how many items contains each conf???
       foreach ($conf2 as $optionCode => $optionValueCode) {
           if (!array_key_exists($optionCode, $conf)) return false;
           if (!in_array($optionValueCode, $conf)) return false;
       }
   
       return true;
   }

[shochdoerfer]

May bad, I completely lost track of this issue. Will have some time in the next weeks to optimize/finish the PR I started a while ago.

[t-n-y]

@shochdoerfer did you had time to look at that issue ? also, would it be possible to make the next release compatible with doctrine/dbal ^3.0 ?

[shochdoerfer]

@t-n-y a few minor things to fix plus getting the tests back in shape. Technically, the code in my branch should work now if you need a solution quickly.

[t-n-y]

Awesome, thank you