ABN-298: Extended net terms, surcharge config, and dev tooling by dgjlindsay · Pull Request #82 · two-inc/magento-plugin

dgjlindsay · 2026-04-15T16:57:38Z

Summary

Extended net terms with buyer-selectable payment terms at checkout
Surcharge configuration grid (fixed fee, percentage, limit per term) with differential mode
Currency-aware fixed fees: saved with store base currency, converted via Magento FX rates at order time, orders rejected if no rate exists
Explicit zero limit respected (null = no cap, 0 = cap at zero)
Dev tooling: FRP proxy, Xdebug helper, debug mode toggle, Makefile targets
Admin header rebrand (removed Magmodules credit)

Test plan

Configure surcharge grid with fixed fees — verify currency code shown in column headers
Place order in same currency as config — surcharge applied without conversion
Place order in different currency — fixed fee and limit converted via Magento rates
Place order in currency with no exchange rate configured — order rejected with clear error
Set limit to explicit 0 — surcharge capped to zero
Unit tests: 115 passing (make test)

🤖 Generated with Claude Code

Fixed fees and limits in the surcharge grid are now saved with the store's base currency at time of configuration. When a sale uses a different currency, Magento's built-in exchange rates convert the amounts; orders are rejected if no rate exists. The admin grid headers now display the active currency code. Explicit zero limits are respected (null = no cap, 0 = cap at zero). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

gemini-code-assist · 2026-04-15T16:57:42Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

claude

No critical issues found. The currency-aware surcharge logic is sound: fixed fees and limits are converted via Magento's exchange rate API, missing rates throw a LocalizedException (which Magento's checkout layer surfaces as a user-facing error), and the null vs 0 limit distinction is correctly handled end-to-end. Test coverage covers same-currency, cross-currency, zero-limit, legacy-empty-currency, and conversion-failure paths. Ready for human approval.

…ew fixes - Replace payment term dropdown with chips showing surcharge per term - Add surcharge line to order summary, update grand total via KO mixin - Shared surcharge model syncs chip selection with summary reactively - Fix tax rate resolution: use getRateRequest()+getRate() instead of getDefaultCalculatedRate() which skips the country/region pipeline - Fix %% in __() strings (positional substitution, not sprintf) - Review fixes: remove unused import, country fallback from store config instead of hardcoded 'NO', strict float comparison, float cast for percentage/fixed config, defensive parseFloat in mixin, translatable fallback in summary component - ConfigProvider computes gross surcharges (incl tax) per term at checkout page load via SurchargeCalculator Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude

Reviewed the updated diff (new commit since previous review). No critical issues found. The currency conversion logic, null/zero limit distinction, and frontend chip selector all look correct. Ready for human approval.

The KO mixin approach for showing surcharges in the order summary was fragile — broke across Magento_Checkout vs Magento_Tax grand-total variants and crashed when _super() called non-existent methods. Replace with Magento's native totals pipeline: - Total Collector (Model/Total/Surcharge) runs during collectTotals(), calculates surcharge via SurchargeCalculator, adds to grand total, stores details in CheckoutSession for ComposeOrder - REST endpoint (POST /V1/two/select-term) sets the selected term in session and returns recalculated totals - ComposeOrder reads surcharge from session instead of recalculating, avoids double-counting since grand total already includes surcharge - Surcharge summary component reads from totals segment (standard Magento pattern) - In-memory fee cache on SurchargeCalculator prevents redundant pricing API calls when collectTotals() fires multiple times - Added make flush target for clearing frontend static content Removed: grand-total-mixin.js, requirejs mixin registrations, valueHasMutated hack, all _super() calls on checkout components. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude · 2026-04-15T21:03:04Z

+        $total->setGrandTotal($grandTotal + $grossAmount);
+        $total->setBaseGrandTotal((float)$total->getBaseGrandTotal() + $grossAmount);
+        $total->setTaxAmount((float)$total->getTaxAmount() + $taxAmount);
+        $total->setBaseTaxAmount((float)$total->getBaseTaxAmount() + $taxAmount);


$grossAmount and $taxAmount are in quote currency, but base_grand_total and base_tax_amount are supposed to be in base currency. For multi-currency stores these will be stored at the wrong magnitude, causing incorrect base-currency order totals and tax reports.

The quote already carries the conversion rate via getBaseToQuoteRate():

Suggested change

$total->setGrandTotal($grandTotal + $grossAmount);

$total->setBaseGrandTotal((float)$total->getBaseGrandTotal() + $grossAmount);

$total->setTaxAmount((float)$total->getTaxAmount() + $taxAmount);

$total->setBaseTaxAmount((float)$total->getBaseTaxAmount() + $taxAmount);

$baseToQuoteRate = (float)$quote->getBaseToQuoteRate() ?: 1.0;

$baseGrossAmount = round($grossAmount / $baseToQuoteRate, 4);

