Skip to content

[GraphQl] Single mutation for adding different types of products to the shopping cart #27914

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
b9e28fd
Single mutation for adding items to the shopping cart
rogyar Apr 13, 2020
13d0942
Review issues fixes
rogyar May 13, 2020
a7ded07
Merge branch '2.4-develop' into 257-single-mutation-add-to-cart
rogyar May 13, 2020
aa5bd2b
Updated composer.lock
rogyar May 13, 2020
05bb308
Merge branch '2.4-develop' into 257-single-mutation-add-to-cart
rogyar May 16, 2020
3da33f5
Updated composer.json files of the modules
rogyar May 16, 2020
87407fa
Added PHPDoc description for the class
rogyar May 19, 2020
b391029
Merge branch '2.4-develop' into 257-single-mutation-add-to-cart
rogyar May 27, 2020
75f531d
Unnecessary interface has been removed
rogyar Jun 1, 2020
62d4146
Use CamelCase for mutation parameters
rogyar Jun 10, 2020
2315faf
Merge branch '2.4-develop' into 257-single-mutation-add-to-cart
rogyar Jul 6, 2020
4c08148
Resolving merge conflicts
rogyar Jul 6, 2020
040663f
Merge branch '2.4-develop' into 257-single-mutation-add-to-cart
dthampy Jul 17, 2020
f5f6b59
Fixed error handling issues
rogyar Jul 19, 2020
52fb7a7
Error handling performance improvements
rogyar Jul 20, 2020
85fd270
Fixing the case when an error message appears twice
rogyar Jul 21, 2020
01e6312
Fixing existing API-functional tests
rogyar Jul 25, 2020
11e23d6
Tests enhancements. Added test case for simple product with customiza…
rogyar Jul 25, 2020
1521ac2
API-functional test for adding product with wrong SKU
rogyar Jul 26, 2020
9be928c
API-functional tests for wrong SKU and wrong quantity
rogyar Jul 26, 2020
bf83c7f
Added end-to-end test for adding product with customizable options to…
rogyar Jul 29, 2020
354fcbd
Added test case for adding available QTY + 1
rogyar Jul 29, 2020
8cbce5c
Minor fixes for tests
rogyar Jul 30, 2020
b26a7e5
Rename ID_V2 to UID
rogyar Jul 30, 2020
633ef00
added support for adding bundle options with custom quantity
Jul 30, 2020
38504fd
minor fix on tests
Jul 31, 2020
394dd7e
minor fix
Jul 31, 2020
06e3c7b
Save quote if there are no errors only
rogyar Jul 31, 2020
0b75992
Merge remote-tracking branch 'mainline/2.4-develop' into 257-single-m…
Jul 31, 2020
c24c12b
Revert quote items in case of an error
rogyar Aug 2, 2020
089ef94
Merge remote-tracking branch 'fork/257-single-mutation-add-to-cart' i…
rogyar Aug 2, 2020
b46af1c
Merge remote-tracking branch 'mainline/2.4-develop' into 257-single-m…
Aug 4, 2020
f91b685
static test fixes
Aug 4, 2020
ec99fef
Adjusted assertion message
rogyar Aug 4, 2020
330e763
- Removed dependency between QuoteGQL and WishlistGQL (https://github…
Aug 4, 2020
f6cfcff
- Updated schema descriptions
Aug 4, 2020
b1159bf
- Fixed/ Removed dependencies in schema between QuoteGQL, SalesGQL an…
Aug 4, 2020
8b925df
- reverting changes in Sales GraphQL
Aug 5, 2020
4c5403d
- removing path from UserInputErrors since its not in scope and curre…
Aug 5, 2020
84ceb30
- minor fix
Aug 5, 2020
5f86828
- fix: field names should not be camel case
Aug 5, 2020
762a3c1
- reverting .htaccess deletion
Aug 5, 2020
6f62556
Merge branch '2.4-develop' into 257-single-mutation-add-to-cart
Aug 6, 2020
473b8d8
- Fixed bug to save a product if another product in cart has an error
Aug 6, 2020
2518eb1
Merge branch '257-single-mutation-add-to-cart' of github.com:rogyar/m…
Aug 6, 2020
4391235
- minor fix
Aug 6, 2020
a556345
minor fix
Aug 11, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ public function resolve(
array $value = null,
array $args = null
) {
if (isset($value['uid'])) {
return $value['uid'];
}
if (!isset($value['option_id']) || empty($value['option_id'])) {
throw new GraphQlInputException(__('"option_id" value should be specified.'));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ public function resolve(
array $value = null,
array $args = null
) {
if (isset($value['uid'])) {
return $value['uid'];
}
if (!isset($value['option_id']) || empty($value['option_id'])) {
throw new GraphQlInputException(__('"option_id" value should be specified.'));
}
Expand Down
5 changes: 5 additions & 0 deletions app/code/Magento/GraphQl/etc/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,8 @@ enum CurrencyEnum @doc(description: "The list of available currency codes") {
TRL
XPF
}

input EnteredOptionInput @doc(description: "Defines a customer-entered option") {
uid: ID! @doc(description: "An encoded ID")
value: String! @doc(description: "Text the customer entered")
}
225 changes: 225 additions & 0 deletions app/code/Magento/Quote/Model/Cart/AddProductsToCart.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Quote\Model\Cart;

use Magento\Catalog\Api\ProductRepositoryInterface;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Quote\Api\CartRepositoryInterface;
use Magento\Quote\Api\Data\CartInterface;
use Magento\Quote\Model\Cart\BuyRequest\BuyRequestBuilder;
use Magento\Quote\Model\Cart\Data\AddProductsToCartOutput;
use Magento\Quote\Model\MaskedQuoteIdToQuoteIdInterface;
use Magento\Quote\Model\Quote;
use Magento\Framework\Message\MessageInterface;

/**
* Unified approach to add products to the Shopping Cart.
* Client code must validate, that customer is eligible to call service with provided {cartId} and {cartItems}
*/
class AddProductsToCart
{
/**#@+
* Error message codes
*/
private const ERROR_PRODUCT_NOT_FOUND = 'PRODUCT_NOT_FOUND';
private const ERROR_INSUFFICIENT_STOCK = 'INSUFFICIENT_STOCK';
private const ERROR_NOT_SALABLE = 'NOT_SALABLE';
private const ERROR_UNDEFINED = 'UNDEFINED';
/**#@-*/

/**
* List of error messages and codes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add a comment that this is an ad-hoc solution to get an error code related to some message. And maybe we can introduce a new class to provide error code based on error message to avoid logic duplicate: \Magento\Sales\Model\Reorder\Reorder::getErrorCode

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree. I was thinking about that but then came up with the conclusion that we will need this new class on the framework level (since we will need to reuse it in different independent modules). As long as we plan to deliver this PR as a part of 2.4.0 release, I don't want to introduce new interfaces or implementations on the framework level. So, let's leave a comment for now, and then, once this PR is delivered, I will introduce a new class and refactor the existing logic.

Thank you.

*/
private const MESSAGE_CODES = [
'Could not find a product with SKU' => self::ERROR_PRODUCT_NOT_FOUND,
'The required options you selected are not available' => self::ERROR_NOT_SALABLE,
'Product that you are trying to add is not available.' => self::ERROR_NOT_SALABLE,
'This product is out of stock' => self::ERROR_INSUFFICIENT_STOCK,
'There are no source items' => self::ERROR_NOT_SALABLE,
'The fewest you may purchase is' => self::ERROR_INSUFFICIENT_STOCK,
'The most you may purchase is' => self::ERROR_INSUFFICIENT_STOCK,
'The requested qty is not available' => self::ERROR_INSUFFICIENT_STOCK,
];

/**
* @var ProductRepositoryInterface
*/
private $productRepository;

/**
* @var array
*/
private $errors = [];

/**
* @var CartRepositoryInterface
*/
private $cartRepository;

/**
* @var MaskedQuoteIdToQuoteIdInterface
*/
private $maskedQuoteIdToQuoteId;

/**
* @var BuyRequestBuilder
*/
private $requestBuilder;

/**
* @param ProductRepositoryInterface $productRepository
* @param CartRepositoryInterface $cartRepository
* @param MaskedQuoteIdToQuoteIdInterface $maskedQuoteIdToQuoteId
* @param BuyRequestBuilder $requestBuilder
*/
public function __construct(
ProductRepositoryInterface $productRepository,
CartRepositoryInterface $cartRepository,
MaskedQuoteIdToQuoteIdInterface $maskedQuoteIdToQuoteId,
BuyRequestBuilder $requestBuilder
) {
$this->productRepository = $productRepository;
$this->cartRepository = $cartRepository;
$this->maskedQuoteIdToQuoteId = $maskedQuoteIdToQuoteId;
$this->requestBuilder = $requestBuilder;
}

/**
* Add cart items to the cart
*
* @param string $maskedCartId
* @param Data\CartItem[] $cartItems
* @return AddProductsToCartOutput
* @throws NoSuchEntityException Could not find a Cart with provided $maskedCartId
*/
public function execute(string $maskedCartId, array $cartItems): AddProductsToCartOutput
{
$cartId = $this->maskedQuoteIdToQuoteId->execute($maskedCartId);
$cart = $this->cartRepository->get($cartId);

foreach ($cartItems as $cartItemPosition => $cartItem) {
$this->addItemToCart($cart, $cartItem, $cartItemPosition);
}

if ($cart->getData('has_error')) {
$errors = $cart->getErrors();

/** @var MessageInterface $error */
foreach ($errors as $error) {
$this->addError($error->getText());
}
}

if (count($this->errors) !== 0) {
/* Revert changes introduced by add to cart processes in case of an error */
$cart->getItemsCollection()->clear();
}

return $this->prepareErrorOutput($cart);
}

/**
* Adds a particular item to the shopping cart
*
* @param CartInterface|Quote $cart
* @param Data\CartItem $cartItem
* @param int $cartItemPosition
*/
private function addItemToCart(CartInterface $cart, Data\CartItem $cartItem, int $cartItemPosition): void
{
$sku = $cartItem->getSku();

if ($cartItem->getQuantity() <= 0) {
$this->addError(__('The product quantity should be greater than 0')->render());

return;
}

try {
$product = $this->productRepository->get($sku, false, null, true);
} catch (NoSuchEntityException $e) {
$this->addError(
__('Could not find a product with SKU "%sku"', ['sku' => $sku])->render(),
$cartItemPosition
);

return;
}

try {
$result = $cart->addProduct($product, $this->requestBuilder->build($cartItem));
$this->cartRepository->save($cart);
} catch (\Throwable $e) {
$this->addError(
__($e->getMessage())->render(),
$cartItemPosition
);
$cart->setHasError(false);

return;
}

if (is_string($result)) {
$errors = array_unique(explode("\n", $result));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For complex errors we should be able utilize composite GraphQlInputException. Please see example \Magento\QuoteGraphQl\Model\Cart\AddSimpleProductToCart::execute.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lenaorobei. Thank you for your suggestion. In case of GraphQl modules, this approach will work. However, in this particular case, we work with the Quote module that has no dependency to GraphQl modules.
We use a similar approach in the Reorder functionality

if (is_string($addProductResult)) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lenaorobei this ugly approach comes from \Magento\Quote\Model\Quote::addProduct and due to we need to provide error message(s) we need to "parse" it in that way, as was "designed" in Quote::addProduct

       /**
         * Error message
         */
        if (is_string($cartCandidates) || $cartCandidates instanceof \Magento\Framework\Phrase) {
            return (string)$cartCandidates;
        }

foreach ($errors as $error) {
$this->addError(__($error)->render(), $cartItemPosition);
}
}
}

/**
* Add order line item error
*
* @param string $message
* @param int $cartItemPosition
* @return void
*/
private function addError(string $message, int $cartItemPosition = 0): void
{
$this->errors[] = new Data\Error(
$message,
$this->getErrorCode($message),
$cartItemPosition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I found a related issue. when we keep adding products to the cart, and if we run into errors like out of stock, the cart becomes empty.
Jul-10-2020 00-15-10

Copy link
Contributor Author

@rogyar rogyar Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good catch. The problem is it's a default system behavior introduced to the core a couple of months ago.

$this->deleteItem($item);

I'm not sure about the best way to overcome it. @prabhuram93 maybe you have ideas, thank you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rogyar. This took me a while to figure it out. Apparently this is happening only on the new mutation, not the existing ones.

Even though the cart item is being deleted in core, the cart is not saved. But I see in this PR the cart is being saved here even when there are errors. app/code/Magento/Quote/Model/Cart/AddProductsToCart.php:116. I don't see any to reason to do that, please correct me if I am wrong.

So fixing that should fix this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @prabhuram93. That's a good point. But unfortunately, without explicit cart saving on Model/Cart/AddProductsToCart.php:116 the quote is not being saved anywhere. You will still get the correct cart output since the corresponding model with the current changes will be used. But you will not get the record saved in the database.

You may test it in the following way:

  • Set stock qty of some product (let's say "simple1") to 10
  • Comment out the following line Model/Cart/AddProductsToCart.php:116
  • Add "simple1" to the shopping cart with QTY 3
  • Add "simple1" using the same mutation (QTY 3)
  • Check the result, you will have total QTY 3 (not 6) because the result of the very first mutation is not being saved anywhere.

So I'm still thinking that the approach of removing item if it has an error

$this->deleteItem($item);

Does the dirty thing.
I'm going to spend more time to overcome this weird behavior but any other idea will be very appreciated.

Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rogyar. I understand that cart is not being saved anywhere else. But I don't see any reason to save the cart when there are errors. So in the scenario you mentioned I wouldn't just comment out the line in step 2, rather we can handle it to save the cart when only there are no errors. That way when there are errors just the error objects will be updated for that particular mutation and the cart will not be saved. When there are no errors cart will be saved. I managed to try it out and it works for both the cases you mentioned. Let me know if this is still unclear. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thank you. We may adjust it. The only thing that will be lost in this case, described in the following scenario.

  • Add 3 different items to the cart
  • There's no issue with first 2 but 3rd is problematic
  • We will have no items in the cart instead of having 2 of them

... but, it's another story :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rogyar, I don't see a use case where we add 3 different items to the cart at the same point. We might do it one after the other, but not together. So we are good on that front.

);
}

/**
* Get message error code.
*
* TODO: introduce a separate class for getting error code from a message
*
* @param string $message
* @return string
*/
private function getErrorCode(string $message): string
{
foreach (self::MESSAGE_CODES as $codeMessage => $code) {
if (false !== stripos($message, $codeMessage)) {
return $code;
}
}

/* If no code was matched, return the default one */
return self::ERROR_UNDEFINED;
}

/**
* Creates a new output from existing errors
*
* @param CartInterface $cart
* @return AddProductsToCartOutput
*/
private function prepareErrorOutput(CartInterface $cart): AddProductsToCartOutput
{
$output = new AddProductsToCartOutput($cart, $this->errors);
$this->errors = [];
$cart->setHasError(false);

return $output;
}
}
61 changes: 61 additions & 0 deletions app/code/Magento/Quote/Model/Cart/BuyRequest/BuyRequestBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Quote\Model\Cart\BuyRequest;

use Magento\Framework\DataObject;
use Magento\Framework\DataObjectFactory;
use Magento\Quote\Model\Cart\Data\CartItem;

/**
* Build buy request for adding products to cart
*/
class BuyRequestBuilder
{
/**
* @var BuyRequestDataProviderInterface[]
*/
private $providers;

/**
* @var DataObjectFactory
*/
private $dataObjectFactory;

/**
* @param DataObjectFactory $dataObjectFactory
* @param array $providers
*/
public function __construct(
DataObjectFactory $dataObjectFactory,
array $providers = []
) {
$this->dataObjectFactory = $dataObjectFactory;
$this->providers = $providers;
}

/**
* Build buy request for adding product to cart
*
* @see \Magento\Quote\Model\Quote::addProduct
* @param CartItem $cartItem
* @return DataObject
*/
public function build(CartItem $cartItem): DataObject
{
$requestData = [
['qty' => $cartItem->getQuantity()]
];

/** @var BuyRequestDataProviderInterface $provider */
foreach ($this->providers as $provider) {
$requestData[] = $provider->execute($cartItem);
}

return $this->dataObjectFactory->create(['data' => array_merge(...$requestData)]);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Quote\Model\Cart\BuyRequest;

use Magento\Quote\Model\Cart\Data\CartItem;

/**
* Provides data for buy request for different types of products
*/
interface BuyRequestDataProviderInterface
{
/**
* Provide buy request data from add to cart item request
*
* @param CartItem $cartItem
* @return array
*/
public function execute(CartItem $cartItem): array;
}
Loading