Skip to content

Product name not shown in message when trying to add more products to cart then there are in stock #8660

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

Closed
wants to merge 1 commit into from

Conversation

basselalaraaj
Copy link

@basselalaraaj basselalaraaj commented Feb 23, 2017

Preconditions
Magento 2.1.x
Steps to reproduce

Set the stock of a product to 20.
Add the same product to the cart with qty greater then 20.
Expected result

A message saying 'We don't have as many "<product_name>" as you requested'.
Actual result

5cefdb6a-f369-11e6-92bb-6debb34ff2aa

Fixes #8560

@basselalaraaj basselalaraaj changed the title Fixes #8560 Product name not shown in message when trying to add more products to cart then there are in stock Feb 23, 2017
@@ -106,6 +106,10 @@ public function checkQuoteItemQty(StockItemInterface $stockItem, $qty, $summaryQ
{
$result = $this->objectFactory->create();
$result->setHasError(false);

$product = $this->productFactory->create();
$product->load($stockItem->getProductId());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use ProductRepository here? Is product name always missing or only in some cases?

Copy link
Author

Choose a reason for hiding this comment

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

@orlangur the name is always missing, because there is no product available.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you need to load Product you have to rely on ProductRepostioryInterface::get method.
loading product using the load method of Product Model you are using deprecated method of Abstract Model https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/Model/AbstractModel.php#L526


$product = $this->productFactory->create();
$product->load($stockItem->getProductId());
$stockItem->setProduct($product);
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, there is unit test failure:

Fatal error: Call to undefined method Mock_StockItemInterface_837dff1c::setProduct() in /home/travis/build/magento/magento2/app/code/Magento/CatalogInventory/Model/StockStateProvider.php on line 112

Is there a way to achieve the same using only defined interfaces?

Copy link
Author

Choose a reason for hiding this comment

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

@orlangur
This is weird, because the interface already loaded Magento\CatalogInventory\Model\Stock\Item
And there is a public function setProduct available.
In my system it is working fine, no errors.

Copy link
Contributor

@orlangur orlangur Feb 23, 2017

Choose a reason for hiding this comment

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

I do believe that the code works ;)

The problem is that this method is not part of StockItemInterface.

Lame way is just add setProduct to mock in failed test. The downside is that the code we have do not follow contract defined by StockItemInterface.

Better way is achieve the same correct behavior but use only defined contracts and do not rely on existing implementation. Why wrong product instance appear in StockItem at all? Of course, it may appear, that correct fix require much more efforts in such case breaking contract with setProduct call may be acceptable at it is lesser evil than having bug.

Copy link
Contributor

Choose a reason for hiding this comment

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

as your method accepts StockItemInterface as parameter, you should rely just on the contract provided by this interface https://github.com/magento/magento2/blob/develop/app/code/Magento/CatalogInventory/Api/Data/StockItemInterface.php
setProduct does not belong to StockItemInterface.
And couldn't belong, because StockItemInterface represents a relation between Stock and Product. You can consider it as a link entity, which holds the information about "what product" "in which quantity" stored on "what stock"

Copy link
Contributor

@maghamed maghamed left a comment

Choose a reason for hiding this comment

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

Please consider usage of Magento API implementing fix

@@ -106,6 +106,10 @@ public function checkQuoteItemQty(StockItemInterface $stockItem, $qty, $summaryQ
{
$result = $this->objectFactory->create();
$result->setHasError(false);

$product = $this->productFactory->create();
$product->load($stockItem->getProductId());
Copy link
Contributor

Choose a reason for hiding this comment

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

If you need to load Product you have to rely on ProductRepostioryInterface::get method.
loading product using the load method of Product Model you are using deprecated method of Abstract Model https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/Model/AbstractModel.php#L526


$product = $this->productFactory->create();
$product->load($stockItem->getProductId());
$stockItem->setProduct($product);
Copy link
Contributor

Choose a reason for hiding this comment

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

as your method accepts StockItemInterface as parameter, you should rely just on the contract provided by this interface https://github.com/magento/magento2/blob/develop/app/code/Magento/CatalogInventory/Api/Data/StockItemInterface.php
setProduct does not belong to StockItemInterface.
And couldn't belong, because StockItemInterface represents a relation between Stock and Product. You can consider it as a link entity, which holds the information about "what product" "in which quantity" stored on "what stock"

@okorshenko okorshenko added this to the March 2017 milestone Mar 1, 2017
@okorshenko okorshenko modified the milestones: March 2017, April 2017 Apr 2, 2017
@okorshenko okorshenko modified the milestones: April 2017, May 2017 May 9, 2017
@okorshenko okorshenko modified the milestones: May 2017, June 2017 Jun 1, 2017
@ishakhsuvarov
Copy link
Contributor

Hi @basselalaraaj
Do you want to continue work on this PR? Please update according to the comments if so.
Thank you

@ishakhsuvarov ishakhsuvarov self-assigned this Jun 8, 2017
@ishakhsuvarov
Copy link
Contributor

@basselalaraaj Closing this PR for now due to inactivity. Please reopen and check the review comments if you wish to continue work on it anytime.
Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants