fix: a fee its own extension refunds must not reach Two's residual - #514
Conversation
Amasty Extra Fee runs its own credit-memo total collector. Two reconciled that fee as an unclaimed residual and offered it for refund, so a memo could credit a fee Amasty had already decided was non-refundable, and the same fee's VAT was then deducted twice from the last memo's tax allowance. Claims the fee through the existing FeeLineProviderInterface seam, reading Amasty's own tables, so it never reaches the residual on any entity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rbP7hJ7nAVqeKHWrui6Fy
| /** | ||
| * Itemizes Amasty "Extra Fee" charges from that module's own tables. | ||
| * | ||
| * Amasty runs its own credit-memo total collector, so claiming the fee here |
There was a problem hiding this comment.
The measurement this fixes. Order 000000072 on a staging Magento shop: items GBP 108.00 net / 14.80 VAT, Two surcharge 7.25 / 1.45, Amasty fee 5.99 / 1.198 (is_eligible_refund = 0). Order tax 17.448, grand 138.688. Two partial credit memos.
Memo 2 is isLast(), and the collectors run 250 core tax -> 305 Amasty -> 600 Two surcharge -> 900 Two other charges:
- Core
Creditmemo\Total\Taxtakes theisLast()branch: taxInvoiced 17.448 - taxRefunded 1.798 = 15.650. - Amasty's collector hits its non-refundable branch and, because this memo is last, subtracts that fee's VAT: 15.650 - 1.198 = 14.452.
- Two's surcharge collector reads 14.452 - 14.800 itemised = -0.348, clamps to 0, and adds its whole 0.85 = 15.302.
Refunded tax 1.798 + 15.302 = 17.100 against 17.448 owed: 0.348 short. The Amasty fee's VAT is deducted twice - once implicitly (memo 1 had already spent it, shrinking memo 2's allowance) and once explicitly by Amasty's last-memo correction.
Without this module the same order reconciles exactly: memo 1 tax 0, memo 2 allowed 15.998 - 1.198 = 14.800 = the item VAT. The double-deduction exists only because Creditmemo\OtherCharges spent 1.198 of the allowance on memo 1.
The 0.348 is the smaller half of it. Two credited a fee the merchant configured non-refundable, so 138.34 went out where 131.498 was owed - a 6.842 over-refund.
| <arguments> | ||
| <argument name="providers" xsi:type="array"/> | ||
| <argument name="providers" xsi:type="array"> | ||
| <item name="amasty_extrafee" xsi:type="object">Two\Gateway\Service\Fee\Provider\AmastyExtraFee</item> |
There was a problem hiding this comment.
Why claim the fee rather than reconcile the shortfall. Two options closed the gap:
- Honour the owning module - Two never refunds a fee whose extension has its own credit-memo collector. Total refunded 131.498, memo totals close exactly.
- Keep refunding it and write back to Amasty's ledger (
amasty_extrafee_creditmemo,amasty_extrafee_order.*_refunded) so it stops deducting. Total refunded 138.688.
(2) moves more money out by overriding is_eligible_refund = 0, an explicit merchant setting. That is not ours to overrule, so this is (1).
The rule is "a fee with its own credit-memo collector", not "the Amasty module": the collector stays vendor-neutral and ownership is declared through the existing FeeLineProviderInterface seam. Registering a provider here was the intended route - the seam's own docblock asks for a real install's field and table names, which this one is built from.
| $name = trim(($row['fee_label'] ?? '') . ' ' . ($row['fee_option_label'] ?? '')); | ||
|
|
||
| return [ | ||
| 'order_item_id' => sprintf( |
There was a problem hiding this comment.
Risk worth a reviewer's eye. An Amasty fee previously reached Two's API as a single other_charges line. It now arrives under this id. Orders placed before this ships carry the old id, so a later refund payload for one of them would cite the new one. Harmless if Two's API does not match refund lines to order lines by order_item_id; if it does, in-flight orders need a thought. With is_eligible_refund = 0 the case cannot arise (no memo row, so no line either way) - it only bites a refundable fee refunded through Amasty's own memo field.
Comments only; no payload or behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rbP7hJ7nAVqeKHWrui6Fy
A fee whose own extension refunds it must never reach Two's "other charges" residual — the residual is what the merchant is offered to refund.
Service\Fee\Provider\AmastyExtraFeeclaims Amasty Extra Fee lines from that module's own tables, for orders, invoices and credit memos.etc/di.xml, the first entry in the previously-emptyFeeLineProviderPool.Api\Fee\FeeLineProviderInterface,Model\Total\Creditmemo\OtherCharges,Service\Order::getOtherChargesLineItem()andAGENTS.md, which stated that a fee reaching grand total via a totals collector never reaches a credit memo and that a tax-engine-integrated extension needs no provider. Both are false for an extension with its own credit-memo collector, and that premise is what produced the bug.setup:di:compileand install work without the module.🤖 Generated with Claude Code
https://claude.ai/code/session_014rbP7hJ7nAVqeKHWrui6Fy