Set minimum qty 1 after cast to int #21928

likemusic · 2019-03-25T19:56:44Z

Description (*)

Set minimum qty 1 for values casted to int from float value.

Fixed Issues (if relevant)

Exception on reorder from admin #21926: Exception on reorder from admin

Manual testing scenarios (*)

See Exception on reorder from admin #21926

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

magento-engcom-team · 2019-03-25T19:56:47Z

Hi @likemusic. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

dmytro-ch

Hi @likemusic,
thank you for your contribution!

Could you please check the minor CR recommendations?

Thank you!

app/code/Magento/CatalogInventory/Model/StockStateProvider.php

dmytro-ch · 2019-03-25T22:07:23Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-03-25T22:07:27Z

Hi @dmytro-ch. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-03-25T22:08:11Z

Hi @dmytro-ch, here is your new Magento instance.
Admin access: https://pr-21928.instances.magento-community.engineering/admin
Login: admin Password: 123123q

dmytro-ch · 2019-03-25T22:09:54Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-03-25T22:09:57Z

Hi @dmytro-ch. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-03-25T22:10:40Z

Hi @dmytro-ch, here is your new Magento instance.
Admin access: https://pr-21928.instances.magento-community.engineering/admin
Login: admin Password: 123123q

app/code/Magento/CatalogInventory/Model/StockStateProvider.php

dmytro-ch · 2019-03-26T11:31:38Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-03-26T11:31:42Z

Hi @dmytro-ch. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-03-26T11:32:34Z

Hi @dmytro-ch, here is your new Magento instance.
Admin access: https://pr-21928.instances.magento-community.engineering/admin
Login: admin Password: 123123q

magento-engcom-team · 2019-03-26T12:15:41Z

Hi @dmytro-ch, thank you for the review.
ENGCOM-4585 has been created to process this Pull Request

orlangur

@likemusic please simplify implementation even more and squash changes into a single commit.

orlangur · 2019-03-26T13:24:21Z

app/code/Magento/CatalogInventory/Model/StockStateProvider.php

