Skip to content

Commit 763d253

Browse files
author
Anna Bukatar
committed
ACP2E-1190: Refunded shipping amount when taxes and a discount are used
1 parent eb1e3a0 commit 763d253

File tree

3 files changed

+208
-16
lines changed

3 files changed

+208
-16
lines changed

‎app/code/Magento/Sales/Model/Order/Creditmemo/Total/Shipping.php‎

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\Sales\Model\Order\Creditmemo\Total;
77

88
use Magento\Framework\Pricing\PriceCurrencyInterface;
9+
use Magento\Tax\Model\Calculation as TaxCalculation;
910

1011
/**
1112
* Order creditmemo shipping total calculation model
@@ -56,12 +57,9 @@ public function collect(\Magento\Sales\Model\Order\Creditmemo $creditmemo)
5657
// amounts including tax
5758
$orderShippingInclTax = $order->getShippingInclTax();
5859
$orderBaseShippingInclTax = $order->getBaseShippingInclTax();
59-
$allowedTaxAmount = $order->getShippingTaxAmount() - $order->getShippingTaxRefunded();
60-
$allowedAmountInclTax = $allowedAmount + $allowedTaxAmount;
61-
$baseAllowedAmountInclTax = $orderBaseShippingInclTax
62-
- $order->getBaseShippingRefunded()
63-
- $order->getBaseShippingTaxRefunded();
64-
$baseAllowedAmountInclTax = max($baseAllowedAmountInclTax, 0);
60+
$allowedAmountInclTax =$this->getAllowedAmountInclTax($order);
61+
$baseAllowedAmountInclTax = $this->getBaseAllowedAmountInclTax($order);
62+
6563
// Check if the desired shipping amount to refund was specified (from invoice or another source).
6664
if ($creditmemo->hasBaseShippingAmount()) {
6765
// For the conditional logic, we will either use amounts that always include tax -OR- never include tax.
@@ -114,6 +112,60 @@ public function collect(\Magento\Sales\Model\Order\Creditmemo $creditmemo)
114112
return $this;
115113
}
116114

115+
/**
116+
* Checks if shipping provided incl tax, tax applied after discount, and discount applied on shipping excl tax
117+
*
118+
* @param \Magento\Sales\Model\Order $order
119+
* @return bool
120+
*/
121+
private function isShippingIncludeTaxWithTaxAfterDiscountOnExcl($order): bool
122+
{
123+
return $this->getTaxConfig()->getCalculationSequence($order->getStoreId())
124+
=== TaxCalculation::CALC_TAX_AFTER_DISCOUNT_ON_EXCL &&
125+
$this->isSuppliedShippingAmountInclTax($order);
126+
}
127+
128+
/**
129+
* Get allowed shipping amount to refund based on tax settings
130+
*
131+
* @param \Magento\Sales\Model\Order $order
132+
* @return float
133+
*/
134+
private function getAllowedAmountInclTax(\Magento\Sales\Model\Order $order): float
135+
{
136+
if ($this->isShippingIncludeTaxWithTaxAfterDiscountOnExcl($order)) {
137+
$result = $order->getShippingInclTax();
138+
foreach ($order->getCreditmemosCollection() as $creditmemo) {
139+
$result -= $creditmemo->getShippingInclTax();
140+
}
141+
} else {
142+
$result = ($order->getShippingAmount() - $order->getShippingRefunded()) +
143+
($order->getShippingTaxAmount() - $order->getShippingTaxRefunded());
144+
}
145+
146+
return $result;
147+
}
148+
149+
/**
150+
* Get base allowed shipping amount to refund based on tax settings
151+
*
152+
* @param \Magento\Sales\Model\Order $order
153+
* @return float
154+
*/
155+
private function getBaseAllowedAmountInclTax(\Magento\Sales\Model\Order $order): float
156+
{
157+
$result = $order->getBaseShippingInclTax();
158+
if ($this->isShippingIncludeTaxWithTaxAfterDiscountOnExcl($order)) {
159+
foreach ($order->getCreditmemosCollection() as $creditmemo) {
160+
$result -= $creditmemo->getBaseShippingInclTax();
161+
}
162+
} else {
163+
$result -= $order->getBaseShippingRefunded() + $order->getBaseShippingTaxRefunded();
164+
}
165+
166+
return max($result, 0);
167+
}
168+
117169
/**
118170
* Returns whether the user specified a shipping amount that already includes tax
119171
*
@@ -132,6 +184,7 @@ private function isSuppliedShippingAmountInclTax($order)
132184
* @return \Magento\Tax\Model\Config
133185
*
134186
* @deprecated 100.1.0
187+
* @see \Magento\Tax\Model\Config
135188
*/
136189
private function getTaxConfig()
137190
{

‎app/code/Magento/Sales/Model/Order/Creditmemo/Total/Tax.php‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
use Magento\Sales\Model\Order\Creditmemo;
1010
use Magento\Sales\Model\Order\Invoice;
1111
use Magento\Sales\Model\ResourceModel\Order\Invoice as ResourceInvoice;
12+
use Magento\Tax\Model\Config as TaxConfig;
13+
use Magento\Tax\Model\Calculation as TaxCalculation;
14+
use Magento\Framework\App\ObjectManager;
1215

1316
/**
1417
* Collects credit memo taxes.
@@ -20,13 +23,22 @@ class Tax extends AbstractTotal
2023
*/
2124
private $resourceInvoice;
2225

26+
/**
27+
* Tax config from Tax model
28+
*
29+
* @var TaxConfig
30+
*/
31+
private $taxConfig;
32+
2333
/**
2434
* @param ResourceInvoice $resourceInvoice
2535
* @param array $data
36+
* @param TaxConfig|null $taxConfig
2637
*/
27-
public function __construct(ResourceInvoice $resourceInvoice, array $data = [])
38+
public function __construct(ResourceInvoice $resourceInvoice, array $data = [], ?TaxConfig $taxConfig = null)
2839
{
2940
$this->resourceInvoice = $resourceInvoice;
41+
$this->taxConfig = $taxConfig ?: ObjectManager::getInstance()->get(TaxConfig::class);
3042
parent::__construct($data);
3143
}
3244

@@ -122,7 +134,8 @@ public function collect(Creditmemo $creditmemo)
122134
$baseShippingDiscountTaxCompensationAmount = 0;
123135
$shippingDelta = $baseOrderShippingAmount - $baseOrderShippingRefundedAmount;
124136

125-
if ($shippingDelta > $creditmemo->getBaseShippingAmount()) {
137+
if ($shippingDelta > $creditmemo->getBaseShippingAmount() ||
138+
$this->isShippingIncludeTaxWithTaxAfterDiscountOnExcl($order->getStoreId())) {
126139
$part = $creditmemo->getShippingAmount() / $orderShippingAmount;
127140
$basePart = $creditmemo->getBaseShippingAmount() / $baseOrderShippingAmount;
128141
$shippingTaxAmount = $order->getShippingTaxAmount() * $part;
@@ -194,6 +207,18 @@ public function collect(Creditmemo $creditmemo)
194207
return $this;
195208
}
196209

210+
/**
211+
* Checks if shipping provided incl tax, tax applied after discount, and discount applied on shipping excl tax
212+
*
213+
* @param int|null $storeId
214+
* @return bool
215+
*/
216+
private function isShippingIncludeTaxWithTaxAfterDiscountOnExcl(?int $storeId): bool
217+
{
218+
return $this->taxConfig->getCalculationSequence($storeId) === TaxCalculation::CALC_TAX_AFTER_DISCOUNT_ON_EXCL &&
219+
$this->taxConfig->displaySalesShippingInclTax($storeId);
220+
}
221+
197222
/**
198223
* Calculate allowed to Credit Memo tax amount
199224
*

‎app/code/Magento/Sales/Test/Unit/Model/Order/Creditmemo/Total/ShippingTest.php‎

Lines changed: 122 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
1515
use Magento\Sales\Model\Order\Creditmemo;
1616
use Magento\Sales\Model\Order\Creditmemo\Total\Shipping;
17+
use Magento\Tax\Model\Calculation as TaxCalculation;
1718
use Magento\Tax\Model\Config;
1819
use PHPUnit\Framework\MockObject\MockObject;
1920
use PHPUnit\Framework\TestCase;
@@ -104,11 +105,11 @@ public function testCollectException()
104105
->with($allowedShippingAmount, null, false)
105106
->willReturn($allowedShippingAmount);
106107

107-
$order = new DataObject(
108+
$order = $this->getOrderMock(
108109
[
109-
'base_shipping_amount' => $orderShippingAmount,
110-
'base_shipping_refunded' => $orderShippingRefunded,
111-
'base_currency' => $currencyMock,
110+
'base_shipping_amount' => $orderShippingAmount,
111+
'base_shipping_refunded' => $orderShippingRefunded,
112+
'base_currency' => $currencyMock
112113
]
113114
);
114115

@@ -126,6 +127,20 @@ public function testCollectException()
126127
$this->shippingCollector->collect($this->creditmemoMock);
127128
}
128129

130+
private function getOrderMock($data)
131+
{
132+
$orderMock = $this->getMockBuilder(\Magento\Sales\Model\Order::class)
133+
->disableOriginalConstructor()
134+
->getMock();
135+
136+
foreach ($data as $method => $returnValue) {
137+
$orderMock
138+
->method('get' . str_replace('_', '', ucwords($method, '_')))
139+
->willReturn($returnValue);
140+
}
141+
return $orderMock;
142+
}
143+
129144
/**
130145
* situation: The admin user did *not* specify any desired refund amount
131146
*
@@ -154,7 +169,7 @@ public function testCollectNoSpecifiedShippingAmount()
154169

155170
$this->taxConfig->expects($this->any())->method('displaySalesShippingInclTax')->willReturn(false);
156171

157-
$order = new DataObject(
172+
$order = $this->getOrderMock(
158173
[
159174
'shipping_amount' => $orderShippingAmount,
160175
'shipping_refunded' => $orderShippingRefunded,
@@ -241,7 +256,7 @@ public function testCollectWithSpecifiedShippingAmount($ratio)
241256

242257
$this->taxConfig->expects($this->any())->method('displaySalesShippingInclTax')->willReturn(false);
243258

244-
$order = new DataObject(
259+
$order = $this->getOrderMock(
245260
[
246261
'shipping_amount' => $orderShippingAmount,
247262
'shipping_refunded' => $orderShippingAmountRefunded,
@@ -346,7 +361,7 @@ public function testCollectUsingTaxInclShippingAmount()
346361
$expectedGrandTotal = $grandTotalBefore + $expectedShippingAmount;
347362
$expectedBaseGrandTtoal = $baseGrandTotalBefore + $expectedBaseShippingAmount;
348363

349-
$order = new DataObject(
364+
$order = $this->getOrderMock(
350365
[
351366
'shipping_amount' => $orderShippingAmount,
352367
'base_shipping_amount' => $baseOrderShippingAmount,
@@ -405,6 +420,7 @@ public function testCollectUsingTaxInclShippingAmount()
405420

406421
$this->shippingCollector->collect($this->creditmemoMock);
407422
}
423+
408424
/**
409425
* situation: The admin user did *not* specify any desired refund amount
410426
*
@@ -434,7 +450,7 @@ public function testCollectRefundShippingAmountIncTax()
434450

435451
$this->taxConfig->expects($this->any())->method('displaySalesShippingInclTax')->willReturn(false);
436452

437-
$order = new DataObject(
453+
$order = $this->getOrderMock(
438454
[
439455
'shipping_amount' => $orderShippingAmount,
440456
'shipping_refunded' => $orderShippingRefunded,
@@ -489,4 +505,102 @@ public function testCollectRefundShippingAmountIncTax()
489505
->willReturnSelf();
490506
$this->shippingCollector->collect($this->creditmemoMock);
491507
}
508+
509+
/**
510+
* situation: The admin user specified the desired refund amount that has taxes and discount embedded within it
511+
*
512+
* @throws LocalizedException
513+
*/
514+
public function testCollectUsingShippingInclTaxAndDiscountOnExclBeforeTax()
515+
{
516+
$this->taxConfig->expects($this->any())->method('displaySalesShippingInclTax')->willReturn(true);
517+
$this->taxConfig->expects($this->any())
518+
->method('getCalculationSequence')
519+
->willReturn(TaxCalculation::CALC_TAX_AFTER_DISCOUNT_ON_EXCL);
520+
521+
$orderShippingAmount = 14.55;
522+
$shippingTaxAmount = 0.45;
523+
$shippingDiscountAmount = 10;
524+
$orderShippingInclTax = 15;
525+
$orderShippingAmountRefunded = 7.27;
526+
$orderShippingAmountInclTaxRefunded = 8;
527+
$shippingTaxRefunded = 0.24;
528+
529+
$currencyMultiple = 2;
530+
$baseOrderShippingAmount = $orderShippingAmount * $currencyMultiple;
531+
$baseShippingTaxAmount = $shippingTaxAmount * $currencyMultiple;
532+
$baseOrderShippingInclTax = $orderShippingInclTax * $currencyMultiple;
533+
$baseOrderShippingAmountRefunded = $orderShippingAmountRefunded * $currencyMultiple;
534+
$baseShippingTaxRefunded = $shippingTaxRefunded * $currencyMultiple;
535+
536+
//determine expected amounts
537+
$expectedShippingAmount = $orderShippingAmount - $orderShippingAmountRefunded;
538+
$expectedShippingAmountInclTax = $orderShippingInclTax - $orderShippingAmountInclTaxRefunded;
539+
540+
$expectedBaseShippingAmount = $expectedShippingAmount * $currencyMultiple;
541+
$expectedBaseShippingAmountInclTax = $expectedShippingAmountInclTax * $currencyMultiple;
542+
543+
$grandTotalBefore = 27;
544+
$baseGrandTotalBefore = $grandTotalBefore * $currencyMultiple;
545+
$expectedGrandTotal = $grandTotalBefore + $expectedShippingAmount;
546+
$expectedBaseGrandTotal = $baseGrandTotalBefore + $expectedBaseShippingAmount;
547+
548+
$order = $this->getOrderMock(
549+
[
550+
'shipping_amount' => $orderShippingAmount,
551+
'base_shipping_amount' => $baseOrderShippingAmount,
552+
'shipping_refunded' => $orderShippingAmountRefunded,
553+
'base_shipping_refunded' => $baseOrderShippingAmountRefunded,
554+
'shipping_incl_tax' => $orderShippingInclTax,
555+
'base_shipping_incl_tax' => $baseOrderShippingInclTax,
556+
'shipping_tax_amount' => $shippingTaxAmount,
557+
'shipping_tax_refunded' => $shippingTaxRefunded,
558+
'base_shipping_tax_amount' => $baseShippingTaxAmount,
559+
'base_shipping_tax_refunded' => $baseShippingTaxRefunded,
560+
'shipping_discount_amount' => $shippingDiscountAmount
561+
]
562+
);
563+
$orderCreditMemo = $this->createMock(Creditmemo::class);
564+
$orderCreditMemo->expects($this->atLeastOnce())
565+
->method('getShippingInclTax')
566+
->willReturn($orderShippingAmountInclTaxRefunded);
567+
$orderCreditMemo->expects($this->atLeastOnce())
568+
->method('getBaseShippingInclTax')
569+
->willReturn($orderShippingAmountInclTaxRefunded * $currencyMultiple);
570+
$order->expects($this->atLeastOnce())
571+
->method('getCreditmemosCollection')
572+
->willReturn([$orderCreditMemo]);
573+
574+
$this->creditmemoMock->expects($this->once())->method('getOrder')->willReturn($order);
575+
$this->creditmemoMock->expects($this->once())->method('hasBaseShippingAmount')->willReturn(false);
576+
$this->creditmemoMock->expects($this->once())->method('getGrandTotal')->willReturn($grandTotalBefore);
577+
$this->creditmemoMock->expects($this->once())->method('getBaseGrandTotal')->willReturn($baseGrandTotalBefore);
578+
579+
//verify
580+
$this->creditmemoMock->expects($this->once())
581+
->method('setShippingAmount')
582+
->with($expectedShippingAmount)
583+
->willReturnSelf();
584+
$this->creditmemoMock->expects($this->once())
585+
->method('setBaseShippingAmount')
586+
->with($expectedBaseShippingAmount)
587+
->willReturnSelf();
588+
$this->creditmemoMock->expects($this->once())
589+
->method('setShippingInclTax')
590+
->with($expectedShippingAmountInclTax)
591+
->willReturnSelf();
592+
$this->creditmemoMock->expects($this->once())
593+
->method('setBaseShippingInclTax')
594+
->with($expectedBaseShippingAmountInclTax)
595+
->willReturnSelf();
596+
$this->creditmemoMock->expects($this->once())
597+
->method('setGrandTotal')
598+
->with($expectedGrandTotal)
599+
->willReturnSelf();
600+
$this->creditmemoMock->expects($this->once())
601+
->method('setBaseGrandTotal')
602+
->with($expectedBaseGrandTotal)
603+
->willReturnSelf();
604+
$this->shippingCollector->collect($this->creditmemoMock);
605+
}
492606
}

0 commit comments

Comments
 (0)