From 7a0534ad48dc33f6edb4f162d80ff12aa3759b30 Mon Sep 17 00:00:00 2001 From: Matias Montes Date: Sat, 24 Mar 2018 13:35:13 -0300 Subject: [PATCH 1/3] Fixed decimal handling in order quantities Added test coverage to getSimpleQtyToShip and getQtyToInvoice Fixes 14328 --- app/code/Magento/Sales/Model/Order/Item.php | 4 +- .../Sales/Test/Unit/Model/Order/ItemTest.php | 197 ++++++++++++++++++ 2 files changed, 199 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Sales/Model/Order/Item.php b/app/code/Magento/Sales/Model/Order/Item.php index da2a06c2db25a..d2f5f5ce56692 100644 --- a/app/code/Magento/Sales/Model/Order/Item.php +++ b/app/code/Magento/Sales/Model/Order/Item.php @@ -233,7 +233,7 @@ public function getQtyToShip() public function getSimpleQtyToShip() { $qty = $this->getQtyOrdered() - $this->getQtyShipped() - $this->getQtyRefunded() - $this->getQtyCanceled(); - return max($qty, 0); + return max(round($qty, 8), 0); } /** @@ -248,7 +248,7 @@ public function getQtyToInvoice() } $qty = $this->getQtyOrdered() - $this->getQtyInvoiced() - $this->getQtyCanceled(); - return max($qty, 0); + return max(round($qty, 8), 0); } /** diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php index 03a388410f335..bdfc365e25add 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php @@ -235,4 +235,201 @@ public function getProductOptionsDataProvider() ] ]; } + + /** + * Test different combinations of item qty setups + * + * @param array $options + * @param float $expectedResult + * + * @dataProvider getItemQtyVariants + */ + public function testGetSimpleQtyToShip(array $options, $expectedResult) + { + $this->model->setData($options); + $this->assertSame($this->model->getSimpleQtyToShip(), $expectedResult['to_ship']); + } + + /** + * Test different combinations of item qty setups + * + * @param array $options + * @param float $expectedResult + * + * @dataProvider getItemQtyVariants + */ + public function testGetQtyToInvoice(array $options, $expectedResult) + { + $this->model->setData($options); + $this->assertSame($this->model->getQtyToInvoice(), $expectedResult['to_invoice']); + } + + /** + * Provides different combinations of qty options for an item and the + * expected qtys pending shipment and invoice + * + * @return array + */ + public function getItemQtyVariants() + { + return [ + 'empty_item' => [ + 'options' => [ + 'qty_ordered' => 0, + 'qty_invoiced' => 0, + 'qty_refunded' => 0, + 'qty_shipped' => 0, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 0.0, + 'to_invoice' => 0.0 + ] + ], + 'ordered_item' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 0, + 'qty_refunded' => 0, + 'qty_shipped' => 0, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 12.0, + 'to_invoice' => 12.0 + ] + ], + 'partially_invoiced' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 4, + 'qty_refunded' => 0, + 'qty_shipped' => 0, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 12.0, + 'to_invoice' => 8.0 + ] + ], + 'completely_invoiced' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 12, + 'qty_refunded' => 0, + 'qty_shipped' => 0, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 12.0, + 'to_invoice' => 0.0 + ] + ], + 'partially_invoiced_refunded' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 5, + 'qty_refunded' => 5, + 'qty_shipped' => 0, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 7.0, + 'to_invoice' => 7.0 + ] + ], + 'partially_refunded' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 12, + 'qty_refunded' => 5, + 'qty_shipped' => 0, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 7.0, + 'to_invoice' => 0.0 + ] + ], + 'partially_shipped' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 0, + 'qty_refunded' => 0, + 'qty_shipped' => 4, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 8.0, + 'to_invoice' => 12.0 + ] + ], + 'partially_refunded_partially_shipped' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 12, + 'qty_refunded' => 5, + 'qty_shipped' => 4, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 3.0, + 'to_invoice' => 0.0 + ] + ], + 'complete' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 12, + 'qty_refunded' => 0, + 'qty_shipped' => 12, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 0.0, + 'to_invoice' => 0.0 + ] + ], + 'canceled' => [ + 'options' => [ + 'qty_ordered' => 12, + 'qty_invoiced' => 0, + 'qty_refunded' => 0, + 'qty_shipped' => 0, + 'qty_canceled' => 12, + ], + 'expectedResult' => [ + 'to_ship' => 0.0, + 'to_invoice' => 0.0 + ] + ], + 'completely_shipped_using_decimals' => [ + 'options' => [ + 'qty_ordered' => 4.4, + 'qty_invoiced' => 0.4, + 'qty_refunded' => 0.4, + 'qty_shipped' => 4, + 'qty_canceled' => 0, + ], + 'expectedResult' => [ + 'to_ship' => 0.0, + 'to_invoice' => 4.0 + ] + ], + 'completely_invoiced_using_decimals' => [ + 'options' => [ + 'qty_ordered' => 4.4, + 'qty_invoiced' => 4, + 'qty_refunded' => 0, + 'qty_shipped' => 4, + 'qty_canceled' => 0.4, + ], + 'expectedResult' => [ + 'to_ship' => 0.0, + 'to_invoice' => 0.0 + ] + ] + ]; + } + } From 5b58a3424e8896d66c0aff279065943b7e9c35fd Mon Sep 17 00:00:00 2001 From: Matias Montes Date: Sun, 25 Mar 2018 16:41:22 -0300 Subject: [PATCH 2/3] Fixed an empty line at the end of the class and reduced the number of lines in the test provider method --- .../Sales/Test/Unit/Model/Order/ItemTest.php | 136 ++++-------------- 1 file changed, 31 insertions(+), 105 deletions(-) diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php index bdfc365e25add..c1f53c3eae45c 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php @@ -275,161 +275,87 @@ public function getItemQtyVariants() return [ 'empty_item' => [ 'options' => [ - 'qty_ordered' => 0, - 'qty_invoiced' => 0, - 'qty_refunded' => 0, - 'qty_shipped' => 0, - 'qty_canceled' => 0, + 'qty_ordered' => 0, 'qty_invoiced' => 0, 'qty_refunded' => 0, 'qty_shipped' => 0, + 'qty_canceled' => 0 ], - 'expectedResult' => [ - 'to_ship' => 0.0, - 'to_invoice' => 0.0 - ] + 'expectedResult' => ['to_ship' => 0.0, 'to_invoice' => 0.0] ], 'ordered_item' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 0, - 'qty_refunded' => 0, - 'qty_shipped' => 0, - 'qty_canceled' => 0, + 'qty_ordered' => 12, 'qty_invoiced' => 0, 'qty_refunded' => 0, 'qty_shipped' => 0, + 'qty_canceled' => 0 ], - 'expectedResult' => [ - 'to_ship' => 12.0, - 'to_invoice' => 12.0 - ] + 'expectedResult' => ['to_ship' => 12.0, 'to_invoice' => 12.0] ], 'partially_invoiced' => [ - 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 4, - 'qty_refunded' => 0, - 'qty_shipped' => 0, + 'options' => ['qty_ordered' => 12, 'qty_invoiced' => 4, 'qty_refunded' => 0, 'qty_shipped' => 0, 'qty_canceled' => 0, ], - 'expectedResult' => [ - 'to_ship' => 12.0, - 'to_invoice' => 8.0 - ] + 'expectedResult' => ['to_ship' => 12.0, 'to_invoice' => 8.0] ], 'completely_invoiced' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 12, - 'qty_refunded' => 0, - 'qty_shipped' => 0, + 'qty_ordered' => 12, 'qty_invoiced' => 12, 'qty_refunded' => 0, 'qty_shipped' => 0, 'qty_canceled' => 0, ], - 'expectedResult' => [ - 'to_ship' => 12.0, - 'to_invoice' => 0.0 - ] + 'expectedResult' => ['to_ship' => 12.0, 'to_invoice' => 0.0] ], 'partially_invoiced_refunded' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 5, - 'qty_refunded' => 5, - 'qty_shipped' => 0, + 'qty_ordered' => 12, 'qty_invoiced' => 5, 'qty_refunded' => 5, 'qty_shipped' => 0, 'qty_canceled' => 0, ], - 'expectedResult' => [ - 'to_ship' => 7.0, - 'to_invoice' => 7.0 - ] + 'expectedResult' => ['to_ship' => 7.0, 'to_invoice' => 7.0] ], 'partially_refunded' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 12, - 'qty_refunded' => 5, - 'qty_shipped' => 0, + 'qty_ordered' => 12, 'qty_invoiced' => 12, 'qty_refunded' => 5, 'qty_shipped' => 0, 'qty_canceled' => 0, ], - 'expectedResult' => [ - 'to_ship' => 7.0, - 'to_invoice' => 0.0 - ] + 'expectedResult' => ['to_ship' => 7.0, 'to_invoice' => 0.0] ], 'partially_shipped' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 0, - 'qty_refunded' => 0, - 'qty_shipped' => 4, - 'qty_canceled' => 0, + 'qty_ordered' => 12, 'qty_invoiced' => 0, 'qty_refunded' => 0, 'qty_shipped' => 4, + 'qty_canceled' => 0 ], - 'expectedResult' => [ - 'to_ship' => 8.0, - 'to_invoice' => 12.0 - ] + 'expectedResult' => ['to_ship' => 8.0, 'to_invoice' => 12.0] ], 'partially_refunded_partially_shipped' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 12, - 'qty_refunded' => 5, - 'qty_shipped' => 4, - 'qty_canceled' => 0, + 'qty_ordered' => 12, 'qty_invoiced' => 12, 'qty_refunded' => 5, 'qty_shipped' => 4, + 'qty_canceled' => 0 ], - 'expectedResult' => [ - 'to_ship' => 3.0, - 'to_invoice' => 0.0 - ] + 'expectedResult' => ['to_ship' => 3.0, 'to_invoice' => 0.0] ], 'complete' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 12, - 'qty_refunded' => 0, - 'qty_shipped' => 12, - 'qty_canceled' => 0, + 'qty_ordered' => 12, 'qty_invoiced' => 12, 'qty_refunded' => 0, 'qty_shipped' => 12, + 'qty_canceled' => 0 ], - 'expectedResult' => [ - 'to_ship' => 0.0, - 'to_invoice' => 0.0 - ] + 'expectedResult' => ['to_ship' => 0.0, 'to_invoice' => 0.0] ], 'canceled' => [ 'options' => [ - 'qty_ordered' => 12, - 'qty_invoiced' => 0, - 'qty_refunded' => 0, - 'qty_shipped' => 0, - 'qty_canceled' => 12, + 'qty_ordered' => 12, 'qty_invoiced' => 0, 'qty_refunded' => 0, 'qty_shipped' => 0, + 'qty_canceled' => 12 ], - 'expectedResult' => [ - 'to_ship' => 0.0, - 'to_invoice' => 0.0 - ] + 'expectedResult' => ['to_ship' => 0.0, 'to_invoice' => 0.0] ], 'completely_shipped_using_decimals' => [ 'options' => [ - 'qty_ordered' => 4.4, - 'qty_invoiced' => 0.4, - 'qty_refunded' => 0.4, - 'qty_shipped' => 4, + 'qty_ordered' => 4.4, 'qty_invoiced' => 0.4, 'qty_refunded' => 0.4, 'qty_shipped' => 4, 'qty_canceled' => 0, ], - 'expectedResult' => [ - 'to_ship' => 0.0, - 'to_invoice' => 4.0 - ] + 'expectedResult' => ['to_ship' => 0.0, 'to_invoice' => 4.0] ], 'completely_invoiced_using_decimals' => [ 'options' => [ - 'qty_ordered' => 4.4, - 'qty_invoiced' => 4, - 'qty_refunded' => 0, - 'qty_shipped' => 4, - 'qty_canceled' => 0.4, + 'qty_ordered' => 4.4, 'qty_invoiced' => 4, 'qty_refunded' => 0, 'qty_shipped' => 4, + 'qty_canceled' => 0.4 ], - 'expectedResult' => [ - 'to_ship' => 0.0, - 'to_invoice' => 0.0 - ] + 'expectedResult' => ['to_ship' => 0.0, 'to_invoice' => 0.0] ] ]; } - } From 3c659994a2aad406af28138a886c6141d134f3ba Mon Sep 17 00:00:00 2001 From: Matias Montes Date: Sun, 25 Mar 2018 16:44:03 -0300 Subject: [PATCH 3/3] Merged to test methods into one. Trying to keep the public method count below 11 --- .../Sales/Test/Unit/Model/Order/ItemTest.php | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php index c1f53c3eae45c..39fffa23dc1ec 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/ItemTest.php @@ -244,23 +244,10 @@ public function getProductOptionsDataProvider() * * @dataProvider getItemQtyVariants */ - public function testGetSimpleQtyToShip(array $options, $expectedResult) + public function testGetSimpleQtyToMethods(array $options, $expectedResult) { $this->model->setData($options); $this->assertSame($this->model->getSimpleQtyToShip(), $expectedResult['to_ship']); - } - - /** - * Test different combinations of item qty setups - * - * @param array $options - * @param float $expectedResult - * - * @dataProvider getItemQtyVariants - */ - public function testGetQtyToInvoice(array $options, $expectedResult) - { - $this->model->setData($options); $this->assertSame($this->model->getQtyToInvoice(), $expectedResult['to_invoice']); }