@@ -119,13 +119,13 @@ public function checkQuoteItemQty(StockItemInterface $stockItem, $qty, $summaryQ
        $result->setItemIsQtyDecimal($stockItem->getIsQtyDecimal());
        if (!$stockItem->getIsQtyDecimal()) {
            $result->setHasQtyOptionUpdate(true);
-            $qty = (int) $qty;
+            $qty = (int) $qty ?: 1;
            /**
             * Adding stock data to quote item
             */
            $result->setItemQty($qty);


Suggested change

$result->setItemQty($qty);

$result->setItemQty((int)$qty ?: 1);

@orlangur $qty variable used below in file many times. I think it should contain already fixed value, isn't it?

@likemusic it is casted to int only here within condition. Please don't forget to squash.

@orlangur I think all other validation should by with casted value.
Assume $qty = 0.1 and $stockItem->getMinSaleQty() is 1. In $result we have 1.
Look at line 130 if ($stockItem->getMinSaleQty() && $qty < $stockItem->getMinSaleQty()) {

If here $qty == 1 will not add error message to $result. If $qty == 0.1 here, we will add error message to $result.
In latest case finaly we will have result with $qty = 1 but with massage that 'The fewest you may purchase is 1.'. It's not logical. Imho all validation with $qty variable in this method should be done with cased to int value.

@likemusic ok, agree, assignment should still be there and $result->setItemQty($qty = (int)$qty ?: 1); would look awkward.

I made:

git rebase -i 2.3-develop git push -f

Is it right way to squash commits in pull-request?

orlangur · 2019-03-26T13:25:15Z

app/code/Magento/CatalogInventory/Model/StockStateProvider.php

-            $qty = $this->getNumber($qty);
-            $origQty = (int) $origQty;
+            $origQty = $this->getNumber($origQty);
+            $origQty = (int) $origQty ?: 1;
            $result->setOrigQty($origQty);


Suggested change

$result->setOrigQty($origQty);

$result->setOrigQty((int)$this->getNumber($origQty) ?: 1);

orlangur

@likemusic great! This is exactly what was needed.

magento-engcom-team · 2019-03-26T15:55:38Z

Hi @orlangur, thank you for the review.
ENGCOM-4585 has been created to process this Pull Request

VasylShvorak · 2019-04-02T12:03:16Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-04-02T12:03:17Z

Hi @VasylShvorak. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-04-02T12:04:13Z

Hi @VasylShvorak, here is your new Magento instance.
Admin access: https://pr-21928.instances.magento-community.engineering/admin
Login: admin Password: 123123q

VasylShvorak · 2019-04-02T12:31:24Z

✔️ QA passed

m2-assistant · 2019-04-08T11:39:19Z

Hi @likemusic, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

meker12 · 2019-06-03T22:28:48Z

Release note"

Catalog

Magento no longer throws a divide by zero exception if a product configuration specifies the Minimum Qty Allowed in Shopping Cart as a decimal value less than one, and sets the Qty Uses Decimals attribute to No .

magento-engcom-team added Component: CatalogInventory Release Line: 2.3 labels Mar 25, 2019

dmytro-ch self-requested a review March 25, 2019 20:03

dmytro-ch self-assigned this Mar 25, 2019

dmytro-ch requested changes Mar 25, 2019

View reviewed changes

app/code/Magento/CatalogInventory/Model/StockStateProvider.php Outdated Show resolved Hide resolved

app/code/Magento/CatalogInventory/Model/StockStateProvider.php Outdated Show resolved Hide resolved

ghost added the Progress: needs update label Mar 25, 2019

dmytro-ch requested changes Mar 25, 2019

View reviewed changes

app/code/Magento/CatalogInventory/Model/StockStateProvider.php Outdated Show resolved Hide resolved

dmytro-ch approved these changes Mar 26, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Mar 26, 2019

orlangur requested changes Mar 26, 2019

View reviewed changes

ghost assigned orlangur Mar 26, 2019

ghost added Progress: needs update and removed Progress: ready for testing labels Mar 26, 2019

Set minimum qty 1 after cast to int

d073b0c

likemusic force-pushed the issue-21926 branch from f49b7fb to d073b0c Compare March 26, 2019 15:43

orlangur approved these changes Mar 26, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Mar 26, 2019

Fix encoding issue

b82503a

VasylShvorak self-assigned this Apr 2, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Apr 2, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Apr 2, 2019

p-bystritsky self-assigned this Apr 2, 2019

ghost added Progress: accept and removed Progress: extended testing labels Apr 5, 2019

magento-engcom-team added the Award: bug fix label Apr 5, 2019

magento-engcom-team mentioned this pull request Apr 8, 2019

Exception on reorder from admin #21926

Closed

magento-engcom-team merged commit b82503a into magento:2.3-develop Apr 8, 2019

magento-engcom-team pushed a commit that referenced this pull request Apr 8, 2019

ENGCOM-4585: Set minimum qty 1 after cast to int #21928

7198662

magento-engcom-team added this to the Release: 2.3.2 milestone Apr 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set minimum qty 1 after cast to int #21928

Set minimum qty 1 after cast to int #21928

likemusic commented Mar 25, 2019 •

edited

Loading

magento-engcom-team commented Mar 25, 2019

dmytro-ch left a comment

dmytro-ch commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

dmytro-ch commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

dmytro-ch commented Mar 26, 2019

magento-engcom-team commented Mar 26, 2019

magento-engcom-team commented Mar 26, 2019

magento-engcom-team commented Mar 26, 2019

orlangur left a comment

orlangur Mar 26, 2019 •

edited

Loading

likemusic Mar 26, 2019

orlangur Mar 26, 2019

likemusic Mar 26, 2019

orlangur Mar 26, 2019

likemusic Mar 26, 2019

orlangur Mar 26, 2019

likemusic Mar 26, 2019

orlangur left a comment

magento-engcom-team commented Mar 26, 2019

VasylShvorak commented Apr 2, 2019

magento-engcom-team commented Apr 2, 2019

magento-engcom-team commented Apr 2, 2019

VasylShvorak commented Apr 2, 2019

m2-assistant bot commented Apr 8, 2019

meker12 commented Jun 3, 2019

	$result->setItemQty($qty);
	$result->setItemQty((int)$qty ?: 1);

	$result->setOrigQty($origQty);
	$result->setOrigQty((int)$this->getNumber($origQty) ?: 1);

Set minimum qty 1 after cast to int #21928

Set minimum qty 1 after cast to int #21928

Conversation

likemusic commented Mar 25, 2019 • edited Loading

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

magento-engcom-team commented Mar 25, 2019

dmytro-ch left a comment

Choose a reason for hiding this comment

dmytro-ch commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

dmytro-ch commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

magento-engcom-team commented Mar 25, 2019

dmytro-ch commented Mar 26, 2019

magento-engcom-team commented Mar 26, 2019

magento-engcom-team commented Mar 26, 2019

magento-engcom-team commented Mar 26, 2019

orlangur left a comment

Choose a reason for hiding this comment

orlangur Mar 26, 2019 • edited Loading

Choose a reason for hiding this comment

likemusic Mar 26, 2019

Choose a reason for hiding this comment

orlangur Mar 26, 2019

Choose a reason for hiding this comment

likemusic Mar 26, 2019

Choose a reason for hiding this comment

orlangur Mar 26, 2019

Choose a reason for hiding this comment

likemusic Mar 26, 2019

Choose a reason for hiding this comment

orlangur Mar 26, 2019

Choose a reason for hiding this comment

likemusic Mar 26, 2019

Choose a reason for hiding this comment

orlangur left a comment

Choose a reason for hiding this comment

magento-engcom-team commented Mar 26, 2019

VasylShvorak commented Apr 2, 2019

magento-engcom-team commented Apr 2, 2019

magento-engcom-team commented Apr 2, 2019

VasylShvorak commented Apr 2, 2019

m2-assistant bot commented Apr 8, 2019

meker12 commented Jun 3, 2019

likemusic commented Mar 25, 2019 •

edited

Loading

orlangur Mar 26, 2019 •

edited

Loading