$baseTaxAmount = round($taxAmount / $baseToQuoteRate, 4);

$total->setGrandTotal($grandTotal + $grossAmount);

$total->setBaseGrandTotal((float)$total->getBaseGrandTotal() + $baseGrossAmount);

$total->setTaxAmount((float)$total->getTaxAmount() + $taxAmount);

$total->setBaseTaxAmount((float)$total->getBaseTaxAmount() + $baseTaxAmount);

Still present in the latest commit (ce5557e). setBaseGrandTotal and setBaseTaxAmount still receive quote-currency values — the suggestion in the original comment (dividing by getBaseToQuoteRate()) has not been applied. This will corrupt base-currency order totals and tax reports on any store with a non-1:1 exchange rate.

Still present in the latest commit (22efeba). setBaseGrandTotal and setBaseTaxAmount are still receiving quote-currency values ($grossAmount / $taxAmount) — the fix has not been applied. Recommend holding merge until this is addressed.

Still present in the latest commit (e2d51d2). setBaseGrandTotal and setBaseTaxAmount are still receiving quote-currency values ($grossAmount / $taxAmount). The fix from the original suggestion has not been applied. Please divide by getBaseToQuoteRate() before setting the base amounts.

The address type check could return early on valid assignments where getAddressType() was empty. Use the standard Magento pattern of checking for items in the shipping assignment instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude

Re-reviewed after latest commit. No new critical issues found, but the base-currency surcharge bug flagged in the inline thread on Model/Total/Surcharge.php remains unresolved — setBaseGrandTotal and setBaseTaxAmount are still populated with quote-currency values. Recommend holding merge until that is fixed.

…smetics - Fix chip labels not updating on coupon apply/remove: subscribe to quote.getTotals() for external totals changes, re-fire select-term AJAX to refresh all chip surcharges - Fix selected term not persisting across page refresh: ConfigProvider sends selectedPaymentTerm from session, JS initializes from it - Fix ConfigProvider using inflated base (included prior surcharge): subtract existing surcharge before computing chip labels - Fix surcharge.js subscribe crash: quote.getTotals().subscribe() not quote.getTotals.subscribe (need function call to get observable) - TermSelection endpoint returns surcharges for all terms so chip labels update on every AJAX response - termSurcharges now ko.observable, termOptions now ko.pureComputed for reactive chip label updates - Total Collector: sort_order 450→600 (after grand_total at 550), guard checks items not address type, debug logging via LogRepository, stores surcharge on $total for fetch() in same request - SurchargeTaxRate warning: clickable links to Tax Rates, Tax Rules, Store Country, Default Tax Country admin pages - Chip CSS: selected state covers :hover/:focus/:active, no grey flash - Updated translations for all three locales Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude

Re-reviewed after latest commit (22efeba). The base-currency surcharge bug on Model/Total/Surcharge.php (inline thread) remains unresolved — setBaseGrandTotal and setBaseTaxAmount still receive quote-currency values. No new critical issues found in the rest of the diff. Recommend holding merge until the base-currency fix is applied.

…ort order Surcharge line and chips now show net (excl. tax); surcharge tax flows into Magento's native Tax line via setTaxAmount. Total Collector skips when a non-Two payment method is selected (clears session too). Summary sort order set to 39 (between shipping at 30 and tax at 40). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude

Re-reviewed after latest commit (e2d51d2). The base-currency surcharge bug on Model/Total/Surcharge.php (inline thread) remains unresolved — setBaseGrandTotal and setBaseTaxAmount still receive quote-currency values. No new critical issues found in the rest of the diff. Recommend holding merge until the base-currency fix is applied.

Comma-separated storage format unchanged — all downstream consumers unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude bot reviewed Apr 15, 2026

View reviewed changes

claude bot reviewed Apr 16, 2026

View reviewed changes

ABN-298/feat: replace payment terms multiselect with checkboxes

0554aec

Comma-separated storage format unchanged — all downstream consumers unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ABN-298: Extended net terms, surcharge config, and dev tooling#82

ABN-298: Extended net terms, surcharge config, and dev tooling#82
dgjlindsay wants to merge 7 commits intomainfrom
doug/ABN-298-extended-net-terms

dgjlindsay commented Apr 15, 2026

Uh oh!

gemini-code-assist bot commented Apr 15, 2026

Uh oh!

claude bot left a comment

Uh oh!

claude bot left a comment

Uh oh!

claude bot Apr 15, 2026

Uh oh!

claude bot Apr 15, 2026

Uh oh!

claude bot Apr 15, 2026

Uh oh!

claude bot Apr 16, 2026

Uh oh!

claude bot left a comment

Uh oh!

claude bot left a comment

Uh oh!

claude bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

dgjlindsay commented Apr 15, 2026

Summary

Test plan

Uh oh!

gemini-code-assist bot commented Apr 15, 2026

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant