From c7a51745749b237a5bef19016cdc037b641aa180 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 15:22:27 +0100 Subject: [PATCH 01/30] fix(TWO-25554): bound each panel's restored-number read to its own form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Test/Js/company-panel-chrome.test.js | 40 ++++++++++++++++++- .../web/js/model/company-capture-component.js | 28 +++++++++++-- view/frontend/web/js/model/company-capture.js | 2 + 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index 314c2e74..07c2c1d5 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -94,7 +94,7 @@ function renderCheckout(options) { '
' + '' + `
` + - addressFields('NO', options.billingNumber) + + addressFields('NO', options.billingNumber, options.billingCompanyIdField) + '
' + extraBilling + '
' + @@ -177,6 +177,16 @@ function picks(panel, item) { panel.selectCompany(item); } +/** + * One macrotask. `watchCapturedIdentity` publishes on `setTimeout(0)`, so a + * negative assertion made before it has run is vacuous. + * + * @returns {Promise} + */ +function flushCapture() { + return new Promise(function (resolve) { setTimeout(resolve, 0); }); +} + /** @returns {Array} the number labels rendered inside one panel's form */ function numbersIn(which) { return Array.prototype.map.call( @@ -265,6 +275,34 @@ describe('the company number is painted under its own panel\'s field', () => { expect(document.querySelectorAll(`.${NUMBER_CLASS}`)).toHaveLength(0); }); + test.each([ + [ + 'shipping', + { shippingCompanyIdField: false, billingNumber: '555555555' }, + 'the shipping form has no number field and one billing neighbour has one' + ], + [ + 'billing', + { billingCompanyIdField: false, shippingNumber: '777777777' }, + 'the billing form has no number field and one shipping neighbour has one' + ] + ])('%s claims nothing from the single neighbour holding a number (%s)', async (actor, fixture) => { + const other = OTHER[actor]; + const restored = { shipping: fixture.shippingNumber, billing: fixture.billingNumber }; + const { panels } = boot(fixture); + expect(panels[actor].mountSelector()).toBe(FIELDS[actor]); + + // `watchCapturedIdentity` publishes on a macrotask, so an assertion made + // before it has run denies a propagation that had not happened yet. + await flushCapture(); + await flushCapture(); + + expect(panels[actor].displayCompanyNumber()).toBe(''); + expect(numbersIn(actor)).toEqual([]); + // The neighbour still paints the number in its OWN field. + expect(numbersIn(other)).toEqual([restored[other]]); + }); + test('a panel whose own form carries no number field claims neither neighbour\'s', () => { // Two billing fieldsets, a number restored into each. The shipping form // has no number field, so the nearest ancestor holding any spans both — diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index 70e08ae1..82e2812e 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -113,6 +113,9 @@ * `refreshMount()` off its own re-render hook instead. * @param {string} options.addressFieldSelector the address step's company * field. + * @param {string} options.addressFormRootSelector the form that field + * belongs to — the one `addressFieldSelector` is built from. Bounds + * every DOM read this panel makes, so absent it reads nothing. * @param {string} options.tileFieldSelector the payment tile's company * field. * @param {function(string): boolean} options.fieldExists whether a selector @@ -584,8 +587,8 @@ /** * A company number a reload restored into this panel's own form. * - * Read rather than written into the identity, which would newly let a - * restored company drive order intent. + * Read rather than written into the identity, which would let a restored + * company drive order intent. * * @returns {string} */ @@ -595,8 +598,10 @@ if (this._boundSelector === this._options.tileFieldSelector) return ''; const field = this.fieldNode(); if (!field) return ''; + const root = this._ownFormRoot(field); + if (!root) return ''; let node = field.parentElement; - while (node) { + while (node && root.contains(node)) { const found = node.querySelectorAll(RESTORED_NUMBER_SELECTOR); // Exactly one: several under one ancestor means it spans a second // address form, so neither is answerable as this panel's own. @@ -606,6 +611,23 @@ return ''; }; + /** + * The form this panel's field belongs to — the ceiling on every DOM read it + * makes, so no read can reach the other panel's form. + * + * `closest` off the field, not a page-wide query for the root: the billing + * root selector matches per-payment-method fieldsets, and the one holding + * THIS field is the only one that is this panel's own. + * + * @param {Element} field + * @returns {?Element} + */ + CompanyCaptureComponent.prototype._ownFormRoot = function (field) { + const selector = this._options.addressFormRootSelector; + if (!selector) return null; + return field.closest(selector); + }; + /** * Paint the company number as plain text under this panel's field. * diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index 0b75a713..1defa597 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -350,6 +350,7 @@ define([ shippingComponent = new CompanyCaptureComponent(Object.assign(sharedHostOptions(), { identity: shippingIdentity, addressFieldSelector: ADDRESS_FIELD_SELECTOR, + addressFormRootSelector: ADDRESS_FORM_ROOT, tileFieldSelector: TILE_FIELD_SELECTOR, fieldExists: function (selector) { if (selector === TILE_FIELD_SELECTOR && !tileIsShippingPanels()) return false; @@ -388,6 +389,7 @@ define([ billingComponent = new CompanyCaptureComponent(Object.assign(sharedHostOptions(), { identity: billingIdentity, addressFieldSelector: BILLING_FIELD_SELECTOR, + addressFormRootSelector: BILLING_FORM_ROOT, // Never falls back to the tile: the tile is the shipping/no-address- // form mount's own fallback, and only one control may ever bind there. tileFieldSelector: '', From 4402875f378eca54f8ba4a09bad903cfd51bb1ab Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 15:22:47 +0100 Subject: [PATCH 02/30] fix(TWO-25554): anchor panel chrome beside the popover's wrapper 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 --- Test/Js/company-panel-chrome.test.js | 43 +++++++++++++++++ view/frontend/web/css/style.css | 4 +- .../web/js/model/company-capture-component.js | 46 ++++++++++++++++--- 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index 07c2c1d5..e07cc873 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -319,6 +319,49 @@ describe('the company number is painted under its own panel\'s field', () => { }); }); +describe('chrome never enters the popover\'s positioning context', () => { + /* + * `.two-company-dropdown` is `position: absolute; top: 100%` against + * `.two-company-field-wrap` (css/style.css), so anything in the wrap's flow + * moves the popover off the field by its own height. jsdom has no layout, so + * the constraint is pinned where it is decided: chrome is a following + * SIBLING of the wrap and never a descendant. + */ + /** A sole trader the registry DOES hold a public number for, so both + * pieces of chrome are on the page at once. */ + const NUMBERED_TRADER = { company_name: 'Numbered Trader', organization_number: '333333333' }; + + test.each(DIRECTIONS)('%s renders both pieces of chrome (%s)', (actor) => { + const { panels } = boot(); + + panels[actor].adoptSoleTrader(NUMBERED_TRADER); + + const wrap = document.querySelector(`${FORMS[actor]} .two-company-field-wrap`); + expect(wrap).not.toBeNull(); + expect(wrap.querySelectorAll(`.${NUMBER_CLASS}, .${LINK_CLASS}`)).toHaveLength(0); + + const number = document.querySelector(`${FORMS[actor]} .${NUMBER_CLASS}`); + const link = document.querySelector(`${FORMS[actor]} .${LINK_CLASS}`); + expect(number.parentElement).toBe(wrap.parentElement); + expect(link.parentElement).toBe(wrap.parentElement); + expect(wrap.nextElementSibling).toBe(number); + expect(number.nextElementSibling).toBe(link); + }); + + test.each(DIRECTIONS)('%s keeps that order across a repaint (%s)', (actor) => { + const { panels } = boot(); + panels[actor].adoptSoleTrader(NUMBERED_TRADER); + + panels[actor].renderChrome(); + + const wrap = document.querySelector(`${FORMS[actor]} .two-company-field-wrap`); + expect(wrap.querySelectorAll(`.${NUMBER_CLASS}, .${LINK_CLASS}`)).toHaveLength(0); + expect(wrap.nextElementSibling.classList.contains(NUMBER_CLASS)).toBe(true); + expect(numbersIn(actor)).toHaveLength(1); + expect(linksIn(actor)).toBe(1); + }); +}); + describe('the "select a different sole trader" link belongs to its own panel', () => { test.each(DIRECTIONS)('%s adopts a sole trader (%s)', (actor) => { const other = OTHER[actor]; diff --git a/view/frontend/web/css/style.css b/view/frontend/web/css/style.css index 0b3c0256..38c442d0 100755 --- a/view/frontend/web/css/style.css +++ b/view/frontend/web/css/style.css @@ -489,8 +489,8 @@ /* * TWO-25326 §5, 2026-08-04 ruling, Bug B. `text-align: end` rather than - * `right` so it follows the writing direction on RTL store views; the panel's - * wrapper spans the input's containing box, so its end edge lines up exactly. + * `right` so it follows the writing direction on RTL store views; this block + * spans the input's containing box, so its end edge lines up exactly. */ .two-company-id-text { display: block; diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index 82e2812e..c696bd79 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -556,11 +556,38 @@ * * @returns {?Element} */ - CompanyCaptureComponent.prototype._chromeHost = function () { + CompanyCaptureComponent.prototype._chromeAnchor = function () { const field = this.fieldNode(); return (field && field.parentElement) || null; }; + /** + * Where this panel's chrome lives: alongside the wrapper, never within it. + * The popover is positioned off the wrapper's own box (`top: 100%`), so + * chrome inside it pushes the popover off the field by the chrome's height. + * + * @returns {?Element} + */ + CompanyCaptureComponent.prototype._chromeHost = function () { + const anchor = this._chromeAnchor(); + return (anchor && anchor.parentElement) || null; + }; + + /** + * A piece of this panel's chrome, matched only among the wrapper's own + * siblings so the lookup cannot descend into the popover or another form. + * + * @param {string} className + * @returns {?Element} + */ + CompanyCaptureComponent.prototype._chromeNode = function (className) { + const host = this._chromeHost(); + if (!host) return null; + return Array.prototype.find.call(host.children, function (child) { + return child.classList.contains(className); + }) || null; + }; + /** Repaint every piece of chrome this panel renders for its own identity. */ CompanyCaptureComponent.prototype.renderChrome = function () { this.renderCompanyNumber(); @@ -636,9 +663,10 @@ * unreadable to a screen reader. */ CompanyCaptureComponent.prototype.renderCompanyNumber = function () { + const anchor = this._chromeAnchor(); const host = this._chromeHost(); - if (!host) return; - const existing = host.querySelector('.' + COMPANY_NUMBER_CLASS); + if (!anchor || !host) return; + const existing = this._chromeNode(COMPANY_NUMBER_CLASS); if (existing) existing.remove(); const number = this.displayCompanyNumber(); if (!number) return; @@ -646,7 +674,7 @@ label.className = COMPANY_NUMBER_CLASS; label.setAttribute('aria-label', this.translate('Company Number')); label.textContent = number; - host.appendChild(label); + host.insertBefore(label, anchor.nextSibling); }; /** @@ -656,9 +684,10 @@ * (TWO-25461 §7). */ CompanyCaptureComponent.prototype.renderSoleTraderLink = function () { + const anchor = this._chromeAnchor(); const host = this._chromeHost(); - if (!host) return; - const existing = host.querySelector('.' + SOLE_TRADER_LINK_CLASS); + if (!anchor || !host) return; + const existing = this._chromeNode(SOLE_TRADER_LINK_CLASS); const adopted = this._identity.soleTraderAdopted(); if (!adopted) { if (existing) existing.remove(); @@ -678,7 +707,10 @@ self._soleTrader.selectDifferentSoleTrader(); }); wrapper.appendChild(link); - host.appendChild(wrapper); + // After the number when there is one, so the two keep a stable order + // across repaints regardless of which was rendered first. + const after = this._chromeNode(COMPANY_NUMBER_CLASS) || anchor; + host.insertBefore(wrapper, after.nextSibling); }; // ----------------------------------------------------------------- chips From 8f3831e395441f4e229ad0d859bf8f3271b898d2 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 15:22:58 +0100 Subject: [PATCH 03/30] fix(TWO-25554): fill billing forms a third-party component renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Model/Checkout/LayoutProcessorPlugin.php | 62 +++++++++---- .../Checkout/LayoutProcessorPluginTest.php | 92 ++++++++++++++++++- 2 files changed, 134 insertions(+), 20 deletions(-) diff --git a/Plugin/Model/Checkout/LayoutProcessorPlugin.php b/Plugin/Model/Checkout/LayoutProcessorPlugin.php index 29070699..e28ffdce 100755 --- a/Plugin/Model/Checkout/LayoutProcessorPlugin.php +++ b/Plugin/Model/Checkout/LayoutProcessorPlugin.php @@ -12,10 +12,10 @@ class LayoutProcessorPlugin { /** - * Core's own billing-address component. Every billing address form is one - * of these, whichever container it was generated into, so matching on it - * is what lets both *Display Billing Address On* settings and every - * payment method code be covered without naming any of them. + * Core's own billing-address component — one of the two things that + * identify a billing form, so neither the *Display Billing Address On* + * setting nor any payment method code has to be named. See + * `isBillingAddressForm()` for the other. */ private const BILLING_ADDRESS_COMPONENT = 'Magento_Checkout/js/view/billing-address'; @@ -161,24 +161,54 @@ private function processBillingFieldsets(array &$jsLayout) private function processBillingForms(array &$container) { foreach ($container as &$node) { - if (!is_array($node) - || ($node['component'] ?? null) !== self::BILLING_ADDRESS_COMPONENT - || !isset($node['dataScopePrefix']) - || !is_string($node['dataScopePrefix']) - || $node['dataScopePrefix'] === '' - || !isset($node['children']['form-fields']['children']) - || !is_array($node['children']['form-fields']['children']) - ) { + if (!is_array($node)) { + continue; + } + if ($this->isBillingAddressForm($node)) { + $fieldset = &$node['children']['form-fields']['children']; + $fieldset['company_id'] = $this->companyIdField($node['dataScopePrefix']); + $this->moveCountryBeforeCompany($fieldset); + unset($fieldset); continue; } - $fieldset = &$node['children']['form-fields']['children']; - $fieldset['company_id'] = $this->companyIdField($node['dataScopePrefix']); - $this->moveCountryBeforeCompany($fieldset); - unset($fieldset); + // A checkout that wraps the billing form in a container of its own + // puts it below this level, and a form never nests inside a form. + if (isset($node['children']) && is_array($node['children'])) { + $this->processBillingForms($node['children']); + } } unset($node); } + /** + * Whether one layout node is a billing address form this plugin can fill. + * + * Core's component OR core's `billingAddress` scope naming, because a + * checkout that substitutes its own billing-address component still binds + * it to that scope — the field's `dataScope` is what makes the number + * submit with the right address, so a node that does not carry that scope + * is not a billing form whatever else it looks like. The scope test alone + * would also admit the shipping fieldset; it is reached from a different + * path and never appears under these containers. + * + * @param array $node + * @return bool + */ + private function isBillingAddressForm(array $node): bool + { + if (!isset($node['dataScopePrefix']) + || !is_string($node['dataScopePrefix']) + || !isset($node['children']['form-fields']['children']) + || !is_array($node['children']['form-fields']['children']) + ) { + return false; + } + + return ($node['component'] ?? null) === self::BILLING_ADDRESS_COMPONENT + ? $node['dataScopePrefix'] !== '' + : strpos($node['dataScopePrefix'], 'billingAddress') === 0; + } + /** * Force `country_id` to render before the native `company` AND `street` * fields of one address fieldset, only when it does not already. diff --git a/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php b/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php index 9c15fb56..db01b828 100644 --- a/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php +++ b/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php @@ -452,7 +452,7 @@ private function process(array $jsLayout, bool $isActive = true): array * (*Display Billing Address On*), and a form is matched by its component * rather than by a path or a method code. * - * @return array, 1: array, 2: array, 3: string}> + * @return array, 1: array, 2: array, 3: string}> */ public static function billingContainerProvider(): array { @@ -492,12 +492,96 @@ public function testCompanyNumberFieldIsInjectedIntoEveryBillingForm( // the real form rather than a path the plugin conjured. $this->assertSame(['company', 'company_id'], array_keys($fields), $description); } + // Nowhere else in the payment subtree — with neither container present + // that is the only thing this row has to say. + $this->assertCount( + count($expected), + $this->injectedFields($this->paymentChildren($jsLayout)), + $description + ); // The shipping injection is unaffected by the billing walk. $this->assertArrayHasKey('company_id', $this->fieldset($jsLayout), $description); } /** - * @return array + * @param array $jsLayout + * @return array the payment step's own `children` + */ + private function paymentChildren(array $jsLayout): array + { + return $jsLayout['components']['checkout']['children']['steps']['children']['billing-step'] + ['children']['payment']['children']; + } + + /** + * @param array $subtree + * @return array every `company_id` this plugin injected below it + */ + private function injectedFields(array $subtree): array + { + $found = []; + foreach ($subtree as $key => $value) { + if (!is_array($value)) { + continue; + } + if ($key === 'company_id') { + $found[] = $value; + continue; + } + $found = array_merge($found, $this->injectedFields($value)); + } + + return $found; + } + + /** + * A checkout that renders billing with its own component, or wraps core's + * in a container of its own, still binds the form to core's + * `billingAddress` scope — and the number has to submit with that address. + * + * @return array, 1: string, 2: string}> + */ + public static function thirdPartyBillingProvider(): array + { + $ownComponent = [ + 'component' => 'Vendor_Checkout/js/view/billing-address', + 'dataScopePrefix' => 'billingAddressvendor_method', + 'children' => ['form-fields' => ['children' => ['company' => ['label' => 'Company']]]], + ]; + + return [ + [$ownComponent, 'billingAddressvendor_method', 'a third-party billing component'], + [ + ['component' => 'Vendor_Checkout/js/view/wrapper', 'children' => ['inner' => $ownComponent]], + 'billingAddressvendor_method', + 'core\'s form wrapped in a third-party container', + ], + ]; + } + + /** + * @dataProvider thirdPartyBillingProvider + * @param array $node + */ + public function testThirdPartyBillingFormsAreFilledToo( + array $node, + string $scopePrefix, + string $description + ): void { + $jsLayout = $this->process($this->seededLayoutWithBilling(['billing-node' => $node])); + + $injected = $this->injectedFields($this->paymentChildren($jsLayout)); + + $this->assertCount(1, $injected, $description); + $this->assertSame( + $scopePrefix . '.custom_attributes.company_id', + $injected[0]['dataScope'], + $description + ); + } + + /** + * @return array */ public static function billingScopeProvider(): array { @@ -530,7 +614,7 @@ public function testBillingScopesAreDerivedFromTheFormsOwnDataScopePrefix( * this module at all, so the stock EAV order reproduces the mis-ordering on * every store. * - * @return array, 1: mixed, 2: string}> + * @return array, 1: mixed, 2: string}> */ public static function billingReorderProvider(): array { @@ -570,7 +654,7 @@ public function testBillingCountrySortsBeforeCompanyAndStreet( } /** - * @return array, 1: string}> + * @return array, 1: string}> */ public static function untouchedBillingNodeProvider(): array { From 7659889e55fd3f167b357ebe5349afed187f27b9 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 15:23:12 +0100 Subject: [PATCH 04/30] docs(TWO-25554): drop a pointer comment to another test file Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-independence.test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 2e28227f..f1c62814 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -355,9 +355,6 @@ describe('a sole-trader adoption lands on one panel only', () => { expect(displayed(other)).toBe(''); expect(organisationNumber(other)).toBe(''); }); - - // Which panel's own "select a different sole trader" link is rendered, and - // whose flow a click on it reaches, is company-panel-chrome.test.js. }); describe('a country switch invalidates its own panel\'s company and nothing else', () => { From 31190bcae332100a78dbe327563ab9b9a810d90a Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 15:58:52 +0100 Subject: [PATCH 05/30] fix(TWO-25554): take a panel's chrome off the page with its mount Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-chrome.test.js | 43 +++++++++++++++++++ .../web/js/model/company-capture-component.js | 17 ++++++++ 2 files changed, 60 insertions(+) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index e07cc873..bb112aa2 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -401,3 +401,46 @@ describe('the "select a different sole trader" link belongs to its own panel', ( expect(soleTraderCalls).toEqual([panels[actor]]); }); }); + +describe('a panel that loses its mount leaves no chrome behind', () => { + const WRAP_CLASS = 'two-company-field-wrap'; + /** Numbered, so both pieces of chrome are on the page at once. */ + const TRADER = { company_name: 'Billing Trader', organization_number: '333333333' }; + + /** What re-checking "same as shipping" leaves: the fieldset, hidden. */ + function hideBillingFieldset() { + document.querySelector(FORMS.billing).setAttribute('data-test-hidden', ''); + } + + test('the billing form keeps neither the number nor a link to a torn-down flow', async () => { + const { panels, soleTraderCalls } = boot(); + panels.billing.adoptSoleTrader(TRADER); + expect(linksIn('billing')).toBe(1); + expect(numbersIn('billing')).toHaveLength(1); + + hideBillingFieldset(); + panels.billing.refreshMount(); + await flushCapture(); + await flushCapture(); + + const form = document.querySelector(FORMS.billing); + expect(form.querySelectorAll(`.${WRAP_CLASS}`)).toHaveLength(0); + expect(numbersIn('billing')).toEqual([]); + expect(linksIn('billing')).toBe(0); + expect(soleTraderCalls).toEqual([]); + }); + + test('the shipping panel keeps its own chrome through the other\'s loss', async () => { + const { panels } = boot(); + panels.shipping.adoptSoleTrader(TRADER); + panels.billing.adoptSoleTrader(TRADER); + + hideBillingFieldset(); + panels.billing.refreshMount(); + await flushCapture(); + await flushCapture(); + + expect(numbersIn('shipping')).toHaveLength(1); + expect(linksIn('shipping')).toBe(1); + }); +}); diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index c696bd79..739cec4c 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -445,6 +445,9 @@ const selector = this.mountSelector(); const previous = this._boundSelector; if (!selector) { + // Before the selector is forgotten: every chrome lookup is made + // through the bound field. + this._removeChrome(); // Neither host is on the page any more. Forgetting where the control // was is what stops `adjacentCountry()` answering for a form that has // gone, and lets the next host that appears mount cleanly. @@ -594,6 +597,20 @@ this.renderSoleTraderLink(); }; + /** + * Take this panel's chrome back off the page. + * + * The chrome is a SIBLING of the wrapper, so `unmount()` does not carry it + * away, and its button drives a flow the same call tears down (TWO-25554). + */ + CompanyCaptureComponent.prototype._removeChrome = function () { + const self = this; + [COMPANY_NUMBER_CLASS, SOLE_TRADER_LINK_CLASS].forEach(function (className) { + const node = self._chromeNode(className); + if (node) node.remove(); + }); + }; + /** * The captured company number as it may be SHOWN, or '' for nothing to * show. Manual entry is name-only capture, so a number shown there would From 5b928749535edbd47a7ebb54ece0ec26d3685e27 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 15:59:38 +0100 Subject: [PATCH 06/30] fix(TWO-25554): retire the mode and the adoption with the capture Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-capture-billing-panel.test.js | 4 ++++ .../web/js/model/company-capture-component.js | 5 ++++- .../frontend/web/js/model/company-identity.js | 20 +++++++++++++++++-- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Test/Js/company-capture-billing-panel.test.js b/Test/Js/company-capture-billing-panel.test.js index ffc40386..cc12e3dd 100644 --- a/Test/Js/company-capture-billing-panel.test.js +++ b/Test/Js/company-capture-billing-panel.test.js @@ -545,11 +545,15 @@ describe('the quote\'s billing address seeds the panel owning the billing role', test('re-checking "same as shipping" retires the billing panel\'s own capture', () => { const { capture, dom } = load(); billingPicks(capture, dom, 'Billing Co'); + capture.billing.identity().soleTraderAdopted(true); + capture.billing.identity().captureMode('soletrader'); sameAsShippingAgain(capture, dom); expect(capture.billing.identity().companyName()).toBe(''); expect(capture.billing.identity().companyId()).toBe(''); + expect(capture.billing.identity().soleTraderAdopted()).toBe(false); + expect(capture.billing.identity().captureMode()).toBe('registered'); }); test('after that re-check the returning buyer\'s saved company seeds SHIPPING, not billing', () => { diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index 739cec4c..157ce378 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -307,10 +307,13 @@ // A search still on the wire would answer for the country the buyer // just left and repopulate what this call is clearing. if (this._panel) this._panel.abortActiveRequest(); + // Read before the retirement, which resets the mode itself: the + // panel still has to be handed back as a search trigger. + const wasSoleTrader = this._identity.isSoleTrader(); this._identity.clear(); this._options.revertAutofilledAddress(); this._soleTrader.forgetAdoptions(); - if (this._identity.isSoleTrader()) this.registeredMode(); + if (wasSoleTrader) this.registeredMode(); } this.refreshSoleTraderAvailability(country); }; diff --git a/view/frontend/web/js/model/company-identity.js b/view/frontend/web/js/model/company-identity.js index b8f4a604..466f0ea4 100644 --- a/view/frontend/web/js/model/company-identity.js +++ b/view/frontend/web/js/model/company-identity.js @@ -199,12 +199,28 @@ notify(); }, - /** Abandon both halves — a country change invalidates the registry. */ + /** + * Retire the whole capture — a country change invalidates the + * registry, and re-checking "same as shipping" retires the panel. + * + * The mode and the adoption go with the pair: a retired capture + * left in sole-trader mode remounts the panel over an empty + * identity, and the resolver reads an adoption flag as a company + * number (TWO-25554). `soleTraderAvailable` is a property of the + * country, not of the capture, and stays. + */ clear: function () { - if (!state.companyName && !state.companyId && !state.companyIdSource) return; + if (!state.companyName && !state.companyId && !state.companyIdSource + && !state.soleTraderAdopted && !state.addressNotice + && state.captureMode === 'registered') { + return; + } state.companyName = ''; state.companyId = ''; state.companyIdSource = ''; + state.soleTraderAdopted = false; + state.addressNotice = ''; + state.captureMode = 'registered'; notify(); }, From 1201d589f495982ee347a165280192cf9e53ab67 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:02:23 +0100 Subject: [PATCH 07/30] fix(TWO-25554): route the quote's billing company on the quote's own answer Co-Authored-By: Claude Sonnet 5 --- Test/Js/amd-harness.js | 7 ++-- Test/Js/company-capture-billing-panel.test.js | 35 ++++++++++++++++--- Test/Js/company-field-display-scope.test.js | 3 +- Test/Js/company-panel-independence.test.js | 19 +++++----- .../gateway-method-company-selection.test.js | 10 +++--- view/frontend/web/js/model/company-capture.js | 9 +++-- .../payment/method-renderer/gateway_method.js | 19 +++++++++- 7 files changed, 77 insertions(+), 25 deletions(-) diff --git a/Test/Js/amd-harness.js b/Test/Js/amd-harness.js index 9b800224..af6c5433 100644 --- a/Test/Js/amd-harness.js +++ b/Test/Js/amd-harness.js @@ -84,8 +84,11 @@ function defaultMocks() { 'domReady!': null, 'Magento_Checkout/js/view/payment/default': Component, 'Magento_Checkout/js/model/quote': { - shippingAddress: makeObservable({}), - billingAddress: makeObservable({}), + // One cache key for both: the quote is what answers "is billing a + // distinct address", so a double with no key at all cannot model + // either answer. Same key means billing IS shipping. + shippingAddress: makeObservable({ getCacheKey: function () { return 'one-address'; } }), + billingAddress: makeObservable({ getCacheKey: function () { return 'one-address'; } }), getTotals: function () { return makeObservable({}); }, getQuoteId: function () { return null; }, paymentMethod: makeObservable(null), diff --git a/Test/Js/company-capture-billing-panel.test.js b/Test/Js/company-capture-billing-panel.test.js index cc12e3dd..a692dd9e 100644 --- a/Test/Js/company-capture-billing-panel.test.js +++ b/Test/Js/company-capture-billing-panel.test.js @@ -516,11 +516,18 @@ describe('the quote\'s billing address seeds the panel owning the billing role', expect(capture.billing.mountSelector()).toBe(''); } - function billingQuoteAddress(company) { + /** + * @param {string} company + * @param {string} [cacheKey] the quote's own answer to "is this a distinct + * address"; the harness quote's shipping key by default, i.e. not + * @returns {object} quote address + */ + function billingQuoteAddress(company, cacheKey) { return { company: company, telephone: '+47 123 45 678', - customAttributes: [{ attribute_code: 'company_id', value: '222' }] + customAttributes: [{ attribute_code: 'company_id', value: '222' }], + getCacheKey: function () { return cacheKey || 'one-address'; } }; } @@ -530,15 +537,33 @@ describe('the quote\'s billing address seeds the panel owning the billing role', dom.fireChange('input[name="billing-address-same-as-shipping"]'); } - test('through the quote\'s billing address, with the fieldset away it seeds SHIPPING', () => { + test('with the fieldset transiently away, a distinct billing address still seeds BILLING', () => { + // The quote answers "is billing a distinct address", not the fieldset's + // visibility: a third-party re-render that takes the fieldset away for a + // moment otherwise puts billing's company in the shipping panel's own + // field (TWO-25554). const { capture, dom } = load(); billingPicks(capture, dom, 'Billing Co'); const renderer = loadRenderer(capture, dom); billingFieldsetAway(dom, capture); - renderer.updateBillingAddress(billingQuoteAddress('Billing Co')); + renderer.updateBillingAddress(billingQuoteAddress('Billing Co', 'billing-of-its-own')); + + expect(capture.billing.identity().companyName()).toBe('Billing Co'); + expect(capture.billing.identity().companyId()).toBe('222'); + expect(capture.shipping.identity().companyName()).toBe(''); + expect(capture.shipping.identity().companyId()).toBe(''); + }); + + test('a billing address the quote says IS the shipping address seeds SHIPPING', () => { + const { capture, dom } = load(); + billingPicks(capture, dom, 'Billing Co'); + const renderer = loadRenderer(capture, dom); + billingFieldsetAway(dom, capture); + + renderer.updateBillingAddress(billingQuoteAddress('Saved Co')); - expect(capture.shipping.identity().companyName()).toBe('Billing Co'); + expect(capture.shipping.identity().companyName()).toBe('Saved Co'); expect(capture.shipping.identity().companyId()).toBe('222'); }); diff --git a/Test/Js/company-field-display-scope.test.js b/Test/Js/company-field-display-scope.test.js index a1207e45..b54f56b6 100644 --- a/Test/Js/company-field-display-scope.test.js +++ b/Test/Js/company-field-display-scope.test.js @@ -280,7 +280,8 @@ describe('a pick on one panel never paints the other panel\'s field', () => { renderer.updateBillingAddress({ company: 'Billing Co', telephone: '+47 123 45 678', - customAttributes: [{ attribute_code: 'company_id', value: '222' }] + customAttributes: [{ attribute_code: 'company_id', value: '222' }], + getCacheKey: function () { return 'billing-of-its-own'; } }); } diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index f1c62814..237f2722 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -503,11 +503,13 @@ describe('the quote\'s billing address belongs to the billing panel', () => { expect(renderer.telephone()).toBe('+4420 7946 0000'); }); - test('a virtual cart with no billing form at all seeds the SHIPPING identity', () => { - // The buyer's only address, and no billing company field is rendered for - // it — so the resolver reads the shipping capture, and seeding the - // billing panel there loses a saved company outright: a `TWO:` or - // sole-trader identity cannot be recovered by searching (TWO-25554). + test('a virtual cart with no billing form rendered still leaves shipping empty', () => { + // The quote holds one address and it is the billing one, so the billing + // identity is where it lands whether or not a form for it is on the page + // — the destination follows the quote, never the DOM (TWO-25554). With + // no form rendered the resolver reads the shipping capture, so the + // company is not offered back to this buyer; the alternative is worse, + // because it paints a billing company into the shipping panel's field. const booted = boot({ isVirtual: true, shippingForm: false, @@ -518,10 +520,9 @@ describe('the quote\'s billing address belongs to the billing panel', () => { renderer.updateBillingAddress(SAVED); - expect(booted.identities.shipping.companyId()).toBe('555'); - expect(booted.identities.shipping.companyName()).toBe('Saved Billing Co'); - expect(booted.capture.identity.companyId()).toBe('555'); - expect(booted.identities.billing.companyId()).toBe(''); + expect(booted.identities.billing.companyId()).toBe('555'); + expect(booted.identities.shipping.companyId()).toBe(''); + expect(booted.identities.shipping.companyName()).toBe(''); }); test('a billing address with no shipping panel mounted still leaves shipping empty', () => { diff --git a/Test/Js/gateway-method-company-selection.test.js b/Test/Js/gateway-method-company-selection.test.js index 89abb49a..4809470a 100644 --- a/Test/Js/gateway-method-company-selection.test.js +++ b/Test/Js/gateway-method-company-selection.test.js @@ -472,17 +472,17 @@ describe('a company picked on the shipping step reaches the payment step', () => }); test('the same company on the BILLING address alone still seeds the billing role', () => { - // No billing panel is mounted on this checkout, so billing is not a - // distinct address and the shipping identity is the only capture the + // The quote's own key, matching its shipping address: billing is not a + // distinct address, so the shipping identity is the only capture the // resolver reads — which is what the resolved observables show - // (TWO-25554). Where the billing panel IS mounted the seed stops there: - // company-capture-billing-panel.test.js. + // (TWO-25554). Where the billing address IS distinct the seed stops at + // the billing identity: company-capture-billing-panel.test.js. const { renderer, billingAddress } = loadWithSections({}); renderer.fillCustomerData(); billingAddress({ - getCacheKey: () => 'k3', + getCacheKey: () => 'k', countryId: 'GB', telephone: '+47 123 45 678', company: 'Billing Example Ltd', diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index 1defa597..58bedf38 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -466,10 +466,15 @@ define([ * the resolver reads (company-source-resolver.js); seeding the billing * panel there discards a saved company outright. * + * @param {boolean} billingIsDistinctAddress the QUOTE's own answer. + * Never a read of the billing fieldset: a checkout that has that + * fieldset transiently away still has two addresses, and routing + * on what is on screen puts billing's company into the shipping + * panel's own field (TWO-25554). * @returns {object} */ - billingRoleIdentity: function () { - return billingIsDistinct() ? billingIdentity : shippingIdentity; + billingRoleIdentity: function (billingIsDistinctAddress) { + return billingIsDistinctAddress ? billingIdentity : shippingIdentity; }, start: function () { shippingComponent.start(); diff --git a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js index 663a0f2e..c546e938 100644 --- a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js +++ b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js @@ -863,6 +863,23 @@ define([ // which is what the component picks its mount by. this.refreshCompanyMount(); }, + /** + * Is the quote's billing address a distinct address from its shipping + * one? The same cache-key comparison updateShippingAddress() gates its + * own relay on. + * + * No shipping address at all — a virtual cart — leaves billing as the + * only address the quote holds, which no shipping address can be the + * same as. + * + * @param {object} billingAddress quote address + * @returns {boolean} + */ + billingAddressIsDistinct: function (billingAddress) { + const shippingAddress = quote.shippingAddress(); + if (!shippingAddress) return true; + return shippingAddress.getCacheKey() != billingAddress.getCacheKey(); + }, /** * The quote's BILLING address. Its company seeds the identity of the * panel that owns the billing ROLE — the billing panel while billing is @@ -879,7 +896,7 @@ define([ const fields = this.readAddressFields(billingAddress); this.applyBuyerFields(fields); if (fields.companyName && fields.companyId) { - companyCapture.billingRoleIdentity().write({ + companyCapture.billingRoleIdentity(this.billingAddressIsDistinct(billingAddress)).write({ companyName: fields.companyName, companyId: fields.companyId }); From 284028b9ac61928f5c3fa8ce7845734572a9378f Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:07:22 +0100 Subject: [PATCH 08/30] fix(TWO-25554): key the address write record on the calling panel's identity Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-independence.test.js | 8 +-- ...mpany-search-address-field-routing.test.js | 18 ++++--- Test/Js/company-search-address-lookup.test.js | 8 ++- Test/Js/company-search-address-writes.test.js | 53 ++++++++++++++++--- Test/Js/company-search-country-switch.test.js | 38 ++++++------- ...method-sole-trader-phone-writeback.test.js | 3 +- view/frontend/web/js/model/company-capture.js | 8 +-- view/frontend/web/js/model/company-search.js | 48 +++++++++-------- .../web/js/view/address-autocomplete.js | 2 +- 9 files changed, 122 insertions(+), 64 deletions(-) diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 237f2722..113c93f9 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -384,8 +384,8 @@ describe('a country switch invalidates its own panel\'s company and nothing else // Each panel picks, which is what puts an address in its own form. picks(booted.panels.shipping, COMPANIES.shipping); picks(booted.panels.billing, COMPANIES.billing); - booted.search.applyAddress(ADDRESSES.shipping, $(SHIPPING_FORM)); - booted.search.applyAddress(ADDRESSES.billing, $(BILLING_FORM)); + booted.search.applyAddress(ADDRESSES.shipping, $(SHIPPING_FORM), booted.identities.shipping); + booted.search.applyAddress(ADDRESSES.billing, $(BILLING_FORM), booted.identities.billing); const before = addressValues(other); const beforeCountry = document.querySelector(COUNTRIES[other]).value; expect(before['city']).not.toBe(''); @@ -406,7 +406,7 @@ describe('a country switch invalidates its own panel\'s company and nothing else const booted = boot(); bootAddressStep(booted); picks(booted.panels[actor], COMPANIES[actor]); - booted.search.applyAddress(ADDRESSES[actor], $(FORMS[actor])); + booted.search.applyAddress(ADDRESSES[actor], $(FORMS[actor]), booted.identities[actor]); expect(addressValues(actor)['city']).toBe(ADDRESSES[actor].city); switchCountry(actor, 'SE'); @@ -458,7 +458,7 @@ describe('a tile-mounted shipping panel has no form of its own', () => { // borrowing are pinned in gateway-method-sole-trader-address-writeback // and company-search-address-lookup. const booted = boot({ shippingForm: false }); - booted.search.applyAddress(ADDRESSES.billing, $(BILLING_FORM)); + booted.search.applyAddress(ADDRESSES.billing, $(BILLING_FORM), booted.identities.billing); const before = addressValues('billing'); expect(before['city']).toBe(ADDRESSES.billing.city); diff --git a/Test/Js/company-search-address-field-routing.test.js b/Test/Js/company-search-address-field-routing.test.js index a6effdab..6eff2eaf 100644 --- a/Test/Js/company-search-address-field-routing.test.js +++ b/Test/Js/company-search-address-field-routing.test.js @@ -192,12 +192,14 @@ function load(regionShape) { // Every write and every revert is scoped to the calling panel's OWN form // (TWO-25554). const root = $('#shipping-new-address-form'); + /** The panel the write record is keyed on. */ + const panel = {}; return { model: model, $: $, root: root, - apply: function (address) { return model.applyAddress(address, root); }, - revert: function () { return model.revertAutofilledAddress(root); }, + apply: function (address) { return model.applyAddress(address, root, panel); }, + revert: function () { return model.revertAutofilledAddress(root, panel); }, /** * @param {string} name field name * @returns {?Element} @@ -472,7 +474,7 @@ describe('every field the write can reach, the revert can take back', () => { street: 'Mill Lane', building: 'Mill House', region: 'California' - }, $('#shipping-new-address-form')); + }, $('#shipping-new-address-form'), {}); const complete = 'city=Los Angeles postcode=90001 street[0]=Mill House street[1]=Mill Lane region_id=12'; @@ -513,7 +515,8 @@ describe('every field the write can reach, the revert can take back', () => { model.applyAddress( { city: 'Los Angeles', street: 'Mill Lane', country_code: 'US' }, - $('#shipping-new-address-form') + $('#shipping-new-address-form'), + {} ); expect(document.querySelector('[name="country_id"]').value).toBe('GB'); @@ -605,7 +608,8 @@ describe('a shop configured for a single street line', () => { model.applyAddress( { street: 'Mill Lane', building: 'Mill House', city: 'Ashford' }, - $('#shipping-new-address-form') + $('#shipping-new-address-form'), + {} ); expect(document.querySelector('[name="street[0]"]').value).toBe('Mill House, Mill Lane'); @@ -651,7 +655,7 @@ describe('the write can be scoped to one address form', () => { const { model, $ } = loadTwoForms(); const before = valueIn(untouched, 'city'); - model.applyAddress({ city: 'Ashford', street: 'Mill Lane' }, $(written)); + model.applyAddress({ city: 'Ashford', street: 'Mill Lane' }, $(written), {}); expect(tagged(description, valueIn(written, 'city'))) .toEqual(tagged(description, 'Ashford')); @@ -668,7 +672,7 @@ describe('the write can be scoped to one address form', () => { const { model, $ } = loadTwoForms(); const root = rootKind === 'empty' ? $('#nothing-here') : rootKind; - expect(tagged(description, model.applyAddress({ city: 'Ashford' }, root))) + expect(tagged(description, model.applyAddress({ city: 'Ashford' }, root, {}))) .toEqual(tagged(description, 0)); expect(valueIn('#shipping-new-address-form', 'city')).toBe('Shipping City'); expect(valueIn('[data-form="billing-new-address"]', 'city')).toBe('Billing City'); diff --git a/Test/Js/company-search-address-lookup.test.js b/Test/Js/company-search-address-lookup.test.js index 5e43f9cf..d4cde0ba 100644 --- a/Test/Js/company-search-address-lookup.test.js +++ b/Test/Js/company-search-address-lookup.test.js @@ -507,12 +507,16 @@ describe('a tile-mounted shipping panel and the one address form there is (TWO-2 test.each([ [ - function (search, root, identity) { return search.applyAddress({ city: 'X' }, root); }, + function (search, root, identity) { + return search.applyAddress({ city: 'X' }, root, identity); + }, 0, 'applyAddress refuses' ], [ - function (search, root) { return search.revertAutofilledAddress(root); }, + function (search, root, identity) { + return search.revertAutofilledAddress(root, identity); + }, 0, 'revertAutofilledAddress refuses' ], diff --git a/Test/Js/company-search-address-writes.test.js b/Test/Js/company-search-address-writes.test.js index 8dadefe1..1eb02694 100644 --- a/Test/Js/company-search-address-writes.test.js +++ b/Test/Js/company-search-address-writes.test.js @@ -251,13 +251,30 @@ const COMPANY_B = { street_address: '2 Second Street' }; +/** + * The identity of the panel that owns one form. The write record is keyed on + * it, so two forms must be driven through two of these. + */ +let panels = {}; + +function panel(rootSelector) { + panels[rootSelector] = panels[rootSelector] || { form: rootSelector }; + return panels[rootSelector]; +} + afterEach(() => { document.body.innerHTML = ''; + panels = {}; }); /** One panel writing an address into ITS OWN form. */ function writeInto(model, address, rootSelector) { - return model.applyAddress(address, $(rootSelector)); + return model.applyAddress(address, $(rootSelector), panel(rootSelector)); +} + +/** The retraction that same panel makes. */ +function revertFrom(model, rootSelector) { + return model.revertAutofilledAddress($(rootSelector), panel(rootSelector)); } describe('an external address payload is routed onto the two address lines', () => { @@ -399,7 +416,7 @@ describe('a retraction stops at the form it was scoped to', () => { writeInto(model, COMPANY_A, PRIMARY); writeInto(model, COMPANY_B, SECONDARY); - expect(model.revertAutofilledAddress($(scoped))).toBe(3); + expect(revertFrom(model, scoped)).toBe(3); expect(read(scoped, 'city')).toBe(''); expect(read(other, 'city')).toBe(other === PRIMARY ? 'London' : 'Stockholm'); @@ -413,7 +430,7 @@ describe('a retraction stops at the form it was scoped to', () => { writeInto(model, COMPANY_A, PRIMARY); writeInto(model, COMPANY_A, SECONDARY); - expect([description, model.revertAutofilledAddress(root)]).toEqual([description, 0]); + expect([description, model.revertAutofilledAddress(root, panel(PRIMARY))]).toEqual([description, 0]); expect(read(PRIMARY, 'city')).toBe('London'); expect(read(SECONDARY, 'city')).toBe('London'); @@ -469,7 +486,7 @@ describe('a retraction survives the checkout rebuilding its own fieldset', () => street0: '1 Example Street' }); - expect(model.revertAutofilledAddress($(SECONDARY))).toBe(3); + expect(revertFrom(model, SECONDARY)).toBe(3); expect(read(SECONDARY, 'city')).toBe(''); expect(read(SECONDARY, 'street0')).toBe(''); }); @@ -484,10 +501,34 @@ describe('a retraction survives the checkout rebuilding its own fieldset', () => street0: '1 Example Street' }); - expect(model.revertAutofilledAddress($(SECONDARY))).toBe(2); + expect(revertFrom(model, SECONDARY)).toBe(2); expect(read(SECONDARY, 'city')).toBe('Ashford'); }); + test('and survives the whole subtree holding that fieldset being replaced', () => { + // A one-step layout rebuilds the payment-methods subtree, which is + // where the billing fieldset lives — so the fieldset ELEMENT is a + // different node afterwards, and a record keyed on it describes a node + // no reader can reach (TWO-25554). + const model = renderCheckout({ regions: true }); + writeInto(model, COMPANY_A, SECONDARY); + + const container = document.querySelector('.checkout-billing-address'); + container.innerHTML = + '
' + + addressFields({ regions: true }) + + '
'; + const values = { city: 'London', postcode: 'EC1A 1BB', street0: '1 Example Street' }; + const rebuilt = document.querySelector(SECONDARY); + Object.keys(values).forEach(function (name) { + rebuilt.querySelector(FIELD_SELECTORS[name]).value = values[name]; + }); + + expect(revertFrom(model, SECONDARY)).toBe(3); + expect(read(SECONDARY, 'city')).toBe(''); + expect(read(SECONDARY, 'street0')).toBe(''); + }); + test('the other panel\'s form is not reachable through the record either', () => { const model = renderCheckout({ regions: true }); writeInto(model, COMPANY_A, PRIMARY); @@ -498,7 +539,7 @@ describe('a retraction survives the checkout rebuilding its own fieldset', () => postcode: '111 22', street0: '2 Second Street' }); - model.revertAutofilledAddress($(SECONDARY)); + revertFrom(model, SECONDARY); expect(read(PRIMARY, 'city')).toBe('London'); expect(read(PRIMARY, 'street0')).toBe('1 Example Street'); diff --git a/Test/Js/company-search-country-switch.test.js b/Test/Js/company-search-country-switch.test.js index da40fd55..dc408c78 100644 --- a/Test/Js/company-search-country-switch.test.js +++ b/Test/Js/company-search-country-switch.test.js @@ -268,7 +268,9 @@ function loadModel() { dom: dom, // Every write and revert is scoped to the calling panel's own form // (TWO-25554); there is no page-wide path. - root: dom.$(PRIMARY_ROOT) + root: dom.$(PRIMARY_ROOT), + /** The panel the write record is keyed on. */ + panel: {} }; } @@ -445,13 +447,13 @@ beforeEach(() => { describe('reverting an address autofilled from the previous country', () => { test('applyAddress records exactly what it wrote, on every field', () => { - const { model, node, root } = loadModel(); + const { model, node, root, panel } = loadModel(); model.applyAddress({ city: 'London', postal_code: 'EC1A 1BB', street_address: '1 Example Street' - }, root); + }, root, panel); expect(node(CITY).val()).toBe('London'); expect(node(CITY).attr(MARKER)).toBe('London'); @@ -460,15 +462,15 @@ describe('reverting an address autofilled from the previous country', () => { }); test('the revert clears an untouched autofill and fires change for it', () => { - const { model, node, dom, root } = loadModel(); + const { model, node, dom, root, panel } = loadModel(); model.applyAddress({ city: 'London', postal_code: 'EC1A 1BB', street_address: '1 Example Street' - }, root); + }, root, panel); dom.triggered.length = 0; - expect(model.revertAutofilledAddress(root)).toBe(3); + expect(model.revertAutofilledAddress(root, panel)).toBe(3); expect(node(CITY).val()).toBe(''); expect(node(POSTCODE).val()).toBe(''); @@ -482,27 +484,27 @@ describe('reverting an address autofilled from the previous country', () => { // The whole reason the marker records the VALUE rather than a boolean. // Over-clearing here deletes buyer input on a keystroke they may have // made minutes earlier — worse than leaving a stale value they can see. - const { model, node, root } = loadModel(); + const { model, node, root, panel } = loadModel(); model.applyAddress({ city: 'London', postal_code: 'EC1A 1BB', street_address: '1 Example Street' - }, root); + }, root, panel); node(CITY).val('Madrid'); - expect(model.revertAutofilledAddress(root)).toBe(2); + expect(model.revertAutofilledAddress(root, panel)).toBe(2); expect(node(CITY).val()).toBe('Madrid'); expect(node(POSTCODE).val()).toBe(''); }); test('a hand-filled form with no autofill behind it is left entirely alone', () => { - const { model, node, root } = loadModel(); + const { model, node, root, panel } = loadModel(); node(CITY).val('Madrid'); node(POSTCODE).val('28001'); node(STREET).val('Calle Example 1'); - expect(model.revertAutofilledAddress(root)).toBe(0); + expect(model.revertAutofilledAddress(root, panel)).toBe(0); expect(node(CITY).val()).toBe('Madrid'); expect(node(POSTCODE).val()).toBe('28001'); @@ -513,25 +515,25 @@ describe('reverting an address autofilled from the previous country', () => { // Two companies sharing a postcode: without the refresh the second // write leaves no recording, and the field reads as buyer-typed — so // the revert would strand it — for the rest of the page's life. - const { model, node, root } = loadModel(); - model.applyAddress({ city: 'London', postal_code: 'EC1A 1BB', street_address: 'One' }, root); + const { model, node, root, panel } = loadModel(); + model.applyAddress({ city: 'London', postal_code: 'EC1A 1BB', street_address: 'One' }, root, panel); node(POSTCODE).removeAttr(MARKER); - model.applyAddress({ city: 'London', postal_code: 'EC1A 1BB', street_address: 'Two' }, root); + model.applyAddress({ city: 'London', postal_code: 'EC1A 1BB', street_address: 'Two' }, root, panel); expect(node(POSTCODE).attr(MARKER)).toBe('EC1A 1BB'); - expect(model.revertAutofilledAddress(root)).toBe(3); + expect(model.revertAutofilledAddress(root, panel)).toBe(3); }); test('an empty registry value is a recording, not an absence', () => { // `''` is what the API sends for a field the registry has nothing for. // A falsiness test here would leave the marker unread and the field // permanently un-revertable. - const { model, node, root } = loadModel(); - model.applyAddress({ city: 'London', postal_code: '', street_address: 'One' }, root); + const { model, node, root, panel } = loadModel(); + model.applyAddress({ city: 'London', postal_code: '', street_address: 'One' }, root, panel); expect(node(POSTCODE).attr(MARKER)).toBe(''); - expect(model.revertAutofilledAddress(root)).toBe(3); + expect(model.revertAutofilledAddress(root, panel)).toBe(3); }); }); diff --git a/Test/Js/gateway-method-sole-trader-phone-writeback.test.js b/Test/Js/gateway-method-sole-trader-phone-writeback.test.js index 25c87bc8..0aa539d2 100644 --- a/Test/Js/gateway-method-sole-trader-phone-writeback.test.js +++ b/Test/Js/gateway-method-sole-trader-phone-writeback.test.js @@ -131,7 +131,8 @@ describe('the address write is not the route the phone takes', () => { companySearch.applyAddress( Object.assign({}, BILLING, { phone_number: '+442012345678' }), - $('#shipping-new-address-form') + $('#shipping-new-address-form'), + {} ); expect(document.querySelector('input[name="city"]').value).toBe('Ashford'); diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index 58bedf38..51f53109 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -368,10 +368,10 @@ define([ ); }, revertAutofilledAddress: function () { - companySearch.revertAutofilledAddress(shippingWriteRoot()); + companySearch.revertAutofilledAddress(shippingWriteRoot(), shippingIdentity); }, applyBuyerAddress: function (source) { - companySearch.applyAddress(source, shippingWriteTarget()); + companySearch.applyAddress(source, shippingWriteTarget(), shippingIdentity); }, applyTelephone: function (phoneNumber) { companySearch.applyTelephone(phoneNumber, shippingWriteTarget()); @@ -410,10 +410,10 @@ define([ ); }, revertAutofilledAddress: function () { - companySearch.revertAutofilledAddress($(BILLING_FORM_ROOT)); + companySearch.revertAutofilledAddress($(BILLING_FORM_ROOT), billingIdentity); }, applyBuyerAddress: function (source) { - companySearch.applyAddress(source, $(BILLING_FORM_ROOT)); + companySearch.applyAddress(source, $(BILLING_FORM_ROOT), billingIdentity); }, applyTelephone: function (phoneNumber) { companySearch.applyTelephone(phoneNumber, $(BILLING_FORM_ROOT)); diff --git a/view/frontend/web/js/model/company-search.js b/view/frontend/web/js/model/company-search.js index 5aecc75b..43e7abf6 100644 --- a/view/frontend/web/js/model/company-search.js +++ b/view/frontend/web/js/model/company-search.js @@ -111,29 +111,28 @@ define([ } /** - * What applyAddress() last wrote into ONE address form, field name → value, - * keyed by that form's own element exactly as addressLookupState() is. + * What applyAddress() last wrote for ONE panel, field name → value. * * The DOM markers are the primary record and this is their survivor: a * checkout rebuilding an address fieldset destroys every attribute on it, * and a country switch after such a rebuild still has to retract the * previous country's address rather than leave it standing (TWO-25554). * - * Per FORM, and read only for the form the calling panel passed in, so - * neither panel can reach the other's recording through it. + * Keyed on the calling panel's IDENTITY, which is per-panel and unreachable + * from the other one, and which a one-step checkout replacing the whole + * payment-methods subtree — where the billing form lives — does not replace. */ const addressWriteRecords = new WeakMap(); /** - * @param {object} root jQuery-wrapped address form - * @returns {object} that form's live record, created empty on first use + * @param {object} identity the calling panel's own identity + * @returns {object} that panel's live record, created empty on first use */ - function addressWriteRecord(root) { - const key = firstElement(root) || root; - let record = addressWriteRecords.get(key); + function addressWriteRecord(identity) { + let record = addressWriteRecords.get(identity); if (!record) { record = {}; - addressWriteRecords.set(key, record); + addressWriteRecords.set(identity, record); } return record; } @@ -602,8 +601,10 @@ define([ * @param {object} address company address or buyer address record * @param {?object} $root jQuery set to scope every field read and write * to; document-wide when null + * @param {object} identity the calling panel's own identity, keying the + * recording */ - function writeAddressInto(self, address, $root) { + function writeAddressInto(self, address, $root, identity) { const values = self.resolveAddressValues(address, $root); const names = Object.keys(values); // What the marker attribute records for each written field — the @@ -629,7 +630,7 @@ define([ // Replaced wholesale rather than merged: a field this payload says // nothing about is retracted below, so a recording for it would outlive // the value it describes. - const record = addressWriteRecord($root); + const record = addressWriteRecord(identity); Object.keys(record).forEach(function (name) { delete record[name]; }); Object.assign(record, recordAs); retractStaleFields( @@ -849,10 +850,11 @@ define([ * what the plugin put there, and forget the recording. * * @param {object} $root jQuery-wrapped address form + * @param {object} identity the calling panel's own identity * @returns {Array} the names of the fields cleared */ - function revertAddressFormFields($root) { - const record = addressWriteRecord($root); + function revertAddressFormFields($root, identity) { + const record = addressWriteRecord(identity); const cleared = []; REVERTABLE_FIELDS.forEach(function (field) { const handle = revertableFieldHandle($root, field); @@ -1150,7 +1152,7 @@ define([ const envelope = unwrapProxyResponse(raw); const response = envelope.ok ? envelope.body : null; if (response && response.addresses && response.addresses.length) { - self.applyAddress(response.addresses[0], root); + self.applyAddress(response.addresses[0], root, identity); return; } announceAddressUnavailable(notify); @@ -1198,15 +1200,17 @@ define([ * * @param {object} address company address or buyer address record * @param {object} root jQuery set for the calling panel's own form + * @param {object} identity the calling panel's own identity, keying the + * recording a later revert reads * @returns {number} 0 — no address other than `root` is ever written */ - applyAddress: function (address, root) { + applyAddress: function (address, root, identity) { console.debug({ logger: 'companySearch.applyAddress', address }); - if (!root || !root.length) { + if (!root || !root.length || !identity) { console.debug({ logger: 'companySearch.applyAddress.refused' }); return 0; } - writeAddressInto(this, address, root); + writeAddressInto(this, address, root, identity); return 0; }, @@ -1347,15 +1351,17 @@ define([ * buyer edit. * * @param {object} root the calling panel's own form + * @param {object} identity the calling panel's own identity, keying the + * recording this reads * @returns {number} how many fields were cleared — for tests, and so a * caller can tell "nothing was ours" from "reverted" */ - revertAutofilledAddress: function (root) { - if (!root || !root.length) { + revertAutofilledAddress: function (root, identity) { + if (!root || !root.length || !identity) { console.debug({ logger: 'companySearch.revertAutofilledAddress.refused' }); return 0; } - const retracted = revertAddressFormFields(root); + const retracted = revertAddressFormFields(root, identity); console.debug({ logger: 'companySearch.revertAutofilledAddress', cleared: retracted.length diff --git a/view/frontend/web/js/view/address-autocomplete.js b/view/frontend/web/js/view/address-autocomplete.js index f50ba987..ac802c08 100755 --- a/view/frontend/web/js/view/address-autocomplete.js +++ b/view/frontend/web/js/view/address-autocomplete.js @@ -120,7 +120,7 @@ define([ } // THIS form alone — the other panel's fields are never touched // (TWO-25554). - companySearch.revertAutofilledAddress($(this.addressFormSelector)); + companySearch.revertAutofilledAddress($(this.addressFormSelector), identity); // Clears the name input, the number field, and the published // `companyData` section the payment tile reads — every surviving // copy of the previous country's company. From 9b3450b0e38727fe9e56253f7c74c9035e5d083a Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:08:13 +0100 Subject: [PATCH 09/30] test(TWO-25554): let the new tables' row descriptions reach the failure diff Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-chrome.test.js | 75 ++++++++++++++++------------ 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index bb112aa2..7ba0b0de 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -22,7 +22,8 @@ const { loadCompanySearchPanel, defaultMocks, brandConfigMock, - installAsyncSimulation + installAsyncSimulation, + tagged } = require('./amd-harness'); const SEARCH = 'view/frontend/web/js/model/company-search.js'; @@ -278,30 +279,35 @@ describe('the company number is painted under its own panel\'s field', () => { test.each([ [ 'shipping', - { shippingCompanyIdField: false, billingNumber: '555555555' }, - 'the shipping form has no number field and one billing neighbour has one' + 'the shipping form has no number field and one billing neighbour has one', + { shippingCompanyIdField: false, billingNumber: '555555555' } ], [ 'billing', - { billingCompanyIdField: false, shippingNumber: '777777777' }, - 'the billing form has no number field and one shipping neighbour has one' + 'the billing form has no number field and one shipping neighbour has one', + { billingCompanyIdField: false, shippingNumber: '777777777' } ] - ])('%s claims nothing from the single neighbour holding a number (%s)', async (actor, fixture) => { - const other = OTHER[actor]; - const restored = { shipping: fixture.shippingNumber, billing: fixture.billingNumber }; - const { panels } = boot(fixture); - expect(panels[actor].mountSelector()).toBe(FIELDS[actor]); - - // `watchCapturedIdentity` publishes on a macrotask, so an assertion made - // before it has run denies a propagation that had not happened yet. - await flushCapture(); - await flushCapture(); - - expect(panels[actor].displayCompanyNumber()).toBe(''); - expect(numbersIn(actor)).toEqual([]); - // The neighbour still paints the number in its OWN field. - expect(numbersIn(other)).toEqual([restored[other]]); - }); + ])( + '%s claims nothing from the single neighbour holding a number (%s)', + async (actor, description, fixture) => { + const other = OTHER[actor]; + const restored = { shipping: fixture.shippingNumber, billing: fixture.billingNumber }; + const { panels } = boot(fixture); + expect(panels[actor].mountSelector()).toBe(FIELDS[actor]); + + // `watchCapturedIdentity` publishes on a macrotask, so an assertion + // made before it has run denies a propagation that had not happened. + await flushCapture(); + await flushCapture(); + + expect(tagged(description, panels[actor].displayCompanyNumber())) + .toEqual(tagged(description, '')); + expect(tagged(description, numbersIn(actor))).toEqual(tagged(description, [])); + // The neighbour still paints the number in its OWN field. + expect(tagged(description, numbersIn(other))) + .toEqual(tagged(description, [restored[other]])); + } + ); test('a panel whose own form carries no number field claims neither neighbour\'s', () => { // Two billing fieldsets, a number restored into each. The shipping form @@ -331,34 +337,39 @@ describe('chrome never enters the popover\'s positioning context', () => { * pieces of chrome are on the page at once. */ const NUMBERED_TRADER = { company_name: 'Numbered Trader', organization_number: '333333333' }; - test.each(DIRECTIONS)('%s renders both pieces of chrome (%s)', (actor) => { + test.each(DIRECTIONS)('%s renders both pieces of chrome (%s)', (actor, description) => { const { panels } = boot(); panels[actor].adoptSoleTrader(NUMBERED_TRADER); const wrap = document.querySelector(`${FORMS[actor]} .two-company-field-wrap`); expect(wrap).not.toBeNull(); - expect(wrap.querySelectorAll(`.${NUMBER_CLASS}, .${LINK_CLASS}`)).toHaveLength(0); + expect(tagged(description, wrap.querySelectorAll(`.${NUMBER_CLASS}, .${LINK_CLASS}`).length)) + .toEqual(tagged(description, 0)); const number = document.querySelector(`${FORMS[actor]} .${NUMBER_CLASS}`); const link = document.querySelector(`${FORMS[actor]} .${LINK_CLASS}`); - expect(number.parentElement).toBe(wrap.parentElement); - expect(link.parentElement).toBe(wrap.parentElement); - expect(wrap.nextElementSibling).toBe(number); - expect(number.nextElementSibling).toBe(link); + expect(tagged(description, number.parentElement)) + .toEqual(tagged(description, wrap.parentElement)); + expect(tagged(description, link.parentElement)) + .toEqual(tagged(description, wrap.parentElement)); + expect(tagged(description, wrap.nextElementSibling)).toEqual(tagged(description, number)); + expect(tagged(description, number.nextElementSibling)).toEqual(tagged(description, link)); }); - test.each(DIRECTIONS)('%s keeps that order across a repaint (%s)', (actor) => { + test.each(DIRECTIONS)('%s keeps that order across a repaint (%s)', (actor, description) => { const { panels } = boot(); panels[actor].adoptSoleTrader(NUMBERED_TRADER); panels[actor].renderChrome(); const wrap = document.querySelector(`${FORMS[actor]} .two-company-field-wrap`); - expect(wrap.querySelectorAll(`.${NUMBER_CLASS}, .${LINK_CLASS}`)).toHaveLength(0); - expect(wrap.nextElementSibling.classList.contains(NUMBER_CLASS)).toBe(true); - expect(numbersIn(actor)).toHaveLength(1); - expect(linksIn(actor)).toBe(1); + expect(tagged(description, wrap.querySelectorAll(`.${NUMBER_CLASS}, .${LINK_CLASS}`).length)) + .toEqual(tagged(description, 0)); + expect(tagged(description, wrap.nextElementSibling.classList.contains(NUMBER_CLASS))) + .toEqual(tagged(description, true)); + expect(tagged(description, numbersIn(actor).length)).toEqual(tagged(description, 1)); + expect(tagged(description, linksIn(actor))).toEqual(tagged(description, 1)); }); }); From ee594a90aebffc87658e9755dcb1c77a5ac74db1 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:08:45 +0100 Subject: [PATCH 10/30] test(TWO-25554): pin the billing predicate against its own false positives Co-Authored-By: Claude Sonnet 5 --- .../Model/Checkout/LayoutProcessorPluginTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php b/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php index db01b828..0c90704a 100644 --- a/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php +++ b/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php @@ -663,6 +663,22 @@ public static function untouchedBillingNodeProvider(): array [['component' => self::BILLING_COMPONENT, 'children' => ['form-fields' => ['children' => []]]], 'a billing form with no dataScopePrefix'], [['component' => self::BILLING_COMPONENT, 'dataScopePrefix' => ''], 'an empty dataScopePrefix'], [['component' => self::BILLING_COMPONENT, 'dataScopePrefix' => 'billingAddressshared'], 'a billing form with no form-fields'], + [ + [ + 'component' => 'Vendor_Module/js/view/some-fieldset', + 'dataScopePrefix' => 'shippingAddress', + 'children' => ['form-fields' => ['children' => ['city' => ['label' => 'City']]]], + ], + 'a fieldset whose scope is not a billing one, complete in every other respect', + ], + [ + [ + 'component' => self::BILLING_COMPONENT, + 'dataScopePrefix' => '', + 'children' => ['form-fields' => ['children' => ['city' => ['label' => 'City']]]], + ], + 'core\'s own component with no scope to bind a number to', + ], ['not-an-array', 'a scalar child'], ]; } From 2208fe5ef022503b83ecdced2b061d700e17cd09 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:10:13 +0100 Subject: [PATCH 11/30] fix(TWO-25554): give the no-destination refusal its own notice Co-Authored-By: Claude Sonnet 5 --- Test/Js/amd-harness.js | 3 +++ Test/Js/company-panel-independence.test.js | 3 ++- Test/Js/company-search-address-lookup.test.js | 5 ++++- i18n/nb_NO.csv | 1 + i18n/nl_NL.csv | 1 + i18n/sv_SE.csv | 1 + view/frontend/web/js/model/company-capture.js | 2 +- view/frontend/web/js/model/company-search.js | 19 +++++++++++++++++-- 8 files changed, 30 insertions(+), 5 deletions(-) diff --git a/Test/Js/amd-harness.js b/Test/Js/amd-harness.js index af6c5433..fd7b45d5 100644 --- a/Test/Js/amd-harness.js +++ b/Test/Js/amd-harness.js @@ -166,6 +166,9 @@ function defaultMocks() { announceAddressUnavailable: function (identity) { identity.addressNotice('address unavailable'); }, + announceAddressUndeliverable: function (identity) { + identity.addressNotice('address undeliverable'); + }, hasPrimaryAddressForm: function () { return true; }, isDegradedResponse: function () { return false; }, // DELEGATED, like the display helpers below: an inert stub would diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 113c93f9..92c31a98 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -650,7 +650,8 @@ describe('the payment tile is the shipping panel\'s mount, or nobody\'s', () => booted.panels.shipping.host().applyBuyerAddress({ city: 'Ashford' }); expect(addressValues('billing')['city']).toBe(''); - expect(booted.identities.shipping.addressNotice()).not.toBe(''); + expect(booted.identities.shipping.addressNotice()) + .toBe('We could not fill in this company\'s address on this page.'); }); test('the tile field never displays the billing panel\'s capture', () => { diff --git a/Test/Js/company-search-address-lookup.test.js b/Test/Js/company-search-address-lookup.test.js index d4cde0ba..31b1f13d 100644 --- a/Test/Js/company-search-address-lookup.test.js +++ b/Test/Js/company-search-address-lookup.test.js @@ -502,7 +502,10 @@ describe('a tile-mounted shipping panel and the one address form there is (TWO-2 expect(identity.companyId()).toBe('12345678'); expect(lookupIds(recorder)).toEqual([]); expect(recorder.written).toHaveLength(0); - expect(identity.addressNotice()).not.toBe(''); + // Its own copy: there is no form below to enter the address into, so + // the fetch-failed notice would send the buyer nowhere. + expect(identity.addressNotice()) + .toBe('We could not fill in this company\'s address on this page.'); }); test.each([ diff --git a/i18n/nb_NO.csv b/i18n/nb_NO.csv index 7e9aafe4..09c6c821 100644 --- a/i18n/nb_NO.csv +++ b/i18n/nb_NO.csv @@ -311,6 +311,7 @@ "Invalid company lookup request.","Ugyldig forespørsel om firmaoppslag." "This order is too large to send for approval.","Denne ordren er for stor til å sendes til godkjenning." "We could not fetch this company's address. Please enter it below.","Vi klarte ikke å hente adressen til dette selskapet. Skriv den inn nedenfor." +"We could not fill in this company's address on this page.","We could not fill in this company's address on this page." "Trusted proxies","Klarerte proxyer" "Trusted proxies: ""%1"" is not a valid IP address or CIDR range.","Klarerte proxyer: ""%1"" er ikke en gyldig IP-adresse eller et gyldig CIDR-område." "Disable checkout rate limiting","Slå av hastighetsbegrensning i kassen" diff --git a/i18n/nl_NL.csv b/i18n/nl_NL.csv index eddd6150..a639931f 100644 --- a/i18n/nl_NL.csv +++ b/i18n/nl_NL.csv @@ -307,6 +307,7 @@ "Invalid company lookup request.","Ongeldige aanvraag voor bedrijfsgegevens." "This order is too large to send for approval.","Deze bestelling is te groot om ter goedkeuring te versturen." "We could not fetch this company's address. Please enter it below.","We konden het adres van dit bedrijf niet ophalen. Voer het hieronder in." +"We could not fill in this company's address on this page.","We could not fill in this company's address on this page." "Trusted proxies","Vertrouwde proxy's" "Trusted proxies: ""%1"" is not a valid IP address or CIDR range.","Vertrouwde proxy's: ""%1"" is geen geldig IP-adres of CIDR-bereik." "Disable checkout rate limiting","Snelheidsbeperking in de afrekening uitschakelen" diff --git a/i18n/sv_SE.csv b/i18n/sv_SE.csv index 9633b37a..9bf49cbe 100644 --- a/i18n/sv_SE.csv +++ b/i18n/sv_SE.csv @@ -308,6 +308,7 @@ "Invalid company lookup request.","Ogiltig förfrågan om företagsuppslag." "This order is too large to send for approval.","Den här ordern är för stor för att skickas för godkännande." "We could not fetch this company's address. Please enter it below.","Vi kunde inte hämta företagets adress. Ange den nedan." +"We could not fill in this company's address on this page.","We could not fill in this company's address on this page." "Trusted proxies","Betrodda proxyservrar" "Trusted proxies: ""%1"" is not a valid IP address or CIDR range.","Betrodda proxyservrar: ""%1"" är inte en giltig IP-adress eller ett giltigt CIDR-intervall." "Disable checkout rate limiting","Inaktivera hastighetsbegränsning i kassan" diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index 51f53109..db1a40c2 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -332,7 +332,7 @@ define([ */ function shippingWriteTarget() { const root = shippingWriteRoot(); - if (!root) companySearch.announceAddressUnavailable(shippingIdentity); + if (!root) companySearch.announceAddressUndeliverable(shippingIdentity); return root; } diff --git a/view/frontend/web/js/model/company-search.js b/view/frontend/web/js/model/company-search.js index 43e7abf6..9c659006 100644 --- a/view/frontend/web/js/model/company-search.js +++ b/view/frontend/web/js/model/company-search.js @@ -793,7 +793,20 @@ define([ ); } - /** The clear half of announceAddressUnavailable(). */ + /** + * Tell the buyer the address had nowhere to land. Its own wording, because + * the sibling notice above asks them to enter it below and this fires + * precisely when there is no form below to enter it into. + * + * @param {object} identity the CALLING panel's own identity + */ + function announceAddressUndeliverable(identity) { + identity.addressNotice( + $t('We could not fill in this company\'s address on this page.') + ); + } + + /** The clear half of both announcements above. */ function withdrawAddressUnavailable(identity) { identity.addressNotice(''); } @@ -980,6 +993,8 @@ define([ /** @see announceAddressUnavailable */ announceAddressUnavailable: announceAddressUnavailable, + /** @see announceAddressUndeliverable */ + announceAddressUndeliverable: announceAddressUndeliverable, /** * Run one company search and hand back rows the panel can render. @@ -1116,7 +1131,7 @@ define([ if (!root || !root.length || !identity) { // A picked company that fills nothing in, with nothing said, // reads as the picker having done nothing. - if (identity) announceAddressUnavailable(identity); + if (identity) announceAddressUndeliverable(identity); console.debug({ logger: 'companySearch.lookupCompanyAddress.refused' }); return null; } From ea87d3ce4f5708b2d39d86aa9bdfbb7a71934480 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:11:28 +0100 Subject: [PATCH 12/30] fix(TWO-25554): require a city before a container answers as the address form Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-search-address-lookup.test.js | 27 ++++++++++++++++++- view/frontend/web/js/model/company-capture.js | 13 +++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/Test/Js/company-search-address-lookup.test.js b/Test/Js/company-search-address-lookup.test.js index 31b1f13d..eea27674 100644 --- a/Test/Js/company-search-address-lookup.test.js +++ b/Test/Js/company-search-address-lookup.test.js @@ -469,7 +469,17 @@ describe('a tile-mounted shipping panel and the one address form there is (TWO-2 * rendered, so the shipping panel falls to the tile and the page's single * address form is the only destination a write could have. */ - const OWN_MARKUP_CHECKOUT = [TILE_FIELD_SELECTOR, 'input[name="street[0]"]']; + const OWN_MARKUP_CHECKOUT = [ + TILE_FIELD_SELECTOR, + 'input[name="street[0]"]', + 'input[name="city"]' + ]; + + /** + * The same checkout where the container around the street line holds no + * city — a themed row rather than the address form. + */ + const STREET_ROW_ONLY = [TILE_FIELD_SELECTOR, 'input[name="street[0]"]']; /** The same checkout with no address form at all — a virtual cart. */ const TILE_ALONE = [TILE_FIELD_SELECTOR]; @@ -491,6 +501,21 @@ describe('a tile-mounted shipping panel and the one address form there is (TWO-2 expect(identity.addressNotice()).toBe(''); }); + test('a container holding the street line alone is not that form', async () => { + // `closest()` answers with the NEAREST container, so a themed row around + // the street line qualifies on the street test alone — and a write + // scoped there fills in a street and nothing else (TWO-25554). + const { component, identity, recorder, pick } = loadMountedComponent(null, STREET_ROW_ONLY); + expect(component.mountSelector()).toBe(TILE_FIELD_SELECTOR); + + await pick('example', SEARCH_RESPONSE); + + expect(lookupIds(recorder)).toEqual([]); + expect(recorder.written).toHaveLength(0); + expect(identity.addressNotice()) + .toBe('We could not fill in this company\'s address on this page.'); + }); + test('with no address form at all the write is refused, and the buyer is told', async () => { // Refused, not guessed at — and NOT silently: the buyer gets neither an // address nor a reason for its absence otherwise. diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index db1a40c2..e99367e5 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -74,6 +74,9 @@ define([ /** @see soleAddressForm — the element kinds a checkout wraps one in. */ const ADDRESS_FORM_CONTAINER_SELECTOR = 'form, fieldset, [data-form]'; + /** @see soleAddressForm — the second field that makes one a whole address. */ + const ADDRESS_CITY_SELECTOR = 'input[name="city"]'; + const brandCode = brandConfig.getActiveTwoBrandCode(); const config = brandCode ? brandConfig(brandCode) : null; @@ -297,9 +300,15 @@ define([ function soleAddressForm() { if ($(ADDRESS_FORM_ROOT).length || $(BILLING_FORM_ROOT).length) return null; const $streets = $(ADDRESS_STREET_SELECTOR); - if ($streets.length !== 1 || typeof $streets.closest !== 'function') return null; + if ($streets.length !== 1) return null; + if (typeof $streets.closest !== 'function') return null; + // `closest()` answers with the NEAREST container, which on a themed + // checkout is often a row holding the street line alone — a write scoped + // there fills in a street and nothing else. The city is what makes the + // container the whole address form. const $form = $streets.closest(ADDRESS_FORM_CONTAINER_SELECTOR); - return $form.length === 1 ? $form : null; + if (!$form.length || typeof $form.find !== 'function') return null; + return $form.find(ADDRESS_CITY_SELECTOR).length ? $form : null; } /** From 9b8b648438b13b5a6be9a06efff0cf216fda3f19 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:12:11 +0100 Subject: [PATCH 13/30] docs(TWO-25554): state the rules these comments describe, and name the harness's divergences Co-Authored-By: Claude Sonnet 5 --- Test/Js/amd-harness.js | 11 +++++++++++ .../web/js/model/company-capture-component.js | 2 +- view/frontend/web/js/model/company-capture.js | 4 ++-- view/frontend/web/js/model/company-search.js | 9 +++++---- .../js/view/payment/method-renderer/gateway_method.js | 6 +++--- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/Test/Js/amd-harness.js b/Test/Js/amd-harness.js index fd7b45d5..8d6eced2 100644 --- a/Test/Js/amd-harness.js +++ b/Test/Js/amd-harness.js @@ -272,6 +272,17 @@ function defaultMocks() { let evaluatingComputed = null; function makeKnockoutMock() { + /** + * A computed over a live read, close enough to model the caching that makes + * a missing notification observable — and no closer. Three divergences from + * real Knockout, all of which a spec must not lean on: the dependency set + * only ever grows, `makeObservable` notifies on every write whether or not + * the value changed, so a re-publish can come from an observable the + * computed no longer reads; and there is no re-entrancy guard. + * + * @param {function} fn + * @returns {function} the computed's value accessor + */ function computed(fn) { const out = makeObservable(undefined); const dependencies = []; diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index 157ce378..5787a317 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -113,7 +113,7 @@ * `refreshMount()` off its own re-render hook instead. * @param {string} options.addressFieldSelector the address step's company * field. - * @param {string} options.addressFormRootSelector the form that field + * @param {string} [options.addressFormRootSelector] the form that field * belongs to — the one `addressFieldSelector` is built from. Bounds * every DOM read this panel makes, so absent it reads nothing. * @param {string} options.tileFieldSelector the payment tile's company diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index e99367e5..f5636255 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -334,8 +334,8 @@ define([ } /** - * The same destination, telling the buyer when there is none: a pick that - * fills nothing in and says nothing reads as the picker having done nothing. + * shippingWriteRoot(), and a notice on the shipping identity when there is + * no such root. * * @returns {?object} jQuery set, or null */ diff --git a/view/frontend/web/js/model/company-search.js b/view/frontend/web/js/model/company-search.js index 9c659006..e70ed8ef 100644 --- a/view/frontend/web/js/model/company-search.js +++ b/view/frontend/web/js/model/company-search.js @@ -43,8 +43,9 @@ define([ /** * Epoch ms before which no registry call is issued, keyed by the calling - * PANEL: a 429 one panel earns must not silence the other's searching or - * put an "address unavailable" notice on its identity (TWO-25554). + * panel's IDENTITY: a 429 one panel earns must not silence the other's + * searching or put an "address unavailable" notice on its identity + * (TWO-25554). * * @see RATE_LIMIT_BACKOFF_MS */ @@ -1016,8 +1017,8 @@ define([ * @param {object} options.token bind identity, so an abort raised * against a torn-down panel cannot cancel the live one's search * @param {object} options.scope the calling panel's rate-limit scope. - * Required: falling back to the bind token scoped the backoff to - * a token a re-render replaces, i.e. to no backoff at all + * Required, and never the bind token: a re-render replaces that + * token, so a backoff scoped to one is never observed * (TWO-25554). * @returns {Promise<{items: Array, unavailable: boolean, aborted: boolean}>} */ diff --git a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js index c546e938..f5973122 100644 --- a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js +++ b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js @@ -77,9 +77,9 @@ define([ */ const tileMountRevision = ko.observable(0); - // Bumped from inside the component's own re-point, so the tile stops - // rendering a second company field on every event that moves the mount and - // not only on the ones a caller remembers to report (TWO-25554). + // Bumped from inside the component's own re-point: one control per field + // has to hold for every event that moves the mount, not only the ones a + // caller remembers to report (TWO-25554). companyCapture.shipping.subscribeMount(function () { tileMountRevision(tileMountRevision() + 1); }); From 70fad6059dc1f24152e80df3c9b3a484161eb178 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:20:31 +0100 Subject: [PATCH 14/30] test(TWO-25554): flush the propagation each new denial denies Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-capture-billing-panel.test.js | 19 +++++++++++++++++-- Test/Js/company-search-address-lookup.test.js | 2 ++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/Test/Js/company-capture-billing-panel.test.js b/Test/Js/company-capture-billing-panel.test.js index a692dd9e..f974af93 100644 --- a/Test/Js/company-capture-billing-panel.test.js +++ b/Test/Js/company-capture-billing-panel.test.js @@ -537,7 +537,20 @@ describe('the quote\'s billing address seeds the panel owning the billing role', dom.fireChange('input[name="billing-address-same-as-shipping"]'); } - test('with the fieldset transiently away, a distinct billing address still seeds BILLING', () => { + /** + * Two macrotasks. `watchCapturedIdentity` publishes on `setTimeout(0)`, so a + * denial made before it has run denies a propagation that had not happened. + * + * @returns {Promise} + */ + function flushCapture() { + return new Promise(function (resolve) { setTimeout(resolve, 0); }) + .then(function () { + return new Promise(function (resolve) { setTimeout(resolve, 0); }); + }); + } + + test('with the fieldset transiently away, a distinct billing address still seeds BILLING', async () => { // The quote answers "is billing a distinct address", not the fieldset's // visibility: a third-party re-render that takes the fieldset away for a // moment otherwise puts billing's company in the shipping panel's own @@ -548,6 +561,7 @@ describe('the quote\'s billing address seeds the panel owning the billing role', billingFieldsetAway(dom, capture); renderer.updateBillingAddress(billingQuoteAddress('Billing Co', 'billing-of-its-own')); + await flushCapture(); expect(capture.billing.identity().companyName()).toBe('Billing Co'); expect(capture.billing.identity().companyId()).toBe('222'); @@ -567,13 +581,14 @@ describe('the quote\'s billing address seeds the panel owning the billing role', expect(capture.shipping.identity().companyId()).toBe('222'); }); - test('re-checking "same as shipping" retires the billing panel\'s own capture', () => { + test('re-checking "same as shipping" retires the billing panel\'s own capture', async () => { const { capture, dom } = load(); billingPicks(capture, dom, 'Billing Co'); capture.billing.identity().soleTraderAdopted(true); capture.billing.identity().captureMode('soletrader'); sameAsShippingAgain(capture, dom); + await flushCapture(); expect(capture.billing.identity().companyName()).toBe(''); expect(capture.billing.identity().companyId()).toBe(''); diff --git a/Test/Js/company-search-address-lookup.test.js b/Test/Js/company-search-address-lookup.test.js index eea27674..9371d7bb 100644 --- a/Test/Js/company-search-address-lookup.test.js +++ b/Test/Js/company-search-address-lookup.test.js @@ -509,6 +509,8 @@ describe('a tile-mounted shipping panel and the one address form there is (TWO-2 expect(component.mountSelector()).toBe(TILE_FIELD_SELECTOR); await pick('example', SEARCH_RESPONSE); + await new Promise(function (resolve) { setTimeout(resolve, 0); }); + await new Promise(function (resolve) { setTimeout(resolve, 0); }); expect(lookupIds(recorder)).toEqual([]); expect(recorder.written).toHaveLength(0); From 675a5b668ab69e51ee08d94104c48885e072c739 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:16:15 +0100 Subject: [PATCH 15/30] test(TWO-25554): pin the retirement where it happens, not after an unrelated one Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-capture-billing-panel.test.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Test/Js/company-capture-billing-panel.test.js b/Test/Js/company-capture-billing-panel.test.js index f974af93..bd624dbc 100644 --- a/Test/Js/company-capture-billing-panel.test.js +++ b/Test/Js/company-capture-billing-panel.test.js @@ -588,12 +588,18 @@ describe('the quote\'s billing address seeds the panel owning the billing role', capture.billing.identity().captureMode('soletrader'); sameAsShippingAgain(capture, dom); - await flushCapture(); + // Synchronously, in the checkbox handler itself. A later availability + // resolution retires an adoption too, for its own reason, and asserting + // only after that would pin nothing about the retirement. expect(capture.billing.identity().companyName()).toBe(''); expect(capture.billing.identity().companyId()).toBe(''); expect(capture.billing.identity().soleTraderAdopted()).toBe(false); expect(capture.billing.identity().captureMode()).toBe('registered'); + + await flushCapture(); + expect(capture.billing.identity().companyName()).toBe(''); + expect(capture.billing.identity().soleTraderAdopted()).toBe(false); }); test('after that re-check the returning buyer\'s saved company seeds SHIPPING, not billing', () => { From 72b011debac33c78ca341c2950f2c4393e785a4f Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:18:21 +0100 Subject: [PATCH 16/30] docs(TWO-25554): collapse five comments to the invariant each carries Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-independence.test.js | 10 ++++------ .../web/js/model/company-capture-component.js | 8 +++----- view/frontend/web/js/model/company-identity.js | 13 ++++++------- view/frontend/web/js/model/company-search.js | 6 +++--- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 92c31a98..768ef786 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -504,12 +504,10 @@ describe('the quote\'s billing address belongs to the billing panel', () => { }); test('a virtual cart with no billing form rendered still leaves shipping empty', () => { - // The quote holds one address and it is the billing one, so the billing - // identity is where it lands whether or not a form for it is on the page - // — the destination follows the quote, never the DOM (TWO-25554). With - // no form rendered the resolver reads the shipping capture, so the - // company is not offered back to this buyer; the alternative is worse, - // because it paints a billing company into the shipping panel's field. + // The destination follows the quote, never the DOM (TWO-25554). With no + // billing form rendered the resolver reads the shipping capture, so this + // buyer is not offered the company back — routing it to shipping to + // avoid that paints a billing company into the shipping panel's field. const booted = boot({ isVirtual: true, shippingForm: false, diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index 5787a317..1b921202 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -448,7 +448,7 @@ const selector = this.mountSelector(); const previous = this._boundSelector; if (!selector) { - // Before the selector is forgotten: every chrome lookup is made + // Before the selector is forgotten — every chrome lookup goes // through the bound field. this._removeChrome(); // Neither host is on the page any more. Forgetting where the control @@ -601,10 +601,8 @@ }; /** - * Take this panel's chrome back off the page. - * - * The chrome is a SIBLING of the wrapper, so `unmount()` does not carry it - * away, and its button drives a flow the same call tears down (TWO-25554). + * Take this panel's chrome back off the page. A SIBLING of the wrapper, so + * `unmount()` leaves it standing over a torn-down flow (TWO-25554). */ CompanyCaptureComponent.prototype._removeChrome = function () { const self = this; diff --git a/view/frontend/web/js/model/company-identity.js b/view/frontend/web/js/model/company-identity.js index 466f0ea4..383fb15f 100644 --- a/view/frontend/web/js/model/company-identity.js +++ b/view/frontend/web/js/model/company-identity.js @@ -200,14 +200,13 @@ }, /** - * Retire the whole capture — a country change invalidates the - * registry, and re-checking "same as shipping" retires the panel. + * Retire the whole capture. The mode and the adoption go with the + * name/number pair: a retired capture left in sole-trader mode + * remounts the panel over an empty identity, and the resolver reads + * an adoption as a company number (TWO-25554). * - * The mode and the adoption go with the pair: a retired capture - * left in sole-trader mode remounts the panel over an empty - * identity, and the resolver reads an adoption flag as a company - * number (TWO-25554). `soleTraderAvailable` is a property of the - * country, not of the capture, and stays. + * `soleTraderAvailable` is a property of the country rather than of + * the capture, and stays. */ clear: function () { if (!state.companyName && !state.companyId && !state.companyIdSource diff --git a/view/frontend/web/js/model/company-search.js b/view/frontend/web/js/model/company-search.js index e70ed8ef..f969d238 100644 --- a/view/frontend/web/js/model/company-search.js +++ b/view/frontend/web/js/model/company-search.js @@ -119,9 +119,9 @@ define([ * and a country switch after such a rebuild still has to retract the * previous country's address rather than leave it standing (TWO-25554). * - * Keyed on the calling panel's IDENTITY, which is per-panel and unreachable - * from the other one, and which a one-step checkout replacing the whole - * payment-methods subtree — where the billing form lives — does not replace. + * Keyed on the calling panel's IDENTITY: unreachable from the other panel, + * and it survives a one-step checkout replacing the payment-methods + * subtree the billing form lives in. */ const addressWriteRecords = new WeakMap(); From 51632c0b03b4c2282e7c36884a180c600a18fed6 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:44:22 +0100 Subject: [PATCH 17/30] test(TWO-25554): let every quote double answer the cache-key comparison Co-Authored-By: Claude Sonnet 5 --- Test/Js/address-step-company-id-text.test.js | 5 ++- Test/Js/amd-harness.js | 37 ++++++++++++++++++- ...ompany-capture-component-lifecycle.test.js | 10 +++-- .../Js/company-capture-signup-prefill.test.js | 24 ++++++++++-- Test/Js/company-panel-chrome.test.js | 5 ++- Test/Js/company-search-address-lookup.test.js | 5 ++- Test/Js/company-search-country-switch.test.js | 25 +++++++++---- ...mpany-search-tile-country-sourcing.test.js | 10 +++-- .../gateway-method-order-intent-proxy.test.js | 13 +++++-- ...y-method-order-intent-request-body.test.js | 15 ++++---- .../gateway-method-sole-trader-popup.test.js | 11 ++++-- ...ethod-sole-trader-select-different.test.js | 5 ++- Test/Js/tile-company-readonly-fields.test.js | 10 ++++- 13 files changed, 129 insertions(+), 46 deletions(-) diff --git a/Test/Js/address-step-company-id-text.test.js b/Test/Js/address-step-company-id-text.test.js index 34ce99ab..a4d4061e 100644 --- a/Test/Js/address-step-company-id-text.test.js +++ b/Test/Js/address-step-company-id-text.test.js @@ -34,7 +34,8 @@ const { loadCompanySearchPanel, defaultMocks, brandConfigMock, - installAsyncSimulation + installAsyncSimulation, + quoteAddress } = require('./amd-harness'); const SEARCH = 'view/frontend/web/js/model/company-search.js'; @@ -82,7 +83,7 @@ function load() { {}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { return { countryId: 'NO' }; }, + billingAddress: quoteAddress({ countryId: 'NO' }), isVirtual: function () { return false; } } ), diff --git a/Test/Js/amd-harness.js b/Test/Js/amd-harness.js index 8d6eced2..e91e4f1c 100644 --- a/Test/Js/amd-harness.js +++ b/Test/Js/amd-harness.js @@ -87,8 +87,8 @@ function defaultMocks() { // One cache key for both: the quote is what answers "is billing a // distinct address", so a double with no key at all cannot model // either answer. Same key means billing IS shipping. - shippingAddress: makeObservable({ getCacheKey: function () { return 'one-address'; } }), - billingAddress: makeObservable({ getCacheKey: function () { return 'one-address'; } }), + shippingAddress: quoteAddress(), + billingAddress: quoteAddress(), getTotals: function () { return makeObservable({}); }, getQuoteId: function () { return null; }, paymentMethod: makeObservable(null), @@ -312,6 +312,36 @@ function makeKnockoutMock() { }; } +/** The cache key both default quote addresses answer with: billing IS shipping. */ +const ONE_ADDRESS_KEY = 'one-address'; + +/** + * A quote address observable of the shape `company-capture.js` reads it in: a + * cache key it can be compared with the other address on, and a `subscribe` the + * predicate's invalidation is wired to. A double supplying neither cannot model + * "is billing a distinct address" at all, and throws where production asks. + * + * @param {object} [fields] address fields the spec itself needs + * @param {string} [cacheKey] defaults to the key shippingAddress also answers + * @returns {function} Knockout-shaped observable + */ +function quoteAddress(fields, cacheKey) { + return makeObservable(quoteAddressValue(fields, cacheKey)); +} + +/** + * The value inside a quoteAddress() observable, for a spec that writes a NEW + * address into one mid-test. + * + * @param {object} [fields] address fields the spec itself needs + * @param {string} [cacheKey] defaults to the key shippingAddress also answers + * @returns {object} + */ +function quoteAddressValue(fields, cacheKey) { + const key = cacheKey || ONE_ADDRESS_KEY; + return Object.assign({ getCacheKey: function () { return key; } }, fields || {}); +} + function makeObservable(initial) { let value = initial; const subscribers = []; @@ -848,6 +878,9 @@ function tagged(description, value) { module.exports = { tagged: tagged, + quoteAddress: quoteAddress, + quoteAddressValue: quoteAddressValue, + makeObservable: makeObservable, dispatchNative: dispatchNative, isProxyRoute: isProxyRoute, HARNESS_BASE_URL: HARNESS_BASE_URL, diff --git a/Test/Js/company-capture-component-lifecycle.test.js b/Test/Js/company-capture-component-lifecycle.test.js index 0f50690d..6747c8f1 100644 --- a/Test/Js/company-capture-component-lifecycle.test.js +++ b/Test/Js/company-capture-component-lifecycle.test.js @@ -32,7 +32,9 @@ const { loadCompanySearchPanel, brandConfigMock, defaultMocks, - installAsyncSimulation + installAsyncSimulation, + quoteAddress, + makeObservable } = require('./amd-harness'); const CONTROLLER = 'view/frontend/web/js/model/company-capture-component.js'; @@ -175,9 +177,9 @@ function load(options) { // `deferCountry` reproduces a guest checkout at boot: the // quote carries no address yet, so the country is only // readable once a form exists to read it from. - billingAddress: function () { - return opts.deferCountry ? null : { countryId: 'GB' }; - } + billingAddress: opts.deferCountry + ? makeObservable(null) + : quoteAddress({ countryId: 'GB' }) } ), 'Two_Gateway/js/model/company-search': companySearchMock diff --git a/Test/Js/company-capture-signup-prefill.test.js b/Test/Js/company-capture-signup-prefill.test.js index 7de0901f..d437f1d4 100644 --- a/Test/Js/company-capture-signup-prefill.test.js +++ b/Test/Js/company-capture-signup-prefill.test.js @@ -11,7 +11,25 @@ 'use strict'; const $ = require('jquery'); -const { loadCompanyCapture, brandConfigMock, defaultMocks } = require('./amd-harness'); +const { + loadCompanyCapture, + brandConfigMock, + defaultMocks, + quoteAddress, + makeObservable +} = require('./amd-harness'); + +/** + * The quote's billing address as an observable carrying a cache key, so the + * capture adapter can compare it with the shipping address the default quote + * double also holds. + * + * @param {?object} address what the spec wants the quote to hold + * @returns {function} Knockout-shaped observable + */ +function quoteObservable(address) { + return address === null ? makeObservable(null) : quoteAddress(address); +} /** * @returns {object} the shipping panel — signupPrefill() carries the company of @@ -28,7 +46,7 @@ function load(billingAddress, guestEmail) { {}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { return billingAddress; }, + billingAddress: quoteObservable(billingAddress), guestEmail: guestEmail } ) @@ -49,7 +67,7 @@ function loadBoth(billingAddress) { 'Magento_Checkout/js/model/quote': Object.assign( {}, defaultMocks()['Magento_Checkout/js/model/quote'], - { billingAddress: function () { return billingAddress; } } + { billingAddress: quoteObservable(billingAddress) } ) }); return capture; diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index 7ba0b0de..a9cb521d 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -23,7 +23,8 @@ const { defaultMocks, brandConfigMock, installAsyncSimulation, - tagged + tagged, + quoteAddress } = require('./amd-harness'); const SEARCH = 'view/frontend/web/js/model/company-search.js'; @@ -145,7 +146,7 @@ function boot(options) { {}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { return { countryId: 'GB' }; }, + billingAddress: quoteAddress({ countryId: 'GB' }, 'billing'), isVirtual: function () { return false; } } ), diff --git a/Test/Js/company-search-address-lookup.test.js b/Test/Js/company-search-address-lookup.test.js index 9371d7bb..fa1a8c77 100644 --- a/Test/Js/company-search-address-lookup.test.js +++ b/Test/Js/company-search-address-lookup.test.js @@ -21,7 +21,8 @@ const { isProxyRoute, proxyEnvelope, HARNESS_BASE_URL, - tagged + tagged, + quoteAddress } = require('./amd-harness'); const IDENTITY = 'view/frontend/web/js/model/company-identity.js'; @@ -365,7 +366,7 @@ function loadMountedComponent(configOverride, present) { 'Magento_Checkout/js/model/quote': Object.assign( {}, defaultMocks()['Magento_Checkout/js/model/quote'], - { billingAddress: function () { return { countryId: 'GB' }; } } + { billingAddress: quoteAddress({ countryId: 'GB' }) } ) }).shipping; component.start(); diff --git a/Test/Js/company-search-country-switch.test.js b/Test/Js/company-search-country-switch.test.js index dc408c78..68381799 100644 --- a/Test/Js/company-search-country-switch.test.js +++ b/Test/Js/company-search-country-switch.test.js @@ -32,7 +32,15 @@ 'use strict'; const jq = require('jquery'); -const { loadAmdModule, defaultMocks, loadCompanyCapture, brandConfigMock } = require('./amd-harness'); +const { + loadAmdModule, + defaultMocks, + loadCompanyCapture, + brandConfigMock, + quoteAddress, + quoteAddressValue, + makeObservable +} = require('./amd-harness'); const MODEL = 'view/frontend/web/js/model/company-search.js'; const ADDRESS_STEP = 'view/frontend/web/js/view/address-autocomplete.js'; @@ -371,11 +379,12 @@ function loadCaptureComponent(options) { this.forgetAdoptions = function () { calls.forgotten += 1; }; } - let billing = 'billingCountry' in opts ? opts.billingCountry : 'GB'; + const billing = 'billingCountry' in opts ? opts.billingCountry : 'GB'; + const billingAddress = billing === null + ? makeObservable(null) + : quoteAddress({ countryId: billing }); const quote = Object.assign({}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { - return billing === null ? null : { countryId: billing }; - }, + billingAddress: billingAddress, isVirtual: function () { return false; } }); @@ -412,7 +421,9 @@ function loadCaptureComponent(options) { component: component, identity: component.identity(), calls: calls, - setBillingCountry: function (iso) { billing = iso; } + setBillingCountry: function (iso) { + billingAddress(iso === null ? null : quoteAddressValue({ countryId: iso })); + } }; } @@ -431,7 +442,7 @@ function loadRenderer(billingCountry) { // such checkout renders. dom.node('#shipping-new-address-form input[name="company"]').length = 0; const quote = Object.assign({}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { return { countryId: billingCountry }; } + billingAddress: quoteAddress({ countryId: billingCountry }) }); const renderer = loadAmdModule(RENDERER, { jquery: dom.$, diff --git a/Test/Js/company-search-tile-country-sourcing.test.js b/Test/Js/company-search-tile-country-sourcing.test.js index 721a7760..c40c9533 100644 --- a/Test/Js/company-search-tile-country-sourcing.test.js +++ b/Test/Js/company-search-tile-country-sourcing.test.js @@ -33,7 +33,9 @@ const { defaultMocks, loadCompanyCapture, brandConfigMock, - tagged + tagged, + quoteAddress, + makeObservable } = require('./amd-harness'); const IDENTITY = 'view/frontend/web/js/model/company-identity.js'; @@ -90,9 +92,9 @@ function load(options) { const billing = 'billingCountry' in opts ? opts.billingCountry : null; const quote = Object.assign({}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { - return billing === null ? null : { countryId: billing }; - }, + billingAddress: billing === null + ? makeObservable(null) + : quoteAddress({ countryId: billing }), isVirtual: function () { return !!opts.isVirtual; } }); diff --git a/Test/Js/gateway-method-order-intent-proxy.test.js b/Test/Js/gateway-method-order-intent-proxy.test.js index a8e3723c..12608e23 100644 --- a/Test/Js/gateway-method-order-intent-proxy.test.js +++ b/Test/Js/gateway-method-order-intent-proxy.test.js @@ -9,7 +9,13 @@ 'use strict'; const jq = require('jquery'); -const { loadAmdModule, defaultMocks, proxyEnvelope, HARNESS_BASE_URL } = require('./amd-harness'); +const { + loadAmdModule, + defaultMocks, + proxyEnvelope, + HARNESS_BASE_URL, + quoteAddress +} = require('./amd-harness'); const RENDERER = 'view/frontend/web/js/view/payment/method-renderer/gateway_method.js'; // The module's own last-resort copy, and a server message deliberately UNLIKE @@ -45,9 +51,8 @@ function loadRenderer() { }; const quote = { getTotals: function () { return function () { return totals; }; }, - billingAddress: function () { - return { countryId: 'NO', firstname: 'Ola', lastname: 'Nordmann' }; - }, + shippingAddress: quoteAddress(), + billingAddress: quoteAddress({ countryId: 'NO', firstname: 'Ola', lastname: 'Nordmann' }), getItems: function () { return []; } }; diff --git a/Test/Js/gateway-method-order-intent-request-body.test.js b/Test/Js/gateway-method-order-intent-request-body.test.js index a7b367fa..5bda9129 100644 --- a/Test/Js/gateway-method-order-intent-request-body.test.js +++ b/Test/Js/gateway-method-order-intent-request-body.test.js @@ -21,7 +21,7 @@ 'use strict'; -const { loadAmdModule, defaultMocks } = require('./amd-harness'); +const { loadAmdModule, defaultMocks, quoteAddress } = require('./amd-harness'); const RENDERER = 'view/frontend/web/js/view/payment/method-renderer/gateway_method.js'; @@ -71,13 +71,12 @@ function loadRenderer() { const quote = { getTotals: function () { return function () { return totals; }; }, - billingAddress: function () { - return { - countryId: 'NO', - firstname: 'Ola', - lastname: 'Nordmann' - }; - }, + shippingAddress: quoteAddress(), + billingAddress: quoteAddress({ + countryId: 'NO', + firstname: 'Ola', + lastname: 'Nordmann' + }), getItems: function () { return [ { diff --git a/Test/Js/gateway-method-sole-trader-popup.test.js b/Test/Js/gateway-method-sole-trader-popup.test.js index e1b5d700..875704cf 100644 --- a/Test/Js/gateway-method-sole-trader-popup.test.js +++ b/Test/Js/gateway-method-sole-trader-popup.test.js @@ -37,7 +37,9 @@ const { defaultMocks, loadCompanySearchPanel, dispatchNative, - brandConfigMock + brandConfigMock, + quoteAddress, + makeObservable } = require('./amd-harness'); const IDENTITY = 'view/frontend/web/js/model/company-identity.js'; @@ -89,9 +91,10 @@ function makeEnv(options) { }; const quote = Object.assign({}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { - return 'billingAddress' in opts ? opts.billingAddress : { countryId: 'GB' }; - }, + billingAddress: (function () { + const address = 'billingAddress' in opts ? opts.billingAddress : { countryId: 'GB' }; + return address ? quoteAddress(address) : makeObservable(address); + })(), getQuoteId: function () { return 'cart-1'; }, isVirtual: function () { return false; } }); diff --git a/Test/Js/gateway-method-sole-trader-select-different.test.js b/Test/Js/gateway-method-sole-trader-select-different.test.js index 69f104f0..d2b012d6 100644 --- a/Test/Js/gateway-method-sole-trader-select-different.test.js +++ b/Test/Js/gateway-method-sole-trader-select-different.test.js @@ -25,7 +25,8 @@ const { defaultMocks, loadCompanySearchPanel, dispatchNative, - brandConfigMock + brandConfigMock, + quoteAddress } = require('./amd-harness'); const SOLE_TRADER = 'view/frontend/web/js/model/sole-trader.js'; @@ -57,7 +58,7 @@ function makeEnv() { {}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { return { countryId: 'GB' }; }, + billingAddress: quoteAddress({ countryId: 'GB' }), getQuoteId: function () { return 'cart-1'; }, isVirtual: function () { return false; } } diff --git a/Test/Js/tile-company-readonly-fields.test.js b/Test/Js/tile-company-readonly-fields.test.js index 61884c6a..70fd781b 100644 --- a/Test/Js/tile-company-readonly-fields.test.js +++ b/Test/Js/tile-company-readonly-fields.test.js @@ -54,7 +54,13 @@ const fs = require('fs'); const path = require('path'); -const { loadAmdModule, defaultMocks, loadCompanyCapture, brandConfigMock } = require('./amd-harness'); +const { + loadAmdModule, + defaultMocks, + loadCompanyCapture, + brandConfigMock, + quoteAddress +} = require('./amd-harness'); const RENDERER = 'view/frontend/web/js/view/payment/method-renderer/gateway_method.js'; const IDENTITY = 'view/frontend/web/js/model/company-identity.js'; @@ -422,7 +428,7 @@ function loadTile() { 'Magento_Checkout/js/model/quote': Object.assign( {}, defaultMocks()['Magento_Checkout/js/model/quote'], - { billingAddress: function () { return { countryId: 'GB' }; } } + { billingAddress: quoteAddress({ countryId: 'GB' }) } ) }) ); From f2aa22738f46217c9030868323685eb53456ca7e Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:44:41 +0100 Subject: [PATCH 18/30] fix(TWO-25554): answer "is billing distinct" from the checkbox and the quote The resolver and the quote-driven seed each had their own answer, and a checkout holding a distinct billing address while rendering no billing company field stranded the buyer's saved company on a panel nothing reads. Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-capture-billing-panel.test.js | 255 +++++++++++++----- Test/Js/company-panel-independence.test.js | 43 ++- .../gateway-method-company-selection.test.js | 51 +++- view/frontend/web/js/model/company-capture.js | 43 ++- .../web/js/model/company-source-resolver.js | 4 +- .../payment/method-renderer/gateway_method.js | 19 +- 6 files changed, 293 insertions(+), 122 deletions(-) diff --git a/Test/Js/company-capture-billing-panel.test.js b/Test/Js/company-capture-billing-panel.test.js index bd624dbc..5bd02c35 100644 --- a/Test/Js/company-capture-billing-panel.test.js +++ b/Test/Js/company-capture-billing-panel.test.js @@ -14,7 +14,9 @@ const { loadAmdModule, loadCompanyCapture, defaultMocks, - brandConfigMock + brandConfigMock, + quoteAddress, + quoteAddressValue } = require('./amd-harness'); const ADDRESS_FORM = '#shipping-new-address-form'; @@ -23,6 +25,10 @@ const ADDRESS_COUNTRY = `${ADDRESS_FORM} select[name="country_id"]`; const BILLING_FORM = '[data-form="billing-new-address"]'; const BILLING_FIELD = `${BILLING_FORM} input[name="company"]`; const BILLING_COUNTRY = `${BILLING_FORM} select[name="country_id"]`; +const BILLING_TOGGLE = 'input[name="billing-address-same-as-shipping"]'; + +/** The cache key that makes the quote's billing address its own, not shipping's. */ +const DISTINCT_BILLING_KEY = 'billing-of-its-own'; /** * A minimal jQuery-shaped double over a fixed set of named nodes, each with a @@ -45,6 +51,7 @@ function makeDom() { let visible = true; let exists = true; let value = ''; + const props = {}; const delegated = []; const n = { get length() { @@ -58,6 +65,11 @@ function makeDom() { is: function (expr) { return expr === ':visible' ? visible : false; }, + prop: function (name, next) { + if (arguments.length < 2) return props[name]; + props[name] = next; + return n; + }, filter: function () { return visible ? n : { length: 0 }; }, @@ -84,6 +96,9 @@ function makeDom() { _setExists: function (v) { exists = v; }, + _setProp: function (name, v) { + props[name] = v; + }, _fireDelegated: function (event, selector) { delegated .filter(function (d) { return d.event === event && d.selector === selector; }) @@ -110,6 +125,9 @@ function makeDom() { setExists: function (selector, value) { node(selector)._setExists(value); }, + setChecked: function (selector, value) { + node(selector)._setProp('checked', value); + }, setCountry: function (selector, value) { node(selector).val(value); }, @@ -124,20 +142,29 @@ function makeDom() { /** * @param {object} [overrides] merged over the standard mocks - * @returns {object} `{ capture, dom }` + * @returns {object} `{ capture, dom, quote }` — the quote's two addresses share + * a cache key, so billing starts as shipping, matching the checked + * checkbox and the absent billing form below */ function load(overrides) { const dom = makeDom(); // Absent until made visible — matches core rendering no billing form at // all under "same as shipping" (checked, the default). dom.setVisible(BILLING_FIELD, false); + dom.setChecked(BILLING_TOGGLE, true); dom.setCountry(ADDRESS_COUNTRY, 'no'); dom.setCountry(BILLING_COUNTRY, 'gb'); + const quote = Object.assign({}, defaultMocks()['Magento_Checkout/js/model/quote'], { + shippingAddress: quoteAddress(), + billingAddress: quoteAddress() + }); + const capture = loadCompanyCapture( Object.assign( { jquery: dom.$, + 'Magento_Checkout/js/model/quote': quote, 'Two_Gateway/js/model/brand-config': brandConfigMock({ isCompanySearchEnabled: true, checkoutApiUrl: 'https://api.example.test', @@ -148,7 +175,20 @@ function load(overrides) { ), { document: document, window: window } ); - return { capture: capture, dom: dom }; + return { capture: capture, dom: dom, quote: quote }; +} + +/** + * The buyer unchecks "my billing address is the same as shipping", core renders + * the billing fieldset, and the quote takes on a second address. + * + * @param {object} dom + * @param {object} quote + */ +function billingBecomesDistinct(dom, quote) { + dom.setChecked(BILLING_TOGGLE, false); + dom.setVisible(BILLING_FIELD, true); + quote.billingAddress(quoteAddressValue({}, DISTINCT_BILLING_KEY)); } describe('the billing panel only ever mounts at its own field', () => { @@ -160,8 +200,8 @@ describe('the billing panel only ever mounts at its own field', () => { }); test('visible (unchecked): billing mounts at its own field', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.billing.start(); expect(capture.billing.mountSelector()).toBe(BILLING_FIELD); @@ -170,8 +210,8 @@ describe('the billing panel only ever mounts at its own field', () => { test('present but hidden (re-checked after being unchecked): billing does not mount', () => { // TWO-25461's own finding, reused here: core can leave the billing // form in the DOM hidden rather than removing it. - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.billing.start(); expect(capture.billing.mountSelector()).toBe(BILLING_FIELD); @@ -184,8 +224,8 @@ describe('the billing panel only ever mounts at its own field', () => { describe('each panel reads ONLY its own address form\'s country — never a shared one', () => { test('billing reads the billing form\'s country, not shipping\'s, even though they differ', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.billing.start(); expect(capture.billing.countryCode()).toBe('gb'); @@ -193,8 +233,8 @@ describe('each panel reads ONLY its own address form\'s country — never a shar }); test('a shipping country change does not move billing\'s answer, and vice versa', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.shipping.start(); capture.billing.start(); @@ -208,8 +248,8 @@ describe('each panel reads ONLY its own address form\'s country — never a shar describe('the two panels\' captures are independent — a pick on one never reaches the other', () => { test('a registered pick on shipping leaves billing\'s own identity untouched', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.shipping.start(); capture.billing.start(); @@ -220,8 +260,8 @@ describe('the two panels\' captures are independent — a pick on one never reac }); test('a registered pick on billing leaves shipping\'s own identity untouched', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.shipping.start(); capture.billing.start(); @@ -244,13 +284,13 @@ describe('the resolved identity, end to end, follows the resolution rule live', }); test('billing distinct with a number: the resolved identity switches to billing\'s pick', () => { - const { capture, dom } = load(); + const { capture, dom, quote } = load(); capture.shipping.start(); capture.billing.start(); capture.shipping.selectCompany({ text: 'Shipping Co', companyId: '111', lookupId: 'l1' }); expect(capture.identity.companyId()).toBe('111'); - dom.setVisible(BILLING_FIELD, true); + billingBecomesDistinct(dom, quote); capture.billing.refreshMount(); capture.billing.selectCompany({ text: 'Billing Co', companyId: '222', lookupId: 'l2' }); @@ -258,8 +298,8 @@ describe('the resolved identity, end to end, follows the resolution rule live', }); test('billing distinct but manual entry: the resolved identity falls back to shipping', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.shipping.start(); capture.billing.start(); capture.shipping.selectCompany({ text: 'Shipping Co', companyId: '111', lookupId: 'l1' }); @@ -310,7 +350,7 @@ describe('a checkbox toggle mid-checkout supersedes the order-intent already in } test('unchecking mid-flow starts a fresh order-intent for billing\'s company, and the old company\'s stale response is dropped', () => { - const { capture, dom } = load(); + const { capture, dom, quote } = load(); capture.shipping.start(); capture.billing.start(); @@ -324,7 +364,7 @@ describe('a checkbox toggle mid-checkout supersedes the order-intent already in expect(requests[0].companyId).toBe('111'); // Billing becomes distinct, with its own company, mid-checkout. - dom.setVisible(BILLING_FIELD, true); + billingBecomesDistinct(dom, quote); capture.billing.refreshMount(); capture.billing.selectCompany({ text: 'Billing Co', companyId: '222', lookupId: 'l2' }); @@ -445,20 +485,20 @@ describe('the "same as shipping" checkbox toggle re-checks both panels\' mounts' // every call and would read as mounted even when refreshMount() was // never re-driven at all (the exact vacuous read this pins against). test('billing mounts once revealed, even though its field already existed hidden at boot', () => { - const { capture, dom } = load(); + const { capture, dom, quote } = load(); capture.start(); expect(capture.billing.panel()).toBeNull(); - dom.setVisible(BILLING_FIELD, true); + billingBecomesDistinct(dom, quote); dom.fireChange(BILLING_TOGGLE); expect(capture.billing.panel()).not.toBeNull(); }); test('unmounts again once re-hidden, same as an explicit refreshMount() already does', () => { - const { capture, dom } = load(); + const { capture, dom, quote } = load(); capture.start(); - dom.setVisible(BILLING_FIELD, true); + billingBecomesDistinct(dom, quote); dom.fireChange(BILLING_TOGGLE); expect(capture.billing.panel()).not.toBeNull(); @@ -477,9 +517,10 @@ describe('the "same as shipping" checkbox toggle re-checks both panels\' mounts' * otherwise — the only capture the resolver reads then, so seeding the billing * panel discards a saved company the buyer may not be able to re-search. * - * Distinctness is the live DOM answer, so a checkout whose billing fieldset is - * away at the moment the quote notifies seeds shipping. The "same as shipping" - * checkbox is what retires billing's own capture, and it is exercised here. + * Distinctness is the buyer's checkbox and the quote's own two addresses, so a + * checkout whose billing fieldset is away at the moment the quote notifies still + * seeds billing. The checkbox is what retires billing's own capture, and it is + * exercised here. */ describe('the quote\'s billing address seeds the panel owning the billing role', () => { const RENDERER = 'view/frontend/web/js/view/payment/method-renderer/gateway_method.js'; @@ -504,10 +545,10 @@ describe('the quote\'s billing address seeds the panel owning the billing role', * `capture.start()` — the checkbox listener that retires a stale billing * capture is wired there, so a per-component boot pins nothing about it. */ - function billingPicks(capture, dom, company) { - dom.setVisible(BILLING_FIELD, true); - capture.start(); - capture.billing.selectCompany({ text: company, companyId: '222', lookupId: 'l2' }); + function billingPicks(booted, company) { + billingBecomesDistinct(booted.dom, booted.quote); + booted.capture.start(); + booted.capture.billing.selectCompany({ text: company, companyId: '222', lookupId: 'l2' }); } /** What Fire's re-render (or a page that has not rendered one yet) leaves. */ @@ -517,24 +558,29 @@ describe('the quote\'s billing address seeds the panel owning the billing role', } /** + * The address the quote notifies with. Also put ON the quote, which is what + * the predicate reads — an address handed to the renderer that the quote + * does not hold is a state no checkout reaches. + * + * @param {object} booted * @param {string} company - * @param {string} [cacheKey] the quote's own answer to "is this a distinct - * address"; the harness quote's shipping key by default, i.e. not * @returns {object} quote address */ - function billingQuoteAddress(company, cacheKey) { - return { + function quoteNotifiesBilling(booted, company) { + const address = quoteAddressValue({ company: company, telephone: '+47 123 45 678', - customAttributes: [{ attribute_code: 'company_id', value: '222' }], - getCacheKey: function () { return cacheKey || 'one-address'; } - }; + customAttributes: [{ attribute_code: 'company_id', value: '222' }] + }, booted.quote.billingAddress().getCacheKey()); + booted.quote.billingAddress(address); + return address; } /** Core's own checkbox, re-checked: billing is shipping again. */ - function sameAsShippingAgain(capture, dom) { - dom.setVisible(BILLING_FIELD, false); - dom.fireChange('input[name="billing-address-same-as-shipping"]'); + function sameAsShippingAgain(booted) { + booted.dom.setVisible(BILLING_FIELD, false); + booted.dom.setChecked(BILLING_TOGGLE, true); + booted.dom.fireChange(BILLING_TOGGLE); } /** @@ -551,43 +597,85 @@ describe('the quote\'s billing address seeds the panel owning the billing role', } test('with the fieldset transiently away, a distinct billing address still seeds BILLING', async () => { - // The quote answers "is billing a distinct address", not the fieldset's - // visibility: a third-party re-render that takes the fieldset away for a - // moment otherwise puts billing's company in the shipping panel's own - // field (TWO-25554). - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + // A third-party re-render takes the fieldset away for a moment while + // neither the checkbox nor the quote has changed; routing on what is on + // screen puts billing's company in the shipping panel's own field + // (TWO-25554). + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, dom } = booted; const renderer = loadRenderer(capture, dom); billingFieldsetAway(dom, capture); - renderer.updateBillingAddress(billingQuoteAddress('Billing Co', 'billing-of-its-own')); + renderer.updateBillingAddress(quoteNotifiesBilling(booted, 'Saved Billing Co')); await flushCapture(); - expect(capture.billing.identity().companyName()).toBe('Billing Co'); + expect(capture.billing.identity().companyName()).toBe('Saved Billing Co'); expect(capture.billing.identity().companyId()).toBe('222'); expect(capture.shipping.identity().companyName()).toBe(''); expect(capture.shipping.identity().companyId()).toBe(''); }); + test('with the fieldset transiently away the resolver still reads BILLING', async () => { + // The seed and the resolver answer off ONE predicate, so the identity + // the seed lands on is the identity downstream reads. Split, this is the + // shape that stranded the company on a panel nobody reads. + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, dom } = booted; + const renderer = loadRenderer(capture, dom); + billingFieldsetAway(dom, capture); + + renderer.updateBillingAddress(quoteNotifiesBilling(booted, 'Saved Billing Co')); + await flushCapture(); + + expect(capture.identity.companyName()).toBe('Saved Billing Co'); + expect(capture.identity.companyId()).toBe('222'); + }); + + test('a returning buyer with no billing company field is still offered the saved company', async () => { + // A saved distinct billing address on a checkout that renders no billing + // company field at all: the seed lands on billing and the resolver reads + // billing, so the tile and order-intent see the company (TWO-25554). + const booted = load(); + const { capture, dom, quote } = booted; + dom.setChecked(BILLING_TOGGLE, false); + dom.setExists(BILLING_FIELD, false); + quote.billingAddress(quoteAddressValue({}, DISTINCT_BILLING_KEY)); + capture.start(); + expect(capture.billing.mountSelector()).toBe(''); + const renderer = loadRenderer(capture, dom); + + renderer.updateBillingAddress(quoteNotifiesBilling(booted, 'Saved Billing Co')); + await flushCapture(); + + expect(capture.identity.companyName()).toBe('Saved Billing Co'); + expect(capture.identity.companyId()).toBe('222'); + expect(capture.shipping.identity().companyName()).toBe(''); + }); + test('a billing address the quote says IS the shipping address seeds SHIPPING', () => { - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, dom, quote } = booted; const renderer = loadRenderer(capture, dom); billingFieldsetAway(dom, capture); + quote.billingAddress(quoteAddressValue()); - renderer.updateBillingAddress(billingQuoteAddress('Saved Co')); + renderer.updateBillingAddress(quoteNotifiesBilling(booted, 'Saved Co')); expect(capture.shipping.identity().companyName()).toBe('Saved Co'); expect(capture.shipping.identity().companyId()).toBe('222'); }); test('re-checking "same as shipping" retires the billing panel\'s own capture', async () => { - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture } = booted; capture.billing.identity().soleTraderAdopted(true); capture.billing.identity().captureMode('soletrader'); - sameAsShippingAgain(capture, dom); + sameAsShippingAgain(booted); // Synchronously, in the checkbox handler itself. A later availability // resolution retires an adoption too, for its own reason, and asserting @@ -602,18 +690,35 @@ describe('the quote\'s billing address seeds the panel owning the billing role', expect(capture.billing.identity().soleTraderAdopted()).toBe(false); }); + test('the checkbox retires the capture before the quote has dropped its second address', () => { + // The checkbox is the buyer saying so, and core updates the quote after + // it. Reading the quote alone leaves the retired panel still winning the + // resolution for as long as that lag lasts. + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, quote } = booted; + expect(capture.identity.companyName()).toBe('Billing Co'); + + sameAsShippingAgain(booted); + + expect(quote.billingAddress().getCacheKey()).toBe(DISTINCT_BILLING_KEY); + expect(capture.identity.companyName()).toBe(''); + }); + test('after that re-check the returning buyer\'s saved company seeds SHIPPING, not billing', () => { // A saved shipping address carries the company as a custom attribute // and reaches the panels only through the quote's billing address. A // billing capture still standing after the re-check routes that seed to // a panel the resolver does not read, and the tile and order-intent // then show nothing at all. - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, dom, quote } = booted; const renderer = loadRenderer(capture, dom); - sameAsShippingAgain(capture, dom); + sameAsShippingAgain(booted); + quote.billingAddress(quoteAddressValue()); - renderer.updateBillingAddress(billingQuoteAddress('Saved Shipping Co')); + renderer.updateBillingAddress(quoteNotifiesBilling(booted, 'Saved Shipping Co')); expect(capture.shipping.identity().companyName()).toBe('Saved Shipping Co'); expect(capture.shipping.identity().companyId()).toBe('222'); @@ -625,8 +730,9 @@ describe('the quote\'s billing address seeds the panel owning the billing role', // view/address-autocomplete.js, off the SHIPPING identity — so a row in // it is the shipping step's by construction, and is how a reload // restores it. - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, dom } = booted; const renderer = loadRenderer(capture, dom); billingFieldsetAway(dom, capture); @@ -640,12 +746,13 @@ describe('the quote\'s billing address seeds the panel owning the billing role', }); test('the telephone on that same billing address still travels', () => { - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, dom } = booted; const renderer = loadRenderer(capture, dom); billingFieldsetAway(dom, capture); - renderer.updateBillingAddress(billingQuoteAddress('Billing Co')); + renderer.updateBillingAddress(quoteNotifiesBilling(booted, 'Billing Co')); expect(renderer.telephone()).toBe('+47123 45 678'); }); @@ -654,12 +761,13 @@ describe('the quote\'s billing address seeds the panel owning the billing role', // Billing is not a distinct address here, so the shipping identity is // the only capture the resolver reads: seeding the billing panel would // discard a saved company the buyer cannot re-search (TWO-25554). - const { capture, dom } = load(); + const booted = load(); + const { capture, dom } = booted; capture.shipping.start(); capture.billing.start(); const renderer = loadRenderer(capture, dom); - renderer.updateBillingAddress(billingQuoteAddress('Some Other Co')); + renderer.updateBillingAddress(quoteNotifiesBilling(booted, 'Some Other Co')); expect(capture.shipping.identity().companyName()).toBe('Some Other Co'); expect(capture.shipping.identity().companyId()).toBe('222'); @@ -669,8 +777,9 @@ describe('the quote\'s billing address seeds the panel owning the billing role', test('the shipping step\'s own company still restores from the section while a billing panel is mounted', () => { // The section is how a reload restores the shipping company, and a // buyer with a distinct billing address must not lose that. - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + const booted = load(); + billingPicks(booted, 'Billing Co'); + const { capture, dom } = booted; const renderer = loadRenderer(capture, dom); renderer.applyCompanyData({ companyName: 'Shipping Co', companyId: '111' }); @@ -733,8 +842,8 @@ describe('a resolved-company change starts a check WITHOUT writing the shipping } test('a billing-only pick leaves the shipping identity empty', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.shipping.start(); capture.billing.start(); loadRendererWithIntent(capture, dom); @@ -747,8 +856,8 @@ describe('a resolved-company change starts a check WITHOUT writing the shipping }); test('and still starts the check for the company that actually resolved', () => { - const { capture, dom } = load(); - dom.setVisible(BILLING_FIELD, true); + const { capture, dom, quote } = load(); + billingBecomesDistinct(dom, quote); capture.shipping.start(); capture.billing.start(); const { requests } = loadRendererWithIntent(capture, dom); diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 768ef786..8bf35d8f 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -28,7 +28,9 @@ const { defaultMocks, brandConfigMock, installAsyncSimulation, - tagged + tagged, + quoteAddress, + makeObservable } = require('./amd-harness'); const SEARCH = 'view/frontend/web/js/model/company-search.js'; @@ -123,7 +125,8 @@ function renderCheckout(options) { * Both panels booted over the real modules, plus the real address step. * * @param {object} [options] `{ shippingForm, billingForm, shippingCountry, - * billingCountry, billingHidden, isVirtual, quoteBillingAddress }` + * billingCountry, billingHidden, isVirtual, quoteBillingAddress, + * quoteShippingAddress }` * @returns {object} `{ capture, search, panels, identities, addressStep, mocks }` */ function boot(options) { @@ -151,14 +154,14 @@ function boot(options) { const search = loadAmdModule(SEARCH, { jquery: $ }, GLOBALS); search.clearResultCache(); + // No shipping address unless a spec asks for one: the quote then holds + // billing alone, which no shipping address can be the same as. const quote = Object.assign( {}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { - return opts.quoteBillingAddress || { countryId: 'GB' }; - }, - shippingAddress: function () { return null; }, + billingAddress: quoteAddress(opts.quoteBillingAddress || { countryId: 'GB' }), + shippingAddress: makeObservable(opts.quoteShippingAddress || null), isVirtual: function () { return !!opts.isVirtual; } } ); @@ -503,11 +506,10 @@ describe('the quote\'s billing address belongs to the billing panel', () => { expect(renderer.telephone()).toBe('+4420 7946 0000'); }); - test('a virtual cart with no billing form rendered still leaves shipping empty', () => { - // The destination follows the quote, never the DOM (TWO-25554). With no - // billing form rendered the resolver reads the shipping capture, so this - // buyer is not offered the company back — routing it to shipping to - // avoid that paints a billing company into the shipping panel's field. + test('a virtual cart with no billing form rendered still offers the company back', () => { + // The seed and the resolver answer off ONE predicate, so a company that + // lands on billing is a company downstream reads — without painting it + // into the shipping panel's own field (TWO-25554). const booted = boot({ isVirtual: true, shippingForm: false, @@ -519,10 +521,29 @@ describe('the quote\'s billing address belongs to the billing panel', () => { renderer.updateBillingAddress(SAVED); expect(booted.identities.billing.companyId()).toBe('555'); + expect(booted.capture.identity.companyId()).toBe('555'); + expect(booted.capture.identity.companyName()).toBe('Saved Billing Co'); expect(booted.identities.shipping.companyId()).toBe(''); expect(booted.identities.shipping.companyName()).toBe(''); }); + test('a billing address the quote says IS the shipping address seeds SHIPPING', () => { + // Billing is not a distinct address, so the shipping identity is the + // only capture the resolver reads: seeding billing discards the company. + const booted = boot({ + billingForm: false, + quoteBillingAddress: SAVED, + quoteShippingAddress: { getCacheKey: function () { return 'billing'; } } + }); + const renderer = bootRenderer(booted); + + renderer.updateBillingAddress(SAVED); + + expect(booted.identities.shipping.companyId()).toBe('555'); + expect(booted.capture.identity.companyId()).toBe('555'); + expect(booted.identities.billing.companyId()).toBe(''); + }); + test('a billing address with no shipping panel mounted still leaves shipping empty', () => { const booted = boot({ shippingForm: false, quoteBillingAddress: SAVED }); const renderer = bootRenderer(booted); diff --git a/Test/Js/gateway-method-company-selection.test.js b/Test/Js/gateway-method-company-selection.test.js index 4809470a..54fe3422 100644 --- a/Test/Js/gateway-method-company-selection.test.js +++ b/Test/Js/gateway-method-company-selection.test.js @@ -24,7 +24,13 @@ 'use strict'; -const { loadAmdModule, defaultMocks, loadCompanyCapture, brandConfigMock } = require('./amd-harness'); +const { + loadAmdModule, + defaultMocks, + loadCompanyCapture, + brandConfigMock, + quoteAddress +} = require('./amd-harness'); const RENDERER = 'view/frontend/web/js/view/payment/method-renderer/gateway_method.js'; const IDENTITY = 'view/frontend/web/js/model/company-identity.js'; @@ -190,7 +196,7 @@ function loadRenderer() { const companySearch = loadAmdModule(SEARCH, { jquery: dom.$ }); const quote = Object.assign({}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { return { countryId: 'GB' }; } + billingAddress: quoteAddress({ countryId: 'GB' }) }); const shared = { jquery: dom.$, @@ -343,7 +349,7 @@ describe('a company picked on the shipping step reaches the payment step', () => const billingAddress = observable(address); const shippingAddress = observable(address); const intents = []; - const renderer = loadAmdModule(RENDERER, { + const mocks = { jquery: dom.$, 'Two_Gateway/js/model/company-identity': identity, 'Magento_Customer/js/customer-data': { @@ -366,13 +372,20 @@ describe('a company picked on the shipping step reaches the payment step', () => shippingMethod: observable({ carrier_code: 'freeshipping' }), isVirtual: () => false } - }); + }; + // The same capture instance the renderer reads, so a spec can see WHICH + // panel's identity a seed landed on — the resolved observables alone + // read the same either way whenever the other panel holds no number. + const capture = loadCompanyCapture(mocks); + const renderer = loadAmdModule(RENDERER, Object.assign({}, mocks, { + 'Two_Gateway/js/model/company-capture': capture + })); renderer.isOrderIntentEnabled = true; renderer.placeOrderIntent = function () { intents.push(renderer.companyId()); return { always: () => ({ done: () => ({ fail: () => {} }) }) }; }; - return { renderer, sections, dom, billingAddress, shippingAddress, intents }; + return { renderer, sections, dom, billingAddress, shippingAddress, intents, capture }; } test('the companyData subscription clears the previous company id', () => { @@ -475,9 +488,8 @@ describe('a company picked on the shipping step reaches the payment step', () => // The quote's own key, matching its shipping address: billing is not a // distinct address, so the shipping identity is the only capture the // resolver reads — which is what the resolved observables show - // (TWO-25554). Where the billing address IS distinct the seed stops at - // the billing identity: company-capture-billing-panel.test.js. - const { renderer, billingAddress } = loadWithSections({}); + // (TWO-25554). + const { renderer, billingAddress, capture } = loadWithSections({}); renderer.fillCustomerData(); @@ -489,10 +501,33 @@ describe('a company picked on the shipping step reaches the payment step', () => customAttributes: [{ attribute_code: 'company_id', value: '87654321' }] }); + expect(capture.shipping.identity().companyId()).toBe('87654321'); + expect(capture.billing.identity().companyId()).toBe(''); expect(renderer.companyName()).toBe('Billing Example Ltd'); expect(renderer.companyId()).toBe('87654321'); expect(renderer.telephone()).toBe('+47123 45 678'); }); + + test('a DISTINCT billing address seeds the BILLING panel, and resolves from it', () => { + // Its own key, so the quote holds two addresses. The seed lands on the + // billing identity and the resolver reads that same identity, so the + // company reaches the tile instead of being stranded (TWO-25554). + const { renderer, billingAddress, capture } = loadWithSections({}); + + renderer.fillCustomerData(); + + billingAddress({ + getCacheKey: () => 'billing-of-its-own', + countryId: 'GB', + company: 'Distinct Billing Ltd', + customAttributes: [{ attribute_code: 'company_id', value: '11223344' }] + }); + + expect(capture.billing.identity().companyId()).toBe('11223344'); + expect(capture.shipping.identity().companyId()).toBe(''); + expect(renderer.companyName()).toBe('Distinct Billing Ltd'); + expect(renderer.companyId()).toBe('11223344'); + }); }); describe('the shipping step agrees with the payment step', () => { diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index f5636255..4db72ed1 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -101,9 +101,34 @@ define([ return typeof $field.is === 'function' ? $field.is(':visible') : true; } - /** Is billing currently a distinct address from shipping? @returns {boolean} */ + /** + * Is billing currently a distinct address from shipping? The single + * authority — the resolver and billingRoleIdentity() both read this one. + * + * The buyer's checkbox and the quote, never whether the billing fieldset is + * on screen: a third-party re-render detaches that fieldset for an instant + * while neither the buyer's intent nor the quote has changed (TWO-25554). + * + * @returns {boolean} + */ function billingIsDistinct() { - return isVisible($(BILLING_FIELD_SELECTOR)); + const $toggle = $(BILLING_TOGGLE_SELECTOR); + if ($toggle.length && $toggle.prop('checked')) return false; + return quoteHoldsDistinctBillingAddress(); + } + + /** + * No shipping address at all — a virtual cart — leaves billing as the only + * address the quote holds, which no shipping address can be the same as. + * + * @returns {boolean} + */ + function quoteHoldsDistinctBillingAddress() { + const billingAddress = quote.billingAddress(); + if (!billingAddress) return false; + const shippingAddress = quote.shippingAddress(); + if (!shippingAddress) return true; + return shippingAddress.getCacheKey() != billingAddress.getCacheKey(); } /** @@ -404,7 +429,6 @@ define([ tileFieldSelector: '', fieldExists: function (selector) { if (!selector) return false; - // See isVisible() and billingIsDistinct() above. return isVisible($(selector)); }, getAdjacentCountry: function () { @@ -454,6 +478,10 @@ define([ // once for that — this only covers a LATER DOM appearance of the // billing form/checkbox that the initial recompute() ran before. $.async(BILLING_FIELD_SELECTOR, onChange); + // Core can select a billing address without the checkbox moving, + // and the quote is the predicate's other input. + quote.billingAddress.subscribe(onChange); + quote.shippingAddress.subscribe(onChange); } }); @@ -475,15 +503,10 @@ define([ * the resolver reads (company-source-resolver.js); seeding the billing * panel there discards a saved company outright. * - * @param {boolean} billingIsDistinctAddress the QUOTE's own answer. - * Never a read of the billing fieldset: a checkout that has that - * fieldset transiently away still has two addresses, and routing - * on what is on screen puts billing's company into the shipping - * panel's own field (TWO-25554). * @returns {object} */ - billingRoleIdentity: function (billingIsDistinctAddress) { - return billingIsDistinctAddress ? billingIdentity : shippingIdentity; + billingRoleIdentity: function () { + return billingIsDistinct() ? billingIdentity : shippingIdentity; }, start: function () { shippingComponent.start(); diff --git a/view/frontend/web/js/model/company-source-resolver.js b/view/frontend/web/js/model/company-source-resolver.js index 719ad65d..38c14ac7 100644 --- a/view/frontend/web/js/model/company-source-resolver.js +++ b/view/frontend/web/js/model/company-source-resolver.js @@ -38,8 +38,8 @@ * @param {object} options.resolved the identity downstream consumers read * @param {function(): boolean} options.billingIsDistinct whether billing * is currently a distinct address from shipping (core's "my - * billing address is the same as shipping" unchecked and a - * billing form rendered) + * billing address is the same as shipping" unchecked and the quote + * holding a billing address that is not its shipping one) * @param {function(function())} [options.watchBillingToggle] report every * time billingIsDistinct()'s answer could have changed */ diff --git a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js index f5973122..732b922e 100644 --- a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js +++ b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js @@ -863,23 +863,6 @@ define([ // which is what the component picks its mount by. this.refreshCompanyMount(); }, - /** - * Is the quote's billing address a distinct address from its shipping - * one? The same cache-key comparison updateShippingAddress() gates its - * own relay on. - * - * No shipping address at all — a virtual cart — leaves billing as the - * only address the quote holds, which no shipping address can be the - * same as. - * - * @param {object} billingAddress quote address - * @returns {boolean} - */ - billingAddressIsDistinct: function (billingAddress) { - const shippingAddress = quote.shippingAddress(); - if (!shippingAddress) return true; - return shippingAddress.getCacheKey() != billingAddress.getCacheKey(); - }, /** * The quote's BILLING address. Its company seeds the identity of the * panel that owns the billing ROLE — the billing panel while billing is @@ -896,7 +879,7 @@ define([ const fields = this.readAddressFields(billingAddress); this.applyBuyerFields(fields); if (fields.companyName && fields.companyId) { - companyCapture.billingRoleIdentity(this.billingAddressIsDistinct(billingAddress)).write({ + companyCapture.billingRoleIdentity().write({ companyName: fields.companyName, companyId: fields.companyId }); From b6b6c9825aa6e659e15af6c2aba7b88473c7ce9b Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 16:44:57 +0100 Subject: [PATCH 19/30] test(TWO-25554): let the direction tables' row descriptions reach the failure diff Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-chrome.test.js | 48 +++++++++++++++------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index a9cb521d..f77d34c5 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -210,17 +210,18 @@ beforeEach(() => { }); describe('the company number is painted under its own panel\'s field', () => { - test.each(DIRECTIONS)('%s picks a company (%s)', (actor) => { + test.each(DIRECTIONS)('%s picks a company (%s)', (actor, description) => { const other = OTHER[actor]; const { panels } = boot(); picks(panels[actor], COMPANIES[actor]); - expect(numbersIn(actor)).toEqual([COMPANIES[actor].companyId]); - expect(numbersIn(other)).toEqual([]); + expect(tagged(description, numbersIn(actor))) + .toEqual(tagged(description, [COMPANIES[actor].companyId])); + expect(tagged(description, numbersIn(other))).toEqual(tagged(description, [])); }); - test.each(DIRECTIONS)('%s is in manual entry (%s)', (actor) => { + test.each(DIRECTIONS)('%s is in manual entry (%s)', (actor, description) => { const { panels } = boot(); picks(panels[actor], COMPANIES[actor]); @@ -228,28 +229,29 @@ describe('the company number is painted under its own panel\'s field', () => { // Name-only capture: a number here would claim a registry identity the // buyer never picked. - expect(numbersIn(actor)).toEqual([]); + expect(tagged(description, numbersIn(actor))).toEqual(tagged(description, [])); }); - test.each(DIRECTIONS)('%s captures an internally-prefixed number (%s)', (actor) => { + test.each(DIRECTIONS)('%s captures an internally-prefixed number (%s)', (actor, description) => { const { panels } = boot(); picks(panels[actor], { text: 'No Public Number Ltd', companyId: 'TWO:abc', lookupId: 'l3' }); - expect(numbersIn(actor)).toEqual([]); + expect(tagged(description, numbersIn(actor))).toEqual(tagged(description, [])); }); - test.each(DIRECTIONS)('%s clears its capture (%s)', (actor) => { + test.each(DIRECTIONS)('%s clears its capture (%s)', (actor, description) => { const { panels, identities } = boot(); picks(panels[actor], COMPANIES[actor]); - expect(numbersIn(actor)).toEqual([COMPANIES[actor].companyId]); + expect(tagged(description, numbersIn(actor))) + .toEqual(tagged(description, [COMPANIES[actor].companyId])); identities[actor].clear(); - expect(numbersIn(actor)).toEqual([]); + expect(tagged(description, numbersIn(actor))).toEqual(tagged(description, [])); }); - test.each(DIRECTIONS)('%s renders a number restored by a reload (%s)', (actor) => { + test.each(DIRECTIONS)('%s renders a number restored by a reload (%s)', (actor, description) => { const other = OTHER[actor]; const restored = { shipping: 'shippingNumber', billing: 'billingNumber' }; @@ -257,9 +259,9 @@ describe('the company number is painted under its own panel\'s field', () => { // the checkoutProvider restores it. const { identities } = boot({ [restored[actor]]: '999999999' }); - expect(identities[actor].companyId()).toBe(''); - expect(numbersIn(actor)).toEqual(['999999999']); - expect(numbersIn(other)).toEqual([]); + expect(tagged(description, identities[actor].companyId())).toEqual(tagged(description, '')); + expect(tagged(description, numbersIn(actor))).toEqual(tagged(description, ['999999999'])); + expect(tagged(description, numbersIn(other))).toEqual(tagged(description, [])); }); test('a tile-mounted panel claims no number restored into the billing form', () => { @@ -375,34 +377,34 @@ describe('chrome never enters the popover\'s positioning context', () => { }); describe('the "select a different sole trader" link belongs to its own panel', () => { - test.each(DIRECTIONS)('%s adopts a sole trader (%s)', (actor) => { + test.each(DIRECTIONS)('%s adopts a sole trader (%s)', (actor, description) => { const other = OTHER[actor]; const { panels } = boot(); panels[actor].adoptSoleTrader(BUYERS[actor]); - expect(linksIn(actor)).toBe(1); - expect(linksIn(other)).toBe(0); + expect(tagged(description, linksIn(actor))).toEqual(tagged(description, 1)); + expect(tagged(description, linksIn(other))).toEqual(tagged(description, 0)); }); - test.each(DIRECTIONS)('%s picks a registered company instead (%s)', (actor) => { + test.each(DIRECTIONS)('%s picks a registered company instead (%s)', (actor, description) => { const { panels } = boot(); picks(panels[actor], COMPANIES[actor]); - expect(linksIn(actor)).toBe(0); + expect(tagged(description, linksIn(actor))).toEqual(tagged(description, 0)); }); - test.each(DIRECTIONS)('%s has its adoption withdrawn (%s)', (actor) => { + test.each(DIRECTIONS)('%s has its adoption withdrawn (%s)', (actor, description) => { const { panels, identities } = boot(); panels[actor].adoptSoleTrader(BUYERS[actor]); identities[actor].soleTraderAdopted(false); - expect(linksIn(actor)).toBe(0); + expect(tagged(description, linksIn(actor))).toEqual(tagged(description, 0)); }); - test.each(DIRECTIONS)('%s link is clicked (%s)', (actor) => { + test.each(DIRECTIONS)('%s link is clicked (%s)', (actor, description) => { const other = OTHER[actor]; const { panels, soleTraderCalls } = boot(); panels[actor].adoptSoleTrader(BUYERS[actor]); @@ -410,7 +412,7 @@ describe('the "select a different sole trader" link belongs to its own panel', ( document.querySelector(`${FORMS[actor]} .${LINK_CLASS}__link`).click(); - expect(soleTraderCalls).toEqual([panels[actor]]); + expect(tagged(description, soleTraderCalls)).toEqual(tagged(description, [panels[actor]])); }); }); From 9989a2faa407ae6906889399c569c8ed122f91df Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 17:15:59 +0100 Subject: [PATCH 20/30] fix(TWO-25554): strip the abandoned host's chrome and ARIA when the mount moves Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-chrome.test.js | 83 +++++++++++++++++++ .../web/js/model/company-capture-component.js | 8 +- .../web/js/model/company-search-panel.js | 27 ++++-- 3 files changed, 109 insertions(+), 9 deletions(-) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index f77d34c5..dab42c31 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -458,3 +458,86 @@ describe('a panel that loses its mount leaves no chrome behind', () => { expect(linksIn('shipping')).toBe(1); }); }); + +describe('a panel that MOVES its mount leaves no chrome behind at the old host', () => { + /** Numbered, so both pieces of chrome are on the page at once. */ + const TRADER = { company_name: 'Tile Trader', organization_number: '333333333' }; + + const TILE_FORM = '#two_gateway_form'; + + /** + * The shipping form arriving after the panel already fell back to the tile + * — a buyer switching off a saved address, which is the move this covers. + */ + function addShippingForm() { + const form = document.createElement('form'); + form.id = 'shipping-new-address-form'; + form.innerHTML = addressFields('GB'); + document.body.insertBefore(form, document.body.firstChild); + } + + /** @returns {Array} number labels rendered inside a container */ + function numbersUnder(root) { + return Array.prototype.map.call( + document.querySelectorAll(`${root} .${NUMBER_CLASS}`), + (node) => node.textContent + ); + } + + /** + * Tile-mounted, sole trader adopted, then the shipping form arrives and the + * mount moves off the tile. + * + * @returns {Promise} `{ panels, soleTraderCalls, tileInput }` + */ + async function movesOffTheTile() { + const booted = boot({ shippingForm: false, billingHidden: true }); + expect(booted.panels.shipping.mountSelector()).toBe(TILE_FIELD); + booted.panels.shipping.adoptSoleTrader(TRADER); + const tileInput = document.querySelector(TILE_FIELD); + expect(numbersUnder(TILE_FORM)).toEqual([TRADER.organization_number]); + expect(document.querySelectorAll(`${TILE_FORM} .${LINK_CLASS}`)).toHaveLength(1); + expect(tileInput.getAttribute('role')).toBe('combobox'); + + addShippingForm(); + booted.panels.shipping.refreshMount(); + await flushCapture(); + await flushCapture(); + + expect(booted.panels.shipping.mountSelector()).toBe(FIELDS.shipping); + return Object.assign({ tileInput: tileInput }, booted); + } + + test('the tile keeps neither the number nor a link to a flow it no longer hosts', async () => { + const { soleTraderCalls } = await movesOffTheTile(); + + expect(numbersUnder(TILE_FORM)).toEqual([]); + expect(document.querySelectorAll(`${TILE_FORM} .${LINK_CLASS}`)).toHaveLength(0); + expect(soleTraderCalls).toEqual([]); + }); + + test.each([ + ['role', 'the abandoned field still announces itself as a combobox'], + ['aria-haspopup', 'it still claims a listbox'], + ['aria-controls', 'it still points at the moved popover'], + ['aria-expanded', 'it still reports the moved popover\'s open state'] + ])('the tile field keeps no %s (%s)', async (attribute, description) => { + const { tileInput } = await movesOffTheTile(); + + expect(tagged(description, tileInput.getAttribute(attribute))) + .toEqual(tagged(description, null)); + }); + + test('the chrome the move carries lands at the new host, once', async () => { + const { panels } = boot({ shippingForm: false, billingHidden: true }); + panels.shipping.adoptSoleTrader(TRADER); + + addShippingForm(); + panels.shipping.refreshMount(); + await flushCapture(); + await flushCapture(); + + expect(numbersIn('shipping')).toEqual([TRADER.organization_number]); + expect(linksIn('shipping')).toBe(1); + }); +}); diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index 1b921202..61e67238 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -447,10 +447,12 @@ CompanyCaptureComponent.prototype.refreshMount = function () { const selector = this.mountSelector(); const previous = this._boundSelector; + // While the OLD selector is still bound — every chrome lookup goes + // through the bound field, so the host being left is unreachable once + // `_boundSelector` moves, and its number and sole-trader link would + // stand alongside the new host's (TWO-25554). + if (previous !== selector) this._removeChrome(); if (!selector) { - // Before the selector is forgotten — every chrome lookup goes - // through the bound field. - this._removeChrome(); // Neither host is on the page any more. Forgetting where the control // was is what stops `adjacentCountry()` answering for a form that has // gone, and lets the next host that appears mount cleanly. diff --git a/view/frontend/web/js/model/company-search-panel.js b/view/frontend/web/js/model/company-search-panel.js index 804a1625..7f59e2b4 100644 --- a/view/frontend/web/js/model/company-search-panel.js +++ b/view/frontend/web/js/model/company-search-panel.js @@ -69,6 +69,24 @@ // silently fails to hide offers the buyer a mode the country cannot serve. const HIDDEN_CLASS = 'two-hidden'; + /** + * What `_attach()` puts on the host field to announce the combobox. Left on + * a field this panel has moved off, it offers a keyboard buyer a listbox + * that no longer exists and points `aria-controls` at a removed popover + * (TWO-25554). + */ + const COMBOBOX_ATTRIBUTES = ['role', 'aria-haspopup', 'aria-controls', 'aria-expanded']; + + /** + * @param {?Element} field + */ + function stripComboboxAttributes(field) { + if (!field) return; + COMBOBOX_ATTRIBUTES.forEach(function (attr) { + field.removeAttribute(attr); + }); + } + /** * Every member the injected `search` API must carry. Checked at * construction because a host that supplies a partial one fails silently: @@ -315,6 +333,7 @@ // wrapper is a second anchor: the sole-trader fallback note then // renders against a host the buyer has left. this._releaseWrap(previous); + stripComboboxAttributes(previous); // Fresh identity, so a search issued by the node this call replaces // resolves into a token nothing is listening for. this._token = {}; @@ -954,9 +973,7 @@ this.close(); if (this._field) { this._unbind(this._field); - ['role', 'aria-haspopup', 'aria-controls', 'aria-expanded'].forEach(function (attr) { - this._field.removeAttribute(attr); - }, this); + stripComboboxAttributes(this._field); } this.renderBackToSearchLink(); }; @@ -1080,9 +1097,7 @@ this.search.abortActiveRequest(this._token); this.removeBackToSearchLink(); this._releaseWrap(this._field); - ['role', 'aria-haspopup', 'aria-controls', 'aria-expanded'].forEach(function (attr) { - if (this._field) this._field.removeAttribute(attr); - }, this); + stripComboboxAttributes(this._field); this._field = null; this._panel = null; this._query = null; From ae954a56553ab00fe65879724bd43c5a5dcb4615 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 17:21:15 +0100 Subject: [PATCH 21/30] fix(TWO-25554): render each panel's address notice at its own field The mirror carries the company fields alone, so no panel's UI state travels into the identity the tile and order-intent read. Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-chrome.test.js | 83 ++++++++++++++++ Test/Js/company-source-resolver.test.js | 47 ++++++--- ...eway-method-intent-approved-notice.test.js | 97 +------------------ Test/Js/proxy-rate-limit-backoff.test.js | 9 +- Test/Js/tile-company-readonly-fields.test.js | 5 +- view/frontend/web/css/style.css | 10 +- .../web/js/model/company-capture-component.js | 42 +++++++- .../frontend/web/js/model/company-identity.js | 15 ++- .../web/js/model/company-source-resolver.js | 9 +- .../payment/method-renderer/gateway_method.js | 13 --- .../web/template/payment/gateway_method.html | 12 --- 11 files changed, 192 insertions(+), 150 deletions(-) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index dab42c31..64c79e18 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -41,6 +41,7 @@ const TILE_FIELD = '#two_gateway_form input#company_name'; const NUMBER_CLASS = 'two-company-id-text'; const LINK_CLASS = 'two-select-different-sole-trader'; +const NOTICE_CLASS = 'two-company-address-notice'; /** The other panel, for a table row naming one. */ const OTHER = { shipping: 'billing', billing: 'shipping' }; @@ -202,6 +203,14 @@ function linksIn(which) { return document.querySelectorAll(`${FORMS[which]} .${LINK_CLASS}`).length; } +/** @returns {Array} the address notices rendered in one panel's form */ +function noticesIn(which) { + return Array.prototype.map.call( + document.querySelectorAll(`${FORMS[which]} .${NOTICE_CLASS}`), + (node) => node.textContent + ); +} + beforeEach(() => { document.body.innerHTML = ''; $(document).off('.twoCompanyCapture'); @@ -416,6 +425,80 @@ describe('the "select a different sole trader" link belongs to its own panel', ( }); }); +/* + * TWO-25554: an address-lookup failure is the panel's own, rendered at the + * panel's own field. It used to travel through the resolved identity into the + * payment tile, where a shipping failure was displayed only while shipping + * happened to win and a billing failure told the buyer to "enter it below" a + * form that was not theirs. + */ +describe('an address notice is painted under its own panel\'s field', () => { + const NOTICE = 'We could not fetch this company\'s address. Please enter it below.'; + const OTHER_NOTICE = 'We could not fill in this company\'s address on this page.'; + + test.each(DIRECTIONS)('%s raises a notice (%s)', (actor, description) => { + const other = OTHER[actor]; + const { identities } = boot(); + + identities[actor].addressNotice(NOTICE); + + expect(tagged(description, noticesIn(actor))).toEqual(tagged(description, [NOTICE])); + expect(tagged(description, noticesIn(other))).toEqual(tagged(description, [])); + }); + + test.each(DIRECTIONS)('%s withdraws its notice (%s)', (actor, description) => { + const { identities } = boot(); + identities[actor].addressNotice(NOTICE); + + identities[actor].addressNotice(''); + + expect(tagged(description, noticesIn(actor))).toEqual(tagged(description, [])); + }); + + test('both panels can hold their own notice at once, each at its own field', () => { + const { identities } = boot(); + + identities.shipping.addressNotice(NOTICE); + identities.billing.addressNotice(OTHER_NOTICE); + + expect(noticesIn('shipping')).toEqual([NOTICE]); + expect(noticesIn('billing')).toEqual([OTHER_NOTICE]); + }); + + test.each(DIRECTIONS)('%s repaints rather than stacking notices (%s)', (actor, description) => { + const { identities } = boot(); + identities[actor].addressNotice(NOTICE); + + identities[actor].addressNotice(OTHER_NOTICE); + + expect(tagged(description, noticesIn(actor))).toEqual(tagged(description, [OTHER_NOTICE])); + }); + + test('a tile-mounted panel paints its notice at the tile, and the other panel\'s form stays clean', () => { + const { panels, identities } = boot({ shippingForm: false, billingHidden: true }); + expect(panels.shipping.mountSelector()).toBe(TILE_FIELD); + + identities.shipping.addressNotice(NOTICE); + + expect( + Array.prototype.map.call( + document.querySelectorAll(`#two_gateway_form .${NOTICE_CLASS}`), + (node) => node.textContent + ) + ).toEqual([NOTICE]); + expect(noticesIn('billing')).toEqual([]); + }); + + test.each(DIRECTIONS)('%s announces its notice to a screen reader (%s)', (actor, description) => { + const { identities } = boot(); + + identities[actor].addressNotice(NOTICE); + + const box = document.querySelector(`${FORMS[actor]} .${NOTICE_CLASS}`); + expect(tagged(description, box.getAttribute('role'))).toEqual(tagged(description, 'alert')); + }); +}); + describe('a panel that loses its mount leaves no chrome behind', () => { const WRAP_CLASS = 'two-company-field-wrap'; /** Numbered, so both pieces of chrome are on the page at once. */ diff --git a/Test/Js/company-source-resolver.test.js b/Test/Js/company-source-resolver.test.js index ab5dd365..e2e74386 100644 --- a/Test/Js/company-source-resolver.test.js +++ b/Test/Js/company-source-resolver.test.js @@ -13,7 +13,7 @@ 'use strict'; -const { loadAmdModule } = require('./amd-harness'); +const { loadAmdModule, tagged } = require('./amd-harness'); const IDENTITY = 'view/frontend/web/js/model/company-identity.js'; const RESOLVER = 'view/frontend/web/js/model/company-source-resolver.js'; @@ -75,7 +75,6 @@ describe('billing not distinct — shipping always wins, wholesale', () => { shipping.companyName('Acme'); resolver.connect(); - expect(resolved.captureMode()).toBe('manual'); expect(resolved.companyName()).toBe('Acme'); expect(resolved.companyId()).toBe(''); }); @@ -196,19 +195,43 @@ describe('re-resolves live, on every input that could change the answer', () => }); }); -describe('the mirror copies every field, not just the name/number pair', () => { - test('soleTraderAvailable, soleTraderBusy and addressNotice all mirror the winning identity', () => { +describe('the mirror carries the company fields and nothing else', () => { + test('the winning identity\'s name, number and source all reach the mirror', () => { const { shipping, resolver, resolved } = build(); - shipping.soleTraderAvailable(true); - shipping.soleTraderBusy(true); - shipping.addressNotice('We could not fetch this company\'s address. Please enter it below.'); + shipping.write( + { companyName: 'Shipping Co', companyId: '111', companyIdSource: 'registry' }, + { authoritative: true } + ); resolver.connect(); - expect(resolved.soleTraderAvailable()).toBe(true); - expect(resolved.soleTraderBusy()).toBe(true); - expect(resolved.addressNotice()).toBe( - 'We could not fetch this company\'s address. Please enter it below.' - ); + expect(resolved.companyName()).toBe('Shipping Co'); + expect(resolved.companyId()).toBe('111'); + expect(resolved.companyIdSource()).toBe('registry'); + }); + + /* + * Each of these is one panel's own UI state, rendered by that panel at its + * own field. Travelling through the mirror, an address failure raised by + * one panel rendered against the other panel's form — or, once the tile + * stopped rendering it at all, nowhere (TWO-25554). + */ + test.each([ + ['captureMode', 'soletrader', 'registered'], + ['addressNotice', 'We could not fetch this address.', ''], + ['soleTraderAdopted', true, false], + ['soleTraderAvailable', true, false], + ['soleTraderBusy', true, false] + ])('%s never reaches the mirror', (field, panelValue, untouched) => { + const { shipping, resolver, resolved } = build(); + resolver.connect(); + // A distinct value: writing back what the identity already holds + // notifies nothing, and a stale mirror would satisfy the assertion. + expect(resolved[field]()).toBe(untouched); + + shipping[field](panelValue); + + expect(tagged(field, shipping[field]())).toEqual(tagged(field, panelValue)); + expect(tagged(field, resolved[field]())).toEqual(tagged(field, untouched)); }); test('a torn read is impossible: a subscriber sees BOTH halves of the pair already updated', () => { diff --git a/Test/Js/gateway-method-intent-approved-notice.test.js b/Test/Js/gateway-method-intent-approved-notice.test.js index ab34fa1e..10998c8c 100644 --- a/Test/Js/gateway-method-intent-approved-notice.test.js +++ b/Test/Js/gateway-method-intent-approved-notice.test.js @@ -62,8 +62,6 @@ const DEFAULT_COPY = { companyNumberToken: '{{companyNumber}}' }; -const ADDRESS_FAILURE_COPY = 'We could not fetch this company\'s address. Please enter it below.'; - const DECLINED_COPY = { withCompany: 'Two is not available for this order by {{companyName}} ({{companyNumber}})', withoutCompany: 'Two is not available for this order', @@ -407,96 +405,6 @@ describe('gateway_method intent-approved notice', () => { }); }); -/** - * A failed address lookup reaches the buyer in its own bordered box, carried - * on the identity bus. It used to go to the checkout-wide message list, which - * nothing in this flow ever cleared. - */ -describe('the address-lookup failure notice lands in the tile box', () => { - function contextOn(addressNotice) { - const identityStub = { - addressNotice: addressNotice, - companyName: koObservable(''), - companyId: koObservable(''), - soleTraderAdopted: koObservable(false), - soleTraderBusy: koObservable(false), - subscribe: function () { - return { dispose: function () {} }; - } - }; - const component = loadAmdModule(RENDERER, { - 'Two_Gateway/js/model/company-capture': { - identity: identityStub, - shipping: { - identity: function () { return identityStub; }, - subscribeMount: function () {} - }, - refreshMount: function () {} - } - }); - const ctx = Object.assign({}, component, { - companyName: koObservable(''), - companyId: koObservable(''), - generalErrorMessage: 'Something went wrong with your order.', - errors: [] - }); - ctx.showErrorMessage = function (message) { ctx.errors.push(message); }; - component.initOrderIntentApprovedNotice.call(ctx, {}); - return ctx; - } - - test.each([ - [ADDRESS_FAILURE_COPY, true], - ['', false] - ])('a bus value of %p reaches the box: %p', (text, expected) => { - const addressNotice = koObservable(''); - const ctx = contextOn(addressNotice); - - addressNotice(text); - - expect(ctx.isAddressNoticeVisible()).toBe(expected); - if (expected) expect(ctx.addressNotice()).toBe(text); - // Never the checkout-wide region. - expect(ctx.errors).toEqual([]); - }); - - test('withdrawing it empties the box', () => { - const addressNotice = koObservable(''); - const ctx = contextOn(addressNotice); - addressNotice(ADDRESS_FAILURE_COPY); - - addressNotice(''); - - expect(ctx.isAddressNoticeVisible()).toBe(false); - }); - - // The real ordering: the address lookup answers FIRST, the slower credit - // check second. The intent verdict must not take the address notice with it. - test.each([ - [{ approved: true }, 'orderIntentApprovedNotice'], - [{ approved: false }, 'orderIntentDeclinedNotice'] - ])('an intent verdict of %p leaves the address notice standing', (response) => { - const addressNotice = koObservable(''); - const ctx = contextOn(addressNotice); - addressNotice(ADDRESS_FAILURE_COPY); - - ctx.processOrderIntentSuccessResponse.call(ctx, response); - - expect(ctx.addressNotice()).toBe(ADDRESS_FAILURE_COPY); - expect(ctx.isAddressNoticeVisible()).toBe(true); - }); - - test('an errored intent leaves the address notice standing', () => { - const addressNotice = koObservable(''); - const ctx = contextOn(addressNotice); - addressNotice(ADDRESS_FAILURE_COPY); - - ctx.processOrderIntentErrorResponse.call(ctx, { status: 500 }); - - expect(ctx.addressNotice()).toBe(ADDRESS_FAILURE_COPY); - }); -}); - /** * The box itself. TWO-25326 (2026-08-05): one bordered container, the same * three semantic colours, and the message ALONE inside it on all four @@ -550,12 +458,11 @@ describe('order-intent message box markup and palette', () => { const blocks = markup.match( /]*class="two-order-intent-message [a-z]+"[\s\S]*?<\/div>/g ); - // Three intent outcomes plus the address-lookup failure. - expect(blocks).toHaveLength(4); + expect(blocks).toHaveLength(3); blocks.forEach((block) => { // A single `text:`-bound element: no nested element to put a // heading in, and no literal copy in the markup either. - expect(block).toMatch(/data-bind="text: (orderIntent\w+|address)Notice"/); + expect(block).toMatch(/data-bind="text: orderIntent\w+Notice"/); expect(block).not.toMatch(/<(h\d|strong|p|span)\b/); }); // Nor a heading immediately before the boxes. diff --git a/Test/Js/proxy-rate-limit-backoff.test.js b/Test/Js/proxy-rate-limit-backoff.test.js index c8379e40..6e210693 100644 --- a/Test/Js/proxy-rate-limit-backoff.test.js +++ b/Test/Js/proxy-rate-limit-backoff.test.js @@ -257,8 +257,8 @@ describe('company search backs off rather than retrying into the ceiling', () => /** * The parked-lookup notice fires synchronously on the pick, and the slower - * credit check answers after it. Both used to write the same observable, so - * the verdict blanked the notice in exactly the case it exists for. + * credit check answers after it — so nothing the order-intent verdict clears + * may reach the panel's own notice, which the buyer still has to act on. */ describe('a parked address lookup keeps its notice through the intent verdict', () => { function wire() { @@ -311,10 +311,11 @@ describe('a parked address lookup keeps its notice through the intent verdict', ); expect(identity.addressNotice()).toContain('enter it below'); + const standing = identity.addressNotice(); + settleVerdict(tile); - expect(identity.addressNotice()).toContain('enter it below'); - expect(tagged(description, tile.isAddressNoticeVisible())).toEqual(tagged(description, true)); + expect(tagged(description, identity.addressNotice())).toEqual(tagged(description, standing)); }); }); diff --git a/Test/Js/tile-company-readonly-fields.test.js b/Test/Js/tile-company-readonly-fields.test.js index 70fd781b..e35d86e5 100644 --- a/Test/Js/tile-company-readonly-fields.test.js +++ b/Test/Js/tile-company-readonly-fields.test.js @@ -392,7 +392,8 @@ const DECLINED_NOTICE_COPY = { /** * The tile: the renderer, the capture component that owns its mount, and the - * identity singleton both read. + * SHIPPING panel's own identity — the one mounted at the tile, and the one + * whose mode and adoption the tile's bindings read (TWO-25554). * * The REAL component, not a stub — every mode transition below is production's * own, so a mode that stopped clearing what it clears fails here. @@ -461,7 +462,7 @@ function loadTile() { errorMessages: { remove: function () {} } }; - return { renderer, component, identity: component.identity, dom, soleTrader }; + return { renderer, component, identity: component.shipping.identity(), dom, soleTrader }; } /** diff --git a/view/frontend/web/css/style.css b/view/frontend/web/css/style.css index 38c442d0..e969147d 100755 --- a/view/frontend/web/css/style.css +++ b/view/frontend/web/css/style.css @@ -390,10 +390,12 @@ } /* - * Persistent "order intent approved" notice, rendered inline inside the - * payment tile (see view/frontend/web/template/payment/gateway_method.html). - * Replaces the transient checkout message-region treatment, which checkout - * cleared on every update. + * The module's persistent inline notice box — the order-intent outcomes in the + * payment tile (view/frontend/web/template/payment/gateway_method.html) and a + * capture panel's own address-lookup failure beside its field + * (`.two-company-address-notice`, company-capture-component.js). Replaces the + * transient checkout message-region treatment, which checkout cleared on every + * update. * * Deliberately low-key: this is reassurance, not a call to action, and it * sits directly under the term chips. Brand overlays that ship their own diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index 61e67238..d77ecfcf 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -80,6 +80,15 @@ const SOLE_TRADER_LINK_CLASS = 'two-select-different-sole-trader'; + /** + * Why the picked company's address could not be filled in. Styled as the + * tile's own notice box (`two-order-intent-message error`) and hooked by + * this class alone, so the chrome lookup cannot reach the tile's boxes. + */ + const ADDRESS_NOTICE_CLASS = 'two-company-address-notice'; + + const ADDRESS_NOTICE_STYLE_CLASSES = ['two-order-intent-message', 'error']; + /** * A company number the checkout restored into an address form, under either * host's field naming. @@ -600,6 +609,7 @@ CompanyCaptureComponent.prototype.renderChrome = function () { this.renderCompanyNumber(); this.renderSoleTraderLink(); + this.renderAddressNotice(); }; /** @@ -608,7 +618,7 @@ */ CompanyCaptureComponent.prototype._removeChrome = function () { const self = this; - [COMPANY_NUMBER_CLASS, SOLE_TRADER_LINK_CLASS].forEach(function (className) { + [COMPANY_NUMBER_CLASS, SOLE_TRADER_LINK_CLASS, ADDRESS_NOTICE_CLASS].forEach(function (className) { const node = self._chromeNode(className); if (node) node.remove(); }); @@ -733,6 +743,36 @@ host.insertBefore(wrapper, after.nextSibling); }; + /** + * Why this panel's picked company could not have its address filled in, + * under this panel's OWN field. + * + * At the field rather than in the payment tile because the copy sends the + * buyer to the address fields "below" it, and a notice rendered anywhere + * but beside the panel that raised it points at the wrong form — or, for a + * panel the tile is not mounted on, at no form the buyer can see + * (TWO-25554). + */ + CompanyCaptureComponent.prototype.renderAddressNotice = function () { + const anchor = this._chromeAnchor(); + const host = this._chromeHost(); + if (!anchor || !host) return; + const existing = this._chromeNode(ADDRESS_NOTICE_CLASS); + if (existing) existing.remove(); + const notice = this._identity.addressNotice(); + if (!notice) return; + const box = document.createElement('div'); + box.className = [ADDRESS_NOTICE_CLASS].concat(ADDRESS_NOTICE_STYLE_CLASSES).join(' '); + // The lookup answers after the buyer has moved on from the field, so + // nothing else would announce the failure to a screen reader. + box.setAttribute('role', 'alert'); + box.textContent = notice; + const after = this._chromeNode(SOLE_TRADER_LINK_CLASS) + || this._chromeNode(COMPANY_NUMBER_CLASS) + || anchor; + host.insertBefore(box, after.nextSibling); + }; + // ----------------------------------------------------------------- chips /** diff --git a/view/frontend/web/js/model/company-identity.js b/view/frontend/web/js/model/company-identity.js index 383fb15f..2e0b255f 100644 --- a/view/frontend/web/js/model/company-identity.js +++ b/view/frontend/web/js/model/company-identity.js @@ -239,14 +239,23 @@ }, /** - * Every field, as one plain object — for a caller mirroring this + * WHICH COMPANY, and nothing else — for a caller mirroring this * identity onto another one (`company-source-resolver.js`) that - * must copy the whole thing in a single notify, not one per field. + * must copy it in a single notify, not one field at a time. + * + * The capture mode and the address notice are the owning PANEL's + * own UI state, rendered at its own field by whichever panel holds + * them; travelling here they surfaced against the other panel's + * form or against no form at all (TWO-25554). * * @returns {object} */ snapshot: function () { - return Object.assign({}, state); + return { + companyName: state.companyName, + companyId: state.companyId, + companyIdSource: state.companyIdSource + }; }, /** diff --git a/view/frontend/web/js/model/company-source-resolver.js b/view/frontend/web/js/model/company-source-resolver.js index 38c14ac7..9a84b2eb 100644 --- a/view/frontend/web/js/model/company-source-resolver.js +++ b/view/frontend/web/js/model/company-source-resolver.js @@ -12,10 +12,11 @@ * from billing first, falling back to shipping only if billing doesn't * present a company number." * - * Never a hybrid of the two: `resolved` is always a live, full mirror of - * exactly ONE of `shipping`/`billing` — every field, not just the - * name/number pair — so a downstream reader (order-intent, the tile's own - * display) sees one coherent identity, the same shape it always has. + * Never a hybrid of the two: `resolved` is always a live mirror of exactly ONE + * of `shipping`/`billing`, so a downstream reader (order-intent, the tile's own + * display) sees one coherent company. What travels is `snapshot()`'s business — + * the company fields alone; each panel's own UI state stays with the panel that + * renders it. * * FRAMEWORK-FREE, for the same reason its two inputs are: both checkouts * load this file, and Hyvä ships no Knockout. diff --git a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js index 732b922e..fbfc43ba 100644 --- a/view/frontend/web/js/view/payment/method-renderer/gateway_method.js +++ b/view/frontend/web/js/view/payment/method-renderer/gateway_method.js @@ -467,15 +467,6 @@ define([ isOrderIntentErrorNoticeVisible: function () { return !!(this.orderIntentErrorNotice && this.orderIntentErrorNotice()); }, - /** - * Same guard, for the company-address lookup failure — not one of the - * order-intent notices, see initOrderIntentApprovedNotice(). - * - * @returns {boolean} - */ - isAddressNoticeVisible: function () { - return !!(this.addressNotice && this.addressNotice()); - }, /** * Blank all three order-intent outcome notices. * @@ -1165,10 +1156,6 @@ define([ // processOrderIntent*Response() re-sets afterwards; a company // edited by hand in the input clears both notices and leaves // them cleared, which is the correct fail-closed outcome. - // Its OWN box, not the intent-error one: the credit check normally - // answers after the address lookup, and clearOrderIntentNotices() - // would blank an address failure the buyer still has to act on. - this.addressNotice = identity.addressNotice; var self = this; /** diff --git a/view/frontend/web/template/payment/gateway_method.html b/view/frontend/web/template/payment/gateway_method.html index 0984588a..f71fb129 100644 --- a/view/frontend/web/template/payment/gateway_method.html +++ b/view/frontend/web/template/payment/gateway_method.html @@ -171,18 +171,6 @@ data-bind="text: orderIntentErrorNotice" > - - - - From dfeacccad740b061978e29708ad6ebd3f9c38632 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 17:24:33 +0100 Subject: [PATCH 22/30] fix(TWO-25554): give the billing panel its own write destination Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-independence.test.js | 69 +++++++++++++++++++ view/frontend/web/js/model/company-capture.js | 41 +++++++++-- 2 files changed, 105 insertions(+), 5 deletions(-) diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 8bf35d8f..bc6bba4c 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -474,6 +474,75 @@ describe('a tile-mounted shipping panel has no form of its own', () => { }); }); +describe('the billing panel\'s own writes have their own destination', () => { + const WRITE_BACK = ADDRESSES.billing; + const PHONE = '+44 1233 000000'; + const UNDELIVERABLE = 'We could not fill in this company\'s address on this page.'; + + /** @returns {string} the telephone in one form */ + function telephoneIn(which) { + return document.querySelector(`${FORMS[which]} [name="telephone"]`).value; + } + + test('a billing sole-trader address lands in the billing form and never the shipping one', () => { + const booted = boot(); + expect(booted.panels.billing.mountSelector()).toBe(FIELDS.billing); + const shippingBefore = addressValues('shipping'); + + booted.panels.billing.host().applyBuyerAddress(WRITE_BACK); + + expect(addressValues('billing')['city']).toBe(WRITE_BACK.city); + expect(addressValues('billing')['postcode']).toBe(WRITE_BACK.postal_code); + expect(addressValues('shipping')).toEqual(shippingBefore); + expect(booted.identities.billing.addressNotice()).toBe(''); + }); + + test('a billing sole-trader telephone lands in the billing form and never the shipping one', () => { + const booted = boot(); + + booted.panels.billing.host().applyTelephone(PHONE); + + expect(telephoneIn('billing')).toBe(PHONE); + expect(telephoneIn('shipping')).toBe(''); + }); + + /* + * Core leaves the fieldset in the DOM hidden once "same as shipping" is + * re-checked, so the billing panel has no destination — and a write-back + * that fills nothing in and says nothing reads as the picker having done + * nothing (TWO-25461 §5). + */ + test.each([ + [ + 'the address half', + function (booted) { booted.panels.billing.host().applyBuyerAddress(WRITE_BACK); }, + 'a billing address write-back with nowhere to land announces instead' + ], + [ + 'the telephone half', + function (booted) { booted.panels.billing.host().applyTelephone(PHONE); }, + 'a billing telephone write-back with nowhere to land announces instead' + ] + ])('with the fieldset gone, %s announces rather than filling nothing in', + (half, write, description) => { + const booted = boot({ billingHidden: true }); + expect(booted.panels.billing.mountSelector()).toBe(''); + + write(booted); + + expect(tagged(description, booted.identities.billing.addressNotice())) + .toEqual(tagged(description, UNDELIVERABLE)); + // Never the other panel's form, which is the one still on screen. + expect(tagged(description, addressValues('shipping')['city'])) + .toEqual(tagged(description, '')); + expect(tagged(description, telephoneIn('shipping'))).toEqual(tagged(description, '')); + // Billing's own identity carries it: it is billing's field the + // notice is rendered beside. + expect(tagged(description, booted.identities.shipping.addressNotice())) + .toEqual(tagged(description, '')); + }); +}); + describe('the quote\'s billing address belongs to the billing panel', () => { const SAVED = { countryId: 'GB', diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index 4db72ed1..ccb20e9f 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -360,7 +360,8 @@ define([ /** * shippingWriteRoot(), and a notice on the shipping identity when there is - * no such root. + * none — a pick that fills nothing in and says nothing reads to the buyer + * as the picker having done nothing (TWO-25461 §5). * * @returns {?object} jQuery set, or null */ @@ -370,6 +371,36 @@ define([ return root; } + /** + * Where the BILLING panel's own writes land, or `null` when there is no + * destination for them. + * + * Its own form and only ever its own form: it has no tile fallback and the + * shipping form is the other panel's (TWO-25554). Keyed on the live mount + * rather than on the selector matching, because core leaves the fieldset in + * the DOM hidden once "same as shipping" is re-checked, and a hidden field + * is not somewhere the buyer can read what was written. + * + * @returns {?object} jQuery set, or null + */ + function billingWriteRoot() { + if (billingComponent.mountSelector() !== BILLING_FIELD_SELECTOR) return null; + const $root = $(BILLING_FORM_ROOT); + return $root.length ? $root : null; + } + + /** + * billingWriteRoot(), and a notice on the billing identity when there is + * none — shippingWriteTarget()'s counterpart, for the same reason. + * + * @returns {?object} jQuery set, or null + */ + function billingWriteTarget() { + const root = billingWriteRoot(); + if (!root) companySearch.announceAddressUndeliverable(billingIdentity); + return root; + } + /** * One control per field: a mounted billing panel is already the buyer's * route to supply a company, and a second required one the resolver ignores @@ -438,18 +469,18 @@ define([ companySearch.lookupCompanyAddress( billingComponent.config(), selectedItem, - $(BILLING_FORM_ROOT), + billingWriteRoot(), billingIdentity ); }, revertAutofilledAddress: function () { - companySearch.revertAutofilledAddress($(BILLING_FORM_ROOT), billingIdentity); + companySearch.revertAutofilledAddress(billingWriteRoot(), billingIdentity); }, applyBuyerAddress: function (source) { - companySearch.applyAddress(source, $(BILLING_FORM_ROOT), billingIdentity); + companySearch.applyAddress(source, billingWriteTarget(), billingIdentity); }, applyTelephone: function (phoneNumber) { - companySearch.applyTelephone(phoneNumber, $(BILLING_FORM_ROOT)); + companySearch.applyTelephone(phoneNumber, billingWriteTarget()); }, getFallbackCountry: function () { return companySearch.currentAddressFormCountry($(BILLING_FORM_ROOT)); From bc4505775bc47557c81b69642637c1d7fb06c7c5 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 17:28:55 +0100 Subject: [PATCH 23/30] fix(TWO-25554): read the "same as shipping" box of the active method only Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-independence.test.js | 74 +++++++++++++++++++ view/frontend/web/js/model/company-capture.js | 37 +++++++++- 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index bc6bba4c..2c393103 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -543,6 +543,80 @@ describe('the billing panel\'s own writes have their own destination', () => { }); }); +/* + * TWO-25554: core renders one "same as shipping" checkbox per payment-method + * renderer, each with its own default. Read page-wide, whichever renderer the + * checkout output first answered for the buyer — so an inactive method's box, + * still at core's checked default, said billing was shipping while the buyer + * had unchecked the box they could actually see. + */ +describe('only the ACTIVE payment method\'s "same as shipping" checkbox is read', () => { + const ACTIVE_METHOD = 'two_payment'; + + /** @returns {string} which panel speaks for the quote's billing address */ + function billingRole(booted) { + return booted.capture.billingRoleIdentity() === booted.identities.billing + ? 'billing' + : 'shipping'; + } + + /** + * An inactive method's checkbox at core's checked default, output BEFORE the + * active method's — which is what a page-wide read lands on. + */ + function addInactiveMethodToggle() { + const container = document.querySelector('.checkout-billing-address'); + const box = document.createElement('input'); + box.type = 'checkbox'; + box.id = 'billing-address-same-as-shipping-checkmo'; + box.name = 'billing-address-same-as-shipping'; + box.checked = true; + container.insertBefore(box, container.firstChild); + } + + test('an inactive method\'s checked box does not answer for the active method', () => { + const booted = boot(); + booted.quote.paymentMethod({ method: ACTIVE_METHOD }); + // The buyer's own box, on the method they are looking at: unchecked. + expect(billingRole(booted)).toBe('billing'); + + addInactiveMethodToggle(); + + expect(billingRole(booted)).toBe('billing'); + }); + + test('the resolved company still follows the billing panel through the extra box', () => { + const booted = boot(); + booted.quote.paymentMethod({ method: ACTIVE_METHOD }); + addInactiveMethodToggle(); + + picks(booted.panels.shipping, COMPANIES.shipping); + picks(booted.panels.billing, COMPANIES.billing); + + expect(booted.capture.identity.companyId()).toBe(COMPANIES.billing.companyId); + }); + + test('the ACTIVE method\'s own box is still obeyed when the buyer checks it', () => { + const booted = boot(); + booted.quote.paymentMethod({ method: ACTIVE_METHOD }); + addInactiveMethodToggle(); + + document.querySelector(`#billing-address-same-as-shipping-${ACTIVE_METHOD}`).checked = true; + + expect(billingRole(booted)).toBe('shipping'); + }); + + test('with no method selected, no box is attributable and the quote answers alone', () => { + const booted = boot(); + addInactiveMethodToggle(); + expect(booted.quote.paymentMethod()).toBeNull(); + + // The quote holds a billing address and no shipping one, so billing is + // an address of its own whatever the unattributable boxes say. + expect(billingRole(booted)).toBe('billing'); + }); +}); + describe('the quote\'s billing address belongs to the billing panel', () => { const SAVED = { countryId: 'GB', diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index ccb20e9f..535bf289 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -65,9 +65,16 @@ define([ /** The country select inside that SAME billing form — never a shared one. */ const BILLING_COUNTRY_SELECTOR = `${BILLING_FORM_ROOT} select[name="country_id"]`; - /** "My billing and shipping address are the same" — core's own checkbox. */ + /** + * "My billing and shipping address are the same" — core's own checkbox, one + * per payment-method renderer. Bare, so a delegated listener hears every + * one of them; see activeBillingToggle() for READING one. + */ const BILLING_TOGGLE_SELECTOR = 'input[name="billing-address-same-as-shipping"]'; + /** Core's own per-renderer id for that checkbox, less the method code. */ + const BILLING_TOGGLE_ID_PREFIX = 'billing-address-same-as-shipping-'; + /** @see soleAddressForm — what makes a container an address form. */ const ADDRESS_STREET_SELECTOR = 'input[name="street[0]"]'; @@ -112,11 +119,35 @@ define([ * @returns {boolean} */ function billingIsDistinct() { - const $toggle = $(BILLING_TOGGLE_SELECTOR); - if ($toggle.length && $toggle.prop('checked')) return false; + const $toggle = activeBillingToggle(); + if ($toggle && $toggle.length && $toggle.prop('checked')) return false; return quoteHoldsDistinctBillingAddress(); } + /** + * The one "same as shipping" checkbox that speaks for the buyer. + * + * Core renders one per payment-method renderer, each with its own default, + * so a page-wide read is answered by whichever renderer the checkout output + * first — an inactive method's box as readily as the active one + * (TWO-25554). A single box on the page is unambiguous whatever its id; past + * that the active method's own is identified by core's id convention, and a + * checkout that renders several and abandons that convention has no + * attributable answer, leaving the quote as the honest source. + * + * @returns {?object} jQuery(-shaped) set, or `null` when no box can be + * attributed to the active method + */ + function activeBillingToggle() { + const $all = $(BILLING_TOGGLE_SELECTOR); + // `> 1` rather than `< 2`: a jQuery-shaped double reports no length at + // all, and presence is the best answer available for those. + if (!($all.length > 1)) return $all; + const selected = quote.paymentMethod(); + const code = selected && selected.method; + return code ? $(`#${BILLING_TOGGLE_ID_PREFIX}${code}`) : null; + } + /** * No shipping address at all — a virtual cart — leaves billing as the only * address the quote holds, which no shipping address can be the same as. From b6845a557526aab99684c731ea76a51b284955c1 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 17:32:46 +0100 Subject: [PATCH 24/30] test(TWO-25554): pin the three panel-separation fixes nothing was holding Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-chrome.test.js | 31 ++++++++ Test/Js/company-panel-independence.test.js | 85 ++++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index 64c79e18..185db14e 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -542,6 +542,37 @@ describe('a panel that loses its mount leaves no chrome behind', () => { }); }); +/* + * TWO-25554: `onCountryChanged()` reads the mode BEFORE `clear()` resets it, so + * the retirement can still tell that sole-trader mode is what it is leaving. + * Read after the clear, the answer is always "no" and the panel is never handed + * back as a search trigger — leaving the buyer on a released field with no route + * back into the search. + */ +describe('a country change out of sole-trader mode hands the field back to the search', () => { + const BACK_CLASS = 'two-company-search-back'; + + test.each(DIRECTIONS)('%s switches country while in sole-trader mode (%s)', async (actor, description) => { + const { panels } = boot(); + // Manual entry first, so the field is RELEASED — a plain input with the + // return link beside it — and only registeredMode() reclaims it. + panels[actor].manualEntryMode(); + panels[actor].soleTraderMode(); + const field = document.querySelector(FIELDS[actor]); + expect(field.getAttribute('role')).toBeNull(); + expect(document.querySelectorAll(`${FORMS[actor]} .${BACK_CLASS}`)).toHaveLength(1); + + panels[actor].onCountryChanged(actor === 'shipping' ? 'no' : 'gb'); + await flushCapture(); + await flushCapture(); + + expect(tagged(description, field.getAttribute('role'))) + .toEqual(tagged(description, 'combobox')); + expect(tagged(description, document.querySelectorAll(`${FORMS[actor]} .${BACK_CLASS}`).length)) + .toEqual(tagged(description, 0)); + }); +}); + describe('a panel that MOVES its mount leaves no chrome behind at the old host', () => { /** Numbered, so both pieces of chrome are on the page at once. */ const TRADER = { company_name: 'Tile Trader', organization_number: '333333333' }; diff --git a/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 2c393103..6ee0bc1a 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -30,6 +30,7 @@ const { installAsyncSimulation, tagged, quoteAddress, + quoteAddressValue, makeObservable } = require('./amd-harness'); @@ -617,6 +618,90 @@ describe('only the ACTIVE payment method\'s "same as shipping" checkbox is read' }); }); +/* + * TWO-25554: what a panel autofilled is recorded per IDENTITY. One page-wide + * record is replaced wholesale by whichever panel writes last, so the first + * panel's revert then judges its own fields against the other panel's values — + * and leaves the previous country's address standing. + */ +describe('each panel\'s record of what it autofilled is its own', () => { + const MARKER = 'data-two-autofilled-value'; + + /** + * A third-party re-render, which drops the per-field markers and leaves the + * record as the only thing a revert can judge against. + */ + function stripMarkers(which) { + Array.prototype.forEach.call( + document.querySelectorAll(`${FORMS[which]} [${MARKER}]`), + (node) => node.removeAttribute(MARKER) + ); + } + + test.each(DIRECTIONS)('%s reverts against its own record (%s)', (actor, description) => { + const other = OTHER[actor]; + const booted = boot(); + booted.search.applyAddress(ADDRESSES[actor], $(FORMS[actor]), booted.identities[actor]); + // The OTHER panel writes SECOND: one shared record is wiped here. + booted.search.applyAddress(ADDRESSES[other], $(FORMS[other]), booted.identities[other]); + expect(addressValues(actor)['city']).toBe(ADDRESSES[actor].city); + stripMarkers(actor); + + booted.search.revertAutofilledAddress($(FORMS[actor]), booted.identities[actor]); + + expect(tagged(description, addressValues(actor)['city'])).toEqual(tagged(description, '')); + // The other panel's record survived its neighbour's revert, so its own + // fields are still both filled and still retractable. + expect(tagged(description, addressValues(other)['city'])) + .toEqual(tagged(description, ADDRESSES[other].city)); + stripMarkers(other); + booted.search.revertAutofilledAddress($(FORMS[other]), booted.identities[other]); + expect(tagged(description, addressValues(other)['city'])).toEqual(tagged(description, '')); + }); +}); + +/* + * TWO-25554: core can select a billing address without the checkbox moving — + * a saved-address pick, or a virtual cart taking one on — and the quote is the + * predicate's other input, so the resolver subscribes to both quote addresses. + */ +describe('a quote address change re-resolves with no checkbox event', () => { + const DISTINCT_KEY = 'billing-of-its-own'; + + /** Both panels captured, billing not yet a distinct address. */ + function bothCaptured() { + const booted = boot({ quoteShippingAddress: quoteAddressValue({ countryId: 'GB' }) }); + picks(booted.panels.shipping, COMPANIES.shipping); + picks(booted.panels.billing, COMPANIES.billing); + expect(booted.capture.identity.companyId()).toBe(COMPANIES.shipping.companyId); + return booted; + } + + test.each([ + ['billingAddress', 'the quote taking on a billing address of its own'], + ['shippingAddress', 'the quote losing the shipping address billing matched'] + ])('%s notifying re-resolves the company (%s)', (which, description) => { + const booted = bothCaptured(); + + // A DISTINCT value: re-writing what the observable already holds + // notifies nothing, and a stale resolution would satisfy this. + if (which === 'billingAddress') { + booted.quote.billingAddress(quoteAddressValue({ countryId: 'GB' }, DISTINCT_KEY)); + } else { + booted.quote.shippingAddress(null); + } + + expect(tagged(description, booted.capture.identity.companyId())) + .toEqual(tagged(description, COMPANIES.billing.companyId)); + // Nothing touched the checkbox — it is still unchecked and still the + // only one on the page. + expect(tagged(description, $('input[name="billing-address-same-as-shipping"]').length)) + .toEqual(tagged(description, 1)); + expect(tagged(description, $('input[name="billing-address-same-as-shipping"]').prop('checked'))) + .toEqual(tagged(description, false)); + }); +}); + describe('the quote\'s billing address belongs to the billing panel', () => { const SAVED = { countryId: 'GB', From 448cc001db64179310f742577c453adba8a42e6c Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 17:36:27 +0100 Subject: [PATCH 25/30] refactor(TWO-25554): drop the unconsumed announceAddressUnavailable export Co-Authored-By: Claude Sonnet 5 --- Test/Js/amd-harness.js | 3 --- Test/Js/company-panel-chrome.test.js | 3 ++- view/frontend/web/js/model/company-search.js | 2 -- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Test/Js/amd-harness.js b/Test/Js/amd-harness.js index e91e4f1c..f6a78fbd 100644 --- a/Test/Js/amd-harness.js +++ b/Test/Js/amd-harness.js @@ -163,9 +163,6 @@ function defaultMocks() { // above: a spec that cares about what the write or the revert // actually does loads the real module. revertAutofilledAddress: function () { return 0; }, - announceAddressUnavailable: function (identity) { - identity.addressNotice('address unavailable'); - }, announceAddressUndeliverable: function (identity) { identity.addressNotice('address undeliverable'); }, diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index 185db14e..eb0e144a 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -305,7 +305,8 @@ describe('the company number is painted under its own panel\'s field', () => { const other = OTHER[actor]; const restored = { shipping: fixture.shippingNumber, billing: fixture.billingNumber }; const { panels } = boot(fixture); - expect(panels[actor].mountSelector()).toBe(FIELDS[actor]); + expect(tagged(description, panels[actor].mountSelector())) + .toEqual(tagged(description, FIELDS[actor])); // `watchCapturedIdentity` publishes on a macrotask, so an assertion // made before it has run denies a propagation that had not happened. diff --git a/view/frontend/web/js/model/company-search.js b/view/frontend/web/js/model/company-search.js index f969d238..c4895c73 100644 --- a/view/frontend/web/js/model/company-search.js +++ b/view/frontend/web/js/model/company-search.js @@ -992,8 +992,6 @@ define([ apiClientParams: apiClientParams, unwrapProxyResponse: unwrapProxyResponse, - /** @see announceAddressUnavailable */ - announceAddressUnavailable: announceAddressUnavailable, /** @see announceAddressUndeliverable */ announceAddressUndeliverable: announceAddressUndeliverable, From 72553ae75dacb038821740bd2e20872f3f55338e Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 17:38:04 +0100 Subject: [PATCH 26/30] docs(TWO-25554): collapse the round's comments to the invariant each carries Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-panel-chrome.test.js | 10 +++++----- view/frontend/web/js/model/company-capture.js | 13 ++++++------- view/frontend/web/js/model/company-identity.js | 12 +++++------- view/frontend/web/js/model/company-search-panel.js | 5 +---- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index eb0e144a..1e408623 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -428,10 +428,10 @@ describe('the "select a different sole trader" link belongs to its own panel', ( /* * TWO-25554: an address-lookup failure is the panel's own, rendered at the - * panel's own field. It used to travel through the resolved identity into the - * payment tile, where a shipping failure was displayed only while shipping - * happened to win and a billing failure told the buyer to "enter it below" a - * form that was not theirs. + * panel's own field. Carried through the resolved identity into the payment + * tile instead, a shipping failure reaches the buyer only while shipping + * happens to win the resolution, and a billing failure tells them to "enter it + * below" a form that is not theirs. */ describe('an address notice is painted under its own panel\'s field', () => { const NOTICE = 'We could not fetch this company\'s address. Please enter it below.'; @@ -623,7 +623,7 @@ describe('a panel that MOVES its mount leaves no chrome behind at the old host', return Object.assign({ tileInput: tileInput }, booted); } - test('the tile keeps neither the number nor a link to a flow it no longer hosts', async () => { + test('the tile keeps neither the number nor a link into a flow it does not host', async () => { const { soleTraderCalls } = await movesOffTheTile(); expect(numbersUnder(TILE_FORM)).toEqual([]); diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index 535bf289..98a2f8ff 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -128,12 +128,11 @@ define([ * The one "same as shipping" checkbox that speaks for the buyer. * * Core renders one per payment-method renderer, each with its own default, - * so a page-wide read is answered by whichever renderer the checkout output - * first — an inactive method's box as readily as the active one - * (TWO-25554). A single box on the page is unambiguous whatever its id; past - * that the active method's own is identified by core's id convention, and a - * checkout that renders several and abandons that convention has no - * attributable answer, leaving the quote as the honest source. + * so a page-wide read is answered by whichever the checkout output first — + * an inactive method's box as readily as the active one (TWO-25554). One + * box is unambiguous whatever its id; past that the active method's own is + * found by core's id convention, and a checkout that renders several and + * abandons that convention leaves the quote as the honest source. * * @returns {?object} jQuery(-shaped) set, or `null` when no box can be * attributed to the active method @@ -410,7 +409,7 @@ define([ * shipping form is the other panel's (TWO-25554). Keyed on the live mount * rather than on the selector matching, because core leaves the fieldset in * the DOM hidden once "same as shipping" is re-checked, and a hidden field - * is not somewhere the buyer can read what was written. + * is nowhere the buyer can read what was written. * * @returns {?object} jQuery set, or null */ diff --git a/view/frontend/web/js/model/company-identity.js b/view/frontend/web/js/model/company-identity.js index 2e0b255f..084a5071 100644 --- a/view/frontend/web/js/model/company-identity.js +++ b/view/frontend/web/js/model/company-identity.js @@ -239,14 +239,12 @@ }, /** - * WHICH COMPANY, and nothing else — for a caller mirroring this - * identity onto another one (`company-source-resolver.js`) that - * must copy it in a single notify, not one field at a time. + * WHICH COMPANY, and nothing else — every other field is the owning + * panel's own UI state, rendered at that panel's own field, and a + * mirror of it speaks for the wrong form (TWO-25554). * - * The capture mode and the address notice are the owning PANEL's - * own UI state, rendered at its own field by whichever panel holds - * them; travelling here they surfaced against the other panel's - * form or against no form at all (TWO-25554). + * One object rather than per-field reads because the mirror + * (`company-source-resolver.js`) must land in a single notify. * * @returns {object} */ diff --git a/view/frontend/web/js/model/company-search-panel.js b/view/frontend/web/js/model/company-search-panel.js index 7f59e2b4..bc249154 100644 --- a/view/frontend/web/js/model/company-search-panel.js +++ b/view/frontend/web/js/model/company-search-panel.js @@ -72,14 +72,11 @@ /** * What `_attach()` puts on the host field to announce the combobox. Left on * a field this panel has moved off, it offers a keyboard buyer a listbox - * that no longer exists and points `aria-controls` at a removed popover + * that is not there and points `aria-controls` at a removed popover * (TWO-25554). */ const COMBOBOX_ATTRIBUTES = ['role', 'aria-haspopup', 'aria-controls', 'aria-expanded']; - /** - * @param {?Element} field - */ function stripComboboxAttributes(field) { if (!field) return; COMBOBOX_ATTRIBUTES.forEach(function (attr) { From 5ad0f1fed0945feceb029e9aad78c5b26b315c0b Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 18:07:41 +0100 Subject: [PATCH 27/30] refactor(TWO-25554): drop four guards shaped around the harness stub The jQuery stub defines length, is, find and closest, so the missing-method cases the guards claimed to cover cannot occur. Co-Authored-By: Claude Sonnet 5 --- view/frontend/web/js/model/company-capture.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index 98a2f8ff..cd0af41a 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -97,15 +97,12 @@ define([ * in the DOM hidden once "same as shipping" is re-checked, and a hidden * field is neither a live mount nor a distinct address. * - * `.is` is feature-detected: jQuery-shaped test doubles model presence - * only, and presence is the best answer available for those. - * - * @param {object} $field a jQuery(-shaped) set + * @param {object} $field a jQuery set * @returns {boolean} */ function isVisible($field) { if (!$field.length) return false; - return typeof $field.is === 'function' ? $field.is(':visible') : true; + return $field.is(':visible'); } /** @@ -139,9 +136,7 @@ define([ */ function activeBillingToggle() { const $all = $(BILLING_TOGGLE_SELECTOR); - // `> 1` rather than `< 2`: a jQuery-shaped double reports no length at - // all, and presence is the best answer available for those. - if (!($all.length > 1)) return $all; + if ($all.length < 2) return $all; const selected = quote.paymentMethod(); const code = selected && selected.method; return code ? $(`#${BILLING_TOGGLE_ID_PREFIX}${code}`) : null; @@ -356,13 +351,12 @@ define([ if ($(ADDRESS_FORM_ROOT).length || $(BILLING_FORM_ROOT).length) return null; const $streets = $(ADDRESS_STREET_SELECTOR); if ($streets.length !== 1) return null; - if (typeof $streets.closest !== 'function') return null; // `closest()` answers with the NEAREST container, which on a themed // checkout is often a row holding the street line alone — a write scoped // there fills in a street and nothing else. The city is what makes the // container the whole address form. const $form = $streets.closest(ADDRESS_FORM_CONTAINER_SELECTOR); - if (!$form.length || typeof $form.find !== 'function') return null; + if (!$form.length) return null; return $form.find(ADDRESS_CITY_SELECTOR).length ? $form : null; } From 26f4768e439a26919863ca94207d9ff04d501bbb Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 18:08:24 +0100 Subject: [PATCH 28/30] docs(TWO-25554): correct activeBillingToggle's @returns and tag a dead row description Co-Authored-By: Claude Sonnet 5 --- Test/Js/company-search-resilience.test.js | 3 ++- view/frontend/web/js/model/company-capture.js | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Test/Js/company-search-resilience.test.js b/Test/Js/company-search-resilience.test.js index 46a229e4..8cf0443f 100644 --- a/Test/Js/company-search-resilience.test.js +++ b/Test/Js/company-search-resilience.test.js @@ -18,6 +18,7 @@ const $ = require('jquery'); const { + tagged, loadAmdModule, loadCompanyCapture, loadCompanySearchPanel, @@ -932,7 +933,7 @@ describe('a company-detail lookup that brings back no address says so', () => { companySearch.lookupCompanyAddress(BASE_CONFIG, { lookupId: 'company-b' }, PANEL_FORM, identity); settle(companySearch, requests); - expect(applied).toEqual(expectedApplied, description); + expect(tagged(description, applied)).toEqual(tagged(description, expectedApplied)); expect(identity.addressNotice()).toBe(''); }); diff --git a/view/frontend/web/js/model/company-capture.js b/view/frontend/web/js/model/company-capture.js index cd0af41a..7d5e920c 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -131,8 +131,9 @@ define([ * found by core's id convention, and a checkout that renders several and * abandons that convention leaves the quote as the honest source. * - * @returns {?object} jQuery(-shaped) set, or `null` when no box can be - * attributed to the active method + * @returns {?object} jQuery set — empty when several boxes are rendered + * and the active method's own is absent; `null` when several are + * rendered and no payment method is selected */ function activeBillingToggle() { const $all = $(BILLING_TOGGLE_SELECTOR); From d25e5414cde38fb0be1266896e9efb0696ba6995 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 18:19:36 +0100 Subject: [PATCH 29/30] test(TWO-25554): pin five panel-separation invariants nothing was holding Each fails under a mutation of the guard it names: the two identity guards in company-search, the quote's no-billing-address branch, _chromeNode's sibling-only scoping, and the billing panel's own-root country fallback. Co-Authored-By: Claude Sonnet 5 --- Test/Js/amd-harness.js | 6 +++- Test/Js/company-capture-billing-panel.test.js | 34 +++++++++++++++++++ Test/Js/company-panel-chrome.test.js | 23 +++++++++++-- Test/Js/company-search-address-writes.test.js | 23 +++++++++++++ 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/Test/Js/amd-harness.js b/Test/Js/amd-harness.js index f6a78fbd..e1e63b84 100644 --- a/Test/Js/amd-harness.js +++ b/Test/Js/amd-harness.js @@ -205,7 +205,11 @@ function defaultMocks() { // No DOM in the inert default: a spec that wants the live // address-form country read has to supply the real module (or its // own double) the same way it already does for the search itself. - currentAddressFormCountry: function () { return ''; }, + // DELEGATED: it is a pure read of the `$root` it is handed, and + // WHICH root each panel hands it is the invariant specs assert. + currentAddressFormCountry: function ($root) { + return realCompanySearch().currentAddressFormCountry($root); + }, // NOT inert — company-capture.js builds the billing panel's own // mount selectors from it, so a mock returning undefined would // exercise selectors production never uses. diff --git a/Test/Js/company-capture-billing-panel.test.js b/Test/Js/company-capture-billing-panel.test.js index 5bd02c35..1de854c2 100644 --- a/Test/Js/company-capture-billing-panel.test.js +++ b/Test/Js/company-capture-billing-panel.test.js @@ -131,6 +131,12 @@ function makeDom() { setCountry: function (selector, value) { node(selector).val(value); }, + // The unmounted fallback read goes through `$root.find(...)` rather + // than the flat country selector, which the stub answers with a node + // of its own. + setFormCountry: function (rootSelector, value) { + node(rootSelector).find('select[name="country_id"]').val(value); + }, // `document` is jsdom's real global, passed as an extraGlobal — the // stub's own `$(document)` resolves it to the same node every time // via `String(document)`, exactly as company-capture.js's own calls do. @@ -244,6 +250,34 @@ describe('each panel reads ONLY its own address form\'s country — never a shar dom.setCountry(BILLING_COUNTRY, 'dk'); expect(capture.shipping.countryCode()).toBe('se'); }); + + test('unmounted, billing falls back to its OWN form and not the shipping form', () => { + // Unmounted there is no adjacent select to read, and with no country on + // the quote either the live form read is the only answer left. + const { capture, dom } = load(); + dom.setFormCountry(BILLING_FORM, 'dk'); + dom.setFormCountry(ADDRESS_FORM, 'se'); + capture.billing.start(); + + expect(capture.billing.mountSelector()).toBe(''); + expect(capture.billing.countryCode()).toBe('dk'); + }); +}); + +describe('billingRoleIdentity() follows billingIsDistinct(), not the presence of a panel', () => { + test('a quote holding no billing address at all leaves shipping in the billing role', () => { + const { capture, dom, quote } = load(); + dom.setVisible(BILLING_FIELD, true); + dom.setChecked(BILLING_TOGGLE, false); + capture.shipping.start(); + capture.billing.start(); + capture.shipping.selectCompany({ text: 'Shipping Co', companyId: '111', lookupId: 'l1' }); + capture.billing.selectCompany({ text: 'Billing Co', companyId: '222', lookupId: 'l2' }); + + quote.billingAddress(null); + + expect(capture.billingRoleIdentity().companyId()).toBe('111'); + }); }); describe('the two panels\' captures are independent — a pick on one never reaches the other', () => { diff --git a/Test/Js/company-panel-chrome.test.js b/Test/Js/company-panel-chrome.test.js index 1e408623..365fb962 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -323,9 +323,8 @@ describe('the company number is painted under its own panel\'s field', () => { ); test('a panel whose own form carries no number field claims neither neighbour\'s', () => { - // Two billing fieldsets, a number restored into each. The shipping form - // has no number field, so the nearest ancestor holding any spans both — - // and neither is answerable as this panel's. + // Two billing fieldsets, a number restored into each, and a shipping + // form with no number field of its own. const { panels } = boot({ shippingCompanyIdField: false, billingNumber: '999999999', @@ -384,6 +383,24 @@ describe('chrome never enters the popover\'s positioning context', () => { expect(tagged(description, numbersIn(actor).length)).toEqual(tagged(description, 1)); expect(tagged(description, linksIn(actor))).toEqual(tagged(description, 1)); }); + + test.each(DIRECTIONS)('%s never claims chrome-classed markup inside the wrap (%s)', (actor, description) => { + const { panels } = boot(); + panels[actor].adoptSoleTrader(NUMBERED_TRADER); + const wrap = document.querySelector(`${FORMS[actor]} .two-company-field-wrap`); + const insidePopover = document.createElement('div'); + insidePopover.className = NUMBER_CLASS; + wrap.appendChild(insidePopover); + + panels[actor].renderChrome(); + + expect(tagged(description, insidePopover.isConnected)).toEqual(tagged(description, true)); + const own = Array.prototype.filter.call( + wrap.parentElement.children, + (child) => child.classList.contains(NUMBER_CLASS) + ); + expect(tagged(description, own.length)).toEqual(tagged(description, 1)); + }); }); describe('the "select a different sole trader" link belongs to its own panel', () => { diff --git a/Test/Js/company-search-address-writes.test.js b/Test/Js/company-search-address-writes.test.js index 1eb02694..8615369d 100644 --- a/Test/Js/company-search-address-writes.test.js +++ b/Test/Js/company-search-address-writes.test.js @@ -570,6 +570,29 @@ describe('a replacement pick does not strand the previous line 2', () => { }); }); +describe('a call with no owning identity is refused before it touches the form', () => { + // The write record is keyed on the calling panel's identity in a WeakMap, + // so a call with none throws part-way and leaves the form half-written. + test.each([ + [ + 'an address write', + (model) => model.applyAddress(COMPANY_A, $(PRIMARY), undefined), + 'no identity means no reversible recording, so nothing is written' + ], + [ + 'a retraction', + (model) => model.revertAutofilledAddress($(PRIMARY), undefined), + 'no identity means no recording to read, so nothing is cleared' + ] + ])('%s', (_label, act, description) => { + const model = renderCheckout({ regions: true }); + buyerTypes(PRIMARY, 'city', 'Ashford'); + + expect(tagged(description, act(model))).toEqual(tagged(description, 0)); + expect(tagged(description, read(PRIMARY, 'city'))).toEqual(tagged(description, 'Ashford')); + }); +}); + describe('a shipping form core is not using is not the buyer\'s own form', () => { test.each([ [{ hiddenPrimary: true }, false, 'hidden inside the saved-addresses wrapper'], From 700736720033af2a4c4e7228f1b2cd9908536589 Mon Sep 17 00:00:00 2001 From: Douglas Lindsay Date: Tue, 1 Sep 2026 18:19:40 +0100 Subject: [PATCH 30/30] refactor(TWO-25554): drop the restored-number multi-match check _ownFormRoot's ceiling is the panel's own address form, and the layout processor injects exactly one company_id field per form, so no ancestor inside that ceiling can hold two. Co-Authored-By: Claude Sonnet 5 --- view/frontend/web/js/model/company-capture-component.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/view/frontend/web/js/model/company-capture-component.js b/view/frontend/web/js/model/company-capture-component.js index d77ecfcf..0416f8ae 100644 --- a/view/frontend/web/js/model/company-capture-component.js +++ b/view/frontend/web/js/model/company-capture-component.js @@ -660,9 +660,7 @@ let node = field.parentElement; while (node && root.contains(node)) { const found = node.querySelectorAll(RESTORED_NUMBER_SELECTOR); - // Exactly one: several under one ancestor means it spans a second - // address form, so neither is answerable as this panel's own. - if (found.length === 1) return found[0].value || ''; + if (found.length) return found[0].value || ''; node = node.parentElement; } return '';