chore: sync staging → main - #341
Open
two-inc[bot] wants to merge 656 commits into
Open
Conversation
dgjlindsay
requested changes
Aug 14, 2026
dgjlindsay
left a comment
dgjlindsay
left a comment
Contributor
There was a problem hiding this comment.
Do not release until Doug says so
ComposeOrder hands this provider $payment->getOrder() - the in-memory object from the current placement transaction, never loaded through OrderRepositoryInterface. Amasty's own OrderRepository plugin only populates amextrafee_fee_amount/amextrafee_tax_amount on afterGet/ afterGetList, so it never ran on that object and the provider silently returned no line - reproduced live on staging, still rejected by checkout-api with the same net/tax mismatch this was meant to fix (Linear ABN-489). Reloading by entity ID forces Amasty's plugin to run; the order row is already persisted by the time ComposeOrder executes.
PHP requires an implementing class's parameter names to match the interface exactly, not just types - $entity vs the interface's $order was a fatal error at class-definition time.
A hand-written anonymous class implementing OrderRepositoryInterface has to match the interface's parameter types/names exactly, which differ across the Magento versions CI tests against and caused a fatal error either way. createMock() generates a compatible stub via reflection against whatever version is actually loaded.
This repo's unit test bootstrap stubs Magento\Sales\Model\Order as a plain data bag (Test/Stubs/SalesModels.php) that does not implement OrderInterface. repositoryReturning() was typed against the interface while every call site passes the stub Order model - a TypeError on every test that reached it.
The method already duck-types every caller via method_exists() rather than depending on OrderExtensionInterface's real (codegen'd) shape, so typing the return as that interface was stricter than the code actually needs - and broke the tests, whose fake extension-attributes objects have no reason to implement it either.
Reload order via repository so Amasty's extension attributes actually populate
Verified live on staging that PR #352's reload-via-repository fix still didn't work - same net/tax mismatch, fresh trace ID. Root cause: Magento\Sales\Model\Service\OrderService::place() calls $order->place() (which runs payment authorize(), i.e. ComposeOrder) BEFORE $orderRepository->save($order). The order ComposeOrder hands this provider has no entity_id yet, so "reload it via the repository" always returns nothing - there is no row to reload for anyone, Amasty's own OrderRepository plugin included. The order does have a quote_id, because the quote has already been converted. Amasty's own OrderRepository plugin reads its quote-fee table by quote_id for exactly this reason; this provider now does the same lookup, isolated in AmastyExtraFeeQuoteReader since amasty/module-extra-fee isn't a build dependency of this repo and the resource-model class can't be type-hinted without breaking DI compile for merchants who never installed it. Linear ABN-489.
Read Amasty's fee by quote_id instead of reloading the order
Order::__construct() declared feeLineProviderPool as ?FeeLineProviderPool = null. Magento's object manager auto-wiring does not construct an object for a nullable-defaulted constructor parameter unless etc/di.xml names it explicitly for the concrete class, which it never did for ComposeOrder/ComposeCapture/ComposeRefund — so every real request built an empty pool via the null-coalescing fallback, and every registered FeeLineProviderInterface (including AmastyExtraFee) silently never ran. Making the dependency required lets auto-wiring inject the real, di.xml-configured pool.
fix(ABN-489): stop the Amasty fee provider pool being silently skipped
Replaces the standalone "<brand> Details" order-id row with a row in Magento's standard Payment Information block, labelled "<brand> order id" via BrandRegistryInterface. Every brand (base Two, ABN overlay, future overlays) gets its own label for free through the existing brand-resolution mechanism — no per-brand override needed. Invoice/ Credit Note download links stay in the existing section, which is otherwise unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…axes Magento copies the quote address's applied_taxes total data onto the order's own extension attributes during quote-to-order conversion, before Order::place() ever runs. Any total-collector extension that registers its tax with Magento's tax engine (Amasty's Extra Fee included) shows up there, so Order::findVerifiedResidualTaxRate() can verify a taxed residual against a rate Magento itself already applied, with no vendor-specific code and no entity_id needed. This supersedes the AmastyExtraFee/AmastyExtraFeeQuoteReader provider added in PR #353/#354 for reconciling Amasty's fee specifically — the generic mechanism covers it (and any other extension that integrates with Magento's tax engine the same way) without a per-vendor adapter. FeeLineProviderInterface remains for the narrower case of an extension that computes its own tax outside Magento's tax engine.
…liation fix(ABN-489): reconcile third-party fee tax generically via applied_taxes
findVerifiedResidualTaxRate() only handled applied_taxes entries as plain arrays. Confirmed live that this is only true briefly, right after ToOrderConverter::afterConvert() sets it: QuoteManagement:: submitQuote() immediately re-merges that converted order into a fresh one via DataObjectHelper::mergeDataObjects(), which rehydrates the array into Magento\Tax\Model\Sales\Order\Tax objects -- the shape ComposeOrder's $entity actually carries. The array-only check silently returned null for every real order, so the mechanism never fired live despite passing CI and unit tests (which only exercised the array shape). Handles both shapes now, with tests pinning each.
…shape fix(ABN-489): handle both applied_taxes shapes in tax reconciliation
FeeLineProviderPool's constructor docblock still described the inline empty-pool default in Order.php that PR #354 removed. Also documents that findVerifiedResidualTaxRate() matches a residual by amount, not identity: the first applied rate whose implied tax reconciles the residual wins, with no proof that rate specifically produced this residual. Narrow in practice (would need two applied rates to coincide on the same net/tax split), and the emitted gross/net/tax amounts stay correct regardless -- only the reported tax_rate/tax_class_name label could be wrong. Found by an independent adversarial review of #354/#356/#357.
docs(ABN-489): fix stale docblock and note rate-matching ambiguity
Aligns the Magento admin label with the cross-plugin name. The config path stays `enable_tax_subtotals` — renaming it would need a data migration for no behavioural gain, and WooCommerce/PrestaShop keep the same key. Adds the unit coverage the aggregation never had: the config gate, the per-rate grouping, and the 6-dp rate precision Two's gross_amount reconciliation depends on.
Four financial-correctness gaps against the WooCommerce/PrestaShop behaviour: - A payment term the buyer selected but the merchant no longer offers was silently swapped for the default at final placement. It now fails the same way the chip-click endpoint already did, through one shared availability check. - The shipping line's tax_rate was computed as tax/net. It is now the rate the store's tax engine declared for that line, matching product lines. - Every composed line's declared tax must follow from its own declared rate and net within 0.02 currency units, or the checkout is declined with a generic notice. - A new "Default Shipping Tax Rate" setting covers a taxed shipping line the tax engine declares no rate for. Unset by default, and unset means refuse the order rather than assume a rate.
9.x is EOL. 10.5 is the latest line still compatible with the pinned php:8.1-cli image (11.x requires PHP 8.2+). All 766 tests pass; 11 non-fatal deprecation notices surfaced, no failures.
…5503) Batch 3 of the cross-platform parity work — admin and config-surface items. - Sole-trader autofill is reached ONLY by clicking the chip. The email-driven prefetch and its auto-adopt are gone; the chip click mints tokens, resolves the buyer and either adopts them or opens signup. - Address autofill is gated on "Autofill company address" alone, never on where the company-search control is mounted. A merchant running search in the payment tile previously got no autofill at all. - The API key field verifies as it is typed (debounced) and, on save, a key the API definitively rejects aborts the save instead of replacing a working one. - The fulfilment-status mapping says out loud that an empty mapping means Two is never notified of fulfilment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Batch 4 of the WooCommerce/PrestaShop parity sweep — checkout and admin UX. - The selected payment term is cross-checked against the live term set immediately before submit, instead of posting a term the API will refuse. - Four order-lifecycle events (created/completed/cancelled/refunded) are now dispatched for downstream integrations; the plugin previously only subscribed to core events. - A missing FX rate for the surcharge withdraws the payment method and refuses placement, rather than throwing out of the totals collector and making checkout unrecoverable. - Enabling percentage mode scans stored limits for terms the grid does not render, so a stale zero cap surfaces then rather than silently clamping the fee later. - The minimum-order help text gets its own wording for a platform floor that cannot be projected into the store base currency. - Company-search "unavailable" copy matches the sibling plugins. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Aligns the payment tile with WooCommerce and PrestaShop: one control with Registered Organisation / Sole Trader / Enter manually as peer chips, replacing a two-item tab bar plus a separately worded "My company is not on the list" button that made manual entry reachable only by opening the search picker and rejecting its results. Presentation and routing only — each mode's own behaviour is untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Encrypted::beforeSave() (the parent) has no return statement, so `return parent::beforeSave();` used a void result. Split into a statement call + bare return, matching the parent's own contract.
At order composition the order is not yet persisted — ComposeOrder runs from Two::authorize() inside Order::place(), before the repository save — so it has no entity id and no sales_order_tax_item rows. Reading the rate through OrderTaxManagementInterface therefore always came back empty at checkout, and every taxed-shipping order either fell back to the static default rate or was refused outright. Read the order's own item_applied_taxes extension attribute first, which quote->order conversion populates in memory, and keep the persisted read for the post-save consumers (capture, refund).
…WO-25503) A flat 0.02 reconciliation tolerance assumes one rounding, on the line, and tax computed on the same base as net_amount. Two supported Magento configurations break both assumptions: the "Unit Price" algorithm rounds per unit and sums, so the residual grows with quantity, and "Before Discount" tax calculation taxes the undiscounted base. Both were common enough to decline ordinary checkouts. Scale the tolerance with quantity and accept net + discount as an alternative base. Separately, the API key backend model verified a candidate against the COMMITTED mode, so switching sandbox -> production and pasting the production key in one save checked the new key against the old environment, took a 401 and failed the whole section save. Verify against the mode submitted in the same request.
…e (TWO-25503) Service\Order::$feeLineProviderPool and FeeLineProviderPool::$logRepository are optional constructor arguments, which the object manager leaves at null, so the registered Amasty Extra Fee provider never fired and the pool's own rejections were never logged. Named in etc/di.xml, verified with dev:di:info. The order payload's payment term comes from additionalData while the surcharge is priced off the session term /select-term writes, and nothing checked the two agreed — a /select-term call that failed mid-flow could place an order on one term carrying the other's fee. Cross-check them at composition, and refuse a non-numeric term rather than letting it cast to the default. A second sole-trader chip click landing while the first lookup was still in flight hit the dedupe key and resumed immediately on a lookup that had minted nothing: no adoption, no popup, and no fallback link either. Return the outstanding chain instead.
LifecycleEventDispatcher deduped on the entity id, which is null during placement, so under multishipping every order after the first was deduped away and two_payment_order_created never fired for them. Fall back to the increment id. Also: guard the fulfilled-order id read like its sibling key; re-key the nb/nl/sv rows for the two reworded admin comments and add the API-key spinner string; and correct comments that no longer describe what the code does — the surcharge FX gate fails open and relies on the placement backstop, openDropdown is threaded through rather than set here, and the totals collector's throw came out of the calculator's own conversion.
…503) Final adversarial review round found: - ApiKeyCheck::getScope()/getScopeId() read the element's own form (the fieldset, which carries no scope) instead of the config Form block the renderer is actually bound to at render time — the live check silently verified against the default scope's environment regardless of which store view was open. Same false-confidence class the save guard was fixed for. - Reconciliation log message reported the "before discount" residual against the wrong base. - lookupSoleTrader() could hand an empty-email call an unrelated in-flight chain's promise.
feat(TWO-25554): give the billing panel its own company number, sole-trader link and country ordering
The billing panel's capture outlived billing being a distinct address, so every quote billing-address notification routed a returning buyer's saved company into a panel the resolver does not read. Core's own checkbox is what retires that capture. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…re-point The tile kept a visible, required company field on the one event that moves the mount, because the renderer's revision counter was bumped only by callers that remembered to. The harness's Knockout double now tracks dependencies, so a binding's caching — and a missing notification — is assertable at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A rebuilt address fieldset takes every autofill marker with it, so a later country switch retracted nothing and left the previous country's address in the buyer's form. Keyed on the form element and read only for the form the calling panel passed in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rm there is A checkout supplying its own address markup left the pick and the sole-trader write-back with nowhere to land, silently. The destination is that single form, established before the write and refused — audibly — when it cannot be. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A panel whose mount went away kept the field's key handlers, its popover and its combobox attributes, so the control the buyer left stayed live under the one they were using. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A scope-less caller was silently given the bind token, which a re-render replaces — so the 429 backoff it earned expired with it. The panel now names itself when its host does not, and the model refuses a search with neither. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ghten two pins Jest ignores a second argument to `toBe()`, so nine rows carried a description nothing would ever print. The address step's own pin now checks an allowlist of the members it may reach for rather than the names of the writers already gone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(TWO-25554): round-5 defects in the billing/shipping panel split
The ancestor walk had no ceiling, so a panel whose own form carries no company_id read the other panel's — a cross-panel DOM read the panel split exists to prevent. The walk now cannot leave the form the field belongs to. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The number label and sole-trader link sat inside `.two-company-field-wrap`, which the popover is positioned against, so chrome in its flow pushed the open dropdown off the field. They are siblings after the wrapper, and a test pins that. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The injection matched only core's own billing-address component, so a checkout substituting its own — or wrapping core's in a container — got no company-number field and the feature silently did nothing there. Core's `billingAddress` scope naming is accepted as well. Also assert the payment subtree in the neither-container-present row, which previously asserted nothing about its own subject. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ity-followup fix(TWO-25554): scope each capture panel's DOM reads and chrome to its own form
Checkout Fields/Order Management/Diagnostics tab headings and every field under Order Management/Diagnostics missing tax-class defaults, debug/SSL/ admin-controls and health-checklist labels had no CSV row in any locale, matching the same gap in woocommerce-plugin (TWO-i18n). Order Management/Diagnostics/Checkout Fields headings are ported from prestashop-plugin's existing human translations; Enable debug logging and Disable SSL verification are ported from woocommerce-plugin's. The rest is machine-translated and unreviewed.
i18n: translate Checkout Fields/Order Management/Diagnostics labels
TWO-25498 re-introduced Title Case on captions that TWO-25326 had sentence-cased, which also orphaned the sentence-case i18n rows still carrying their human translations. Renames the catalogue keys with the captions so those translations follow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
45 rows per locale, mostly recovered from the sibling plugins' existing human translations. Also corrects nb/sv "Health", which rendered as bodily health, and a "brower" typo in the firewall-token help. The new gate asserts every admin caption has a non-empty row in all three locales and that no caption is Title Case — this is the second round of the same defect, so it needs a check rather than another sweep. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Checklist values, grid headers, dropdown options and validation refusals reach the screen from PHP rather than a form definition, so the XML-only gate would not have caught the round this PR fixes. Concatenated msgids are joined the way Magento looks them up. Also realigns prose that quotes a live label with that label's casing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cating it Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both assertions pinned the strings this PR corrects. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: sentence-case admin captions and close the admin translation gaps
Base-plugin unit tests used a specific partner overlay's product name as a payment-title fixture. Replaced with a generic placeholder — the tests only verify title rendering, not any real brand string.
…-fixtures Remove partner brand reference from test fixtures
Every buyer- and merchant-facing surcharge amount was rendered net regardless of configuration, so a store set to show gross prices showed a net surcharge beside gross everything else. Net and tax were already snapshotted at each entity's creation time, so this is display-only: Service\Order\SurchargeDisplay reads the store's "Display Prices" switch for the cart and sales tiers and each renderer picks accordingly. "Both" emits the paired excl/incl rows core uses for Subtotal and Shipping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per-term failure logging moved into TermSurchargePreview, and pin the predicate core's TotalsReader uses to split a multi-row fetch(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: show the surcharge net or gross per the store's tax display setting
displayedTermSurcharges() picked gross whenever mode was not 'excl', so Both mode (which shows both rows in the order-summary total) fed gross into the single-value chip too. Only 'incl' should select gross; 'excl' and 'both' now both resolve to net.
Fix: chip shows net in Both surcharge display mode
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated rolling sync PR opened by
.github/workflows/auto-pr.yml.Merges everything currently on
stagingintomain. Auto-updates as new commits land onstaging. Close manually if you need to skip a sync window.