From 79e61d8f9e4b107b76cdd0e2517ba9ca7b3bc53d Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Thu, 4 Jun 2026 21:54:59 +0100 Subject: [PATCH 1/2] fix(surcharge): show net surcharge in invoice/order/creditmemo totals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The totals display blocks (on-screen and PDF) rendered the surcharge GROSS (net + VAT). That matched the OLD inflated grand total, but since the collector fix (ABN-443 / #201) the grand total is net-based, so the gross row no longer reconciles — and it double-presents the VAT (once in the Tax line, once inside the surcharge row). Show the surcharge NET in both blocks; its VAT stays in the Tax line, exactly as on checkout. Net rows (items + shipping + tax + net surcharge) now sum to the grand total. Adds unit coverage for both blocks. Refs ABN-443 Co-Authored-By: Claude Opus 4.8 (1M context) --- Block/Sales/Total/Surcharge.php | 13 ++- Model/Pdf/Total/Surcharge.php | 5 +- Test/Stubs/DataObject.php | 13 +++ Test/Stubs/SalesModels.php | 9 ++ Test/Unit/Block/Sales/Total/SurchargeTest.php | 98 +++++++++++++++++++ Test/Unit/Model/Pdf/Total/SurchargeTest.php | 71 ++++++++++++++ 6 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 Test/Unit/Block/Sales/Total/SurchargeTest.php create mode 100644 Test/Unit/Model/Pdf/Total/SurchargeTest.php diff --git a/Block/Sales/Total/Surcharge.php b/Block/Sales/Total/Surcharge.php index 99480e35..37745b31 100644 --- a/Block/Sales/Total/Surcharge.php +++ b/Block/Sales/Total/Surcharge.php @@ -35,17 +35,16 @@ public function initTotals(): self } $amount = (float)$source->getDataUsingMethod('two_surcharge_amount'); - $taxAmount = (float)$source->getDataUsingMethod('two_surcharge_tax_amount'); if ($amount <= 0) { return $this; } - // Display gross (net + tax) so the row matches the invoice/order grand - // total math; tax is otherwise hidden in the Tax line. - $value = $amount + $taxAmount; - $baseAmount = (float)$source->getDataUsingMethod('base_two_surcharge_amount'); - $baseTax = (float)$source->getDataUsingMethod('base_two_surcharge_tax_amount'); - $baseValue = $baseAmount + $baseTax; + // Display the NET surcharge — its VAT lives in the Tax line, exactly as + // on checkout. The net rows (items + shipping + tax + net surcharge) + // then reconcile to the grand total. (Showing gross here double- + // presents the surcharge VAT — once in the Tax line, once in this row.) + $value = $amount; + $baseValue = (float)$source->getDataUsingMethod('base_two_surcharge_amount'); $label = $source->getDataUsingMethod('two_surcharge_description'); if (!$label) { diff --git a/Model/Pdf/Total/Surcharge.php b/Model/Pdf/Total/Surcharge.php index 72e11bdf..c95f4388 100644 --- a/Model/Pdf/Total/Surcharge.php +++ b/Model/Pdf/Total/Surcharge.php @@ -25,7 +25,6 @@ public function getTotalsForDisplay() { $source = $this->getSource(); $amount = (float)$source->getDataUsingMethod('two_surcharge_amount'); - $tax = (float)$source->getDataUsingMethod('two_surcharge_tax_amount'); if ($amount <= 0) { return []; } @@ -33,7 +32,9 @@ public function getTotalsForDisplay() $label = $source->getDataUsingMethod('two_surcharge_description') ?: (string)__('Two Surcharge'); - $value = $this->getAmountPrefix() . $this->getOrder()->formatPriceTxt($amount + $tax); + // NET surcharge — its VAT is shown in the Tax line, matching the + // on-screen totals, the checkout, and the grand total. + $value = $this->getAmountPrefix() . $this->getOrder()->formatPriceTxt($amount); return [[ 'amount' => $value, diff --git a/Test/Stubs/DataObject.php b/Test/Stubs/DataObject.php index 50f37909..f7c57fd9 100644 --- a/Test/Stubs/DataObject.php +++ b/Test/Stubs/DataObject.php @@ -13,6 +13,19 @@ class DataObject /** @var array */ protected $_data = []; + public function __construct(array $data = []) + { + $this->_data = $data; + } + + public function getData($key = '') + { + if ($key === '') { + return $this->_data; + } + return $this->_data[$key] ?? null; + } + public function __call($method, $args) { $prefix = substr($method, 0, 3); diff --git a/Test/Stubs/SalesModels.php b/Test/Stubs/SalesModels.php index 937c3a70..381ce4d5 100644 --- a/Test/Stubs/SalesModels.php +++ b/Test/Stubs/SalesModels.php @@ -39,6 +39,15 @@ public function getData($key = '', $index = null) return $this->_data[$key] ?? null; } + /** + * Mirrors Magento\Framework\DataObject::getDataUsingMethod for the simple + * data-bag case the totals block exercises (no custom getter override). + */ + public function getDataUsingMethod($key, $args = null) + { + return $this->_data[$key] ?? null; + } + public function hasData($key = ''): bool { if ($key === '') { diff --git a/Test/Unit/Block/Sales/Total/SurchargeTest.php b/Test/Unit/Block/Sales/Total/SurchargeTest.php new file mode 100644 index 00000000..14665032 --- /dev/null +++ b/Test/Unit/Block/Sales/Total/SurchargeTest.php @@ -0,0 +1,98 @@ +setData('two_surcharge_amount', self::NET); + $s->setData('base_two_surcharge_amount', self::NET); + $s->setData('two_surcharge_tax_amount', self::TAX); + $s->setData('base_two_surcharge_tax_amount', self::TAX); + $s->setData('two_surcharge_description', 'Zakelijk op Rekening - 60 dagen'); + return $s; + } + + /** + * Build the block with its framework collaborators stubbed: a parent + * totals block exposing the source and capturing the registered row. + */ + private function makeBlock(Order $source, \stdClass $capture): Surcharge + { + $parent = new class($source, $capture) { + private $src; + private $cap; + public function __construct($src, $cap) + { + $this->src = $src; + $this->cap = $cap; + } + public function getSource() + { + return $this->src; + } + public function addTotalBefore($total, $before) + { + $this->cap->total = $total; + $this->cap->before = $before; + return $this; + } + }; + + return new class($parent) extends Surcharge { + private $p; + public function __construct($p) + { + $this->p = $p; + } + public function getParentBlock() + { + return $this->p; + } + }; + } + + public function testDisplaysSurchargeNetNotGross(): void + { + $capture = new \stdClass(); + $block = $this->makeBlock($this->makeSource(), $capture); + + $block->initTotals(); + + $this->assertNotNull($capture->total ?? null, 'a surcharge totals row must be registered'); + $this->assertEqualsWithDelta( + self::NET, + (float)$capture->total->getValue(), + 0.0001, + 'surcharge row must show NET; its VAT belongs in the Tax line (matches checkout)' + ); + $this->assertEqualsWithDelta( + self::NET, + (float)$capture->total->getData('base_value'), + 0.0001, + 'base_value must also be NET' + ); + $this->assertSame('Zakelijk op Rekening - 60 dagen', $capture->total->getLabel()); + $this->assertSame('grand_total', $capture->before); + } +} diff --git a/Test/Unit/Model/Pdf/Total/SurchargeTest.php b/Test/Unit/Model/Pdf/Total/SurchargeTest.php new file mode 100644 index 00000000..3ee5f266 --- /dev/null +++ b/Test/Unit/Model/Pdf/Total/SurchargeTest.php @@ -0,0 +1,71 @@ +setData('two_surcharge_amount', 23.99); + $source->setData('two_surcharge_tax_amount', 5.16); + $source->setData('two_surcharge_description', 'Zakelijk op Rekening - 60 dagen'); + + $orderFmt = new class { + public function formatPriceTxt($v) + { + return number_format((float)$v, 2, '.', ''); + } + }; + + $block = new class($source, $orderFmt) extends Surcharge { + private $s; + private $o; + public function __construct($s, $o) + { + $this->s = $s; + $this->o = $o; + } + public function getSource() + { + return $this->s; + } + public function getOrder() + { + return $this->o; + } + public function getAmountPrefix() + { + return ''; + } + public function getFontSize() + { + return 7; + } + }; + + $rows = $block->getTotalsForDisplay(); + + $this->assertCount(1, $rows); + $this->assertSame( + '23.99', + $rows[0]['amount'], + 'PDF surcharge row must show NET (23.99), not gross (29.15)' + ); + $this->assertSame('Zakelijk op Rekening - 60 dagen:', $rows[0]['label']); + } +} From 81e319dc814045926de318b6f33656658d7e5e5c Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Thu, 4 Jun 2026 22:02:38 +0100 Subject: [PATCH 2/2] fix(surcharge): place surcharge row directly above the Tax line The surcharge contributes to the tax base, so showing net surcharge then tax reads more naturally (and matches checkout ordering). Insert the totals row before `tax` instead of before `grand_total`. Refs ABN-443 Co-Authored-By: Claude Opus 4.8 (1M context) --- Block/Sales/Total/Surcharge.php | 5 ++++- Test/Unit/Block/Sales/Total/SurchargeTest.php | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Block/Sales/Total/Surcharge.php b/Block/Sales/Total/Surcharge.php index 37745b31..553f0b78 100644 --- a/Block/Sales/Total/Surcharge.php +++ b/Block/Sales/Total/Surcharge.php @@ -51,6 +51,9 @@ public function initTotals(): self $label = (string)__('Two Surcharge'); } + // Place the surcharge row directly above the Tax line — the surcharge + // is part of the tax base, so net surcharge then tax reads naturally + // (and matches checkout ordering). $parent->addTotalBefore( new DataObject([ 'code' => 'two_surcharge', @@ -58,7 +61,7 @@ public function initTotals(): self 'base_value' => $baseValue, 'label' => $label, ]), - 'grand_total' + 'tax' ); return $this; diff --git a/Test/Unit/Block/Sales/Total/SurchargeTest.php b/Test/Unit/Block/Sales/Total/SurchargeTest.php index 14665032..259b7ce1 100644 --- a/Test/Unit/Block/Sales/Total/SurchargeTest.php +++ b/Test/Unit/Block/Sales/Total/SurchargeTest.php @@ -93,6 +93,10 @@ public function testDisplaysSurchargeNetNotGross(): void 'base_value must also be NET' ); $this->assertSame('Zakelijk op Rekening - 60 dagen', $capture->total->getLabel()); - $this->assertSame('grand_total', $capture->before); + $this->assertSame( + 'tax', + $capture->before, + 'surcharge row must sit directly above the Tax line (it contributes to the tax base)' + ); } }