Why don't perform availability check when add item to cart

[kayue]

Right now PutVariantBasedConfigutableItemToCartRequest only perform very simple check

May I know is it intentional to leave it availability checking out?

[mamazu]

Good question.

[kayue]

Actually there is no inventory check throughout the entire API checkout.

[mamazu]

At least in the checkout complete step there should be some kind of validation. Is it not caught by the Sylius internal logic that tries to reserve the items?

[kayue]

Nope that didn't happen.

[kayue]

Just noticed this duplicated with #509

[gomcodoctor]

Any ETA for this bug fix?

[mamazu]

If it is as easy as it seems then it is just adding a validation on some requests. But I want to get @lchrusciel opinion on it as I am not aware of all the ins and outs of Sylius and its stock tracking.

[gomcodoctor]

I already added validation on change quantity request, it working fine.

issue with adding new product to cart request validation - we can add validation like above but it will need EXTRA database query for searching products, I personally feel its not good idea, we should add validation on command to save DB query.

kindly give your suggestions, i may be wrong

[mamazu]

I haven't considered this but I know that there is a validator for availability what about this one?

[gomcodoctor]

We have to implement this

https://github.com/Sylius/Sylius/blob/a70a38983770583c706991abd3328f2b77e7c42e/src/Sylius/Bundle/CoreBundle/Validator/Constraints/CartItemAvailabilityValidator.php

this is request which is currently validated

https://github.com/Sylius/ShopApiPlugin/blob/master/src/Request/Cart/PutVariantBasedConfigurableItemToCartRequest.php

this is handler

https://github.com/Sylius/ShopApiPlugin/blob/master/src/Handler/Cart/PutVariantBasedConfigurableItemToCartHandler.php

[mamazu]

Sounds good. Maybe we can also reuse the validator.

[gomcodoctor]

ya we can reuse validator,

i think its better to add validator directly in

https://github.com/Sylius/ShopApiPlugin/blob/master/src/Handler/Cart/PutVariantBasedConfigurableItemToCartHandler.php

this is will save double queries

[mamazu]

Well the usual behaviour is to double validate it. Once for the view layer so that the Controller can return a 400 when the product is not in stock and once in the actual logic, which doesn't need to be tied to the shop api using it.

[lchrusciel]

As @mamazu said. Validation on request to show proper info + on command level an exception.

It should be a part of 1.1

[alexander-schranz]

would #648 be the correct way to fix this issue?

[diimpp]

No, it should be implemented as validator and return validation error instead of 500 critical error.

Case: Last item is bought by someone else within browsing session of current customer. He sees outdated catalog and tries to add out of stock item to cart and sees 500 error instead of 400 error

[alexander-schranz]

@diimpp Ok if I look at ProductInCartChannel it seems there exist both a Validator:

ShopApiPlugin/src/Resources/config/validation/cart/PutSimpleItemToCartRequest.xml

Line 16 in fc87069

<constraint name="Sylius\ShopApiPlugin\Validator\Constraints\ProductInCartChannel" />

and a check in the Handler:

ShopApiPlugin/src/Handler/Cart/PutSimpleItemToCartHandler.php

Line 52 in d5c273a

Assert::true($this->channelChecker->isProductInCartChannel($product, $cart), 'Product is not in same channel as cart');

Think then we should do also both in this cases or?

[diimpp]

@alexander-schranz Yes, that would be correct to put check both in handler and in validator. Sorry, I've misread originally, thought it was in Action itself.