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/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 9b800224..e1e63b84 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: quoteAddress(), + billingAddress: quoteAddress(), getTotals: function () { return makeObservable({}); }, getQuoteId: function () { return null; }, paymentMethod: makeObservable(null), @@ -160,8 +163,8 @@ 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'); }, hasPrimaryAddressForm: function () { return true; }, isDegradedResponse: function () { return false; }, @@ -202,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. @@ -266,6 +273,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 = []; @@ -295,6 +313,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 = []; @@ -831,6 +879,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-billing-panel.test.js b/Test/Js/company-capture-billing-panel.test.js index ffc40386..1de854c2 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,9 +125,18 @@ 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); }, + // 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. @@ -124,20 +148,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 +181,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 +206,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 +216,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 +230,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 +239,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(); @@ -204,12 +250,40 @@ 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', () => { 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 +294,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 +318,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 +332,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 +384,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 +398,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 +519,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 +551,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 +579,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. */ @@ -516,40 +591,152 @@ describe('the quote\'s billing address seeds the panel owning the billing role', expect(capture.billing.mountSelector()).toBe(''); } - function billingQuoteAddress(company) { - return { + /** + * 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 + * @returns {object} quote address + */ + function quoteNotifiesBilling(booted, company) { + const address = quoteAddressValue({ company: company, telephone: '+47 123 45 678', 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); } - test('through the quote\'s billing address, with the fieldset away it seeds SHIPPING', () => { - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + /** + * 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 () => { + // 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(quoteNotifiesBilling(booted, 'Saved Billing Co')); + await flushCapture(); + + 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 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('Billing Co')); + renderer.updateBillingAddress(quoteNotifiesBilling(booted, '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'); }); - test('re-checking "same as shipping" retires the billing panel\'s own capture', () => { - const { capture, dom } = load(); - billingPicks(capture, dom, 'Billing Co'); + test('re-checking "same as shipping" retires the billing panel\'s own capture', async () => { + 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 + // 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('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', () => { @@ -558,12 +745,14 @@ describe('the quote\'s billing address seeds the panel owning the billing role', // 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'); @@ -575,8 +764,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); @@ -590,12 +780,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'); }); @@ -604,12 +795,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'); @@ -619,8 +811,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' }); @@ -683,8 +876,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); @@ -697,8 +890,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-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-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-chrome.test.js b/Test/Js/company-panel-chrome.test.js index 314c2e74..365fb962 100644 --- a/Test/Js/company-panel-chrome.test.js +++ b/Test/Js/company-panel-chrome.test.js @@ -22,7 +22,9 @@ const { loadCompanySearchPanel, defaultMocks, brandConfigMock, - installAsyncSimulation + installAsyncSimulation, + tagged, + quoteAddress } = require('./amd-harness'); const SEARCH = 'view/frontend/web/js/model/company-search.js'; @@ -39,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' }; @@ -94,7 +97,7 @@ function renderCheckout(options) { '
' + '' + `
` + - addressFields('NO', options.billingNumber) + + addressFields('NO', options.billingNumber, options.billingCompanyIdField) + '
' + extraBilling + '
' + @@ -144,7 +147,7 @@ function boot(options) { {}, defaultMocks()['Magento_Checkout/js/model/quote'], { - billingAddress: function () { return { countryId: 'GB' }; }, + billingAddress: quoteAddress({ countryId: 'GB' }, 'billing'), isVirtual: function () { return false; } } ), @@ -177,6 +180,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( @@ -190,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'); @@ -198,17 +219,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]); @@ -216,28 +238,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' }; @@ -245,9 +268,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', () => { @@ -265,10 +288,43 @@ describe('the company number is painted under its own panel\'s field', () => { expect(document.querySelectorAll(`.${NUMBER_CLASS}`)).toHaveLength(0); }); + test.each([ + [ + 'shipping', + 'the shipping form has no number field and one billing neighbour has one', + { shippingCompanyIdField: false, billingNumber: '555555555' } + ], + [ + 'billing', + '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, description, fixture) => { + const other = OTHER[actor]; + const restored = { shipping: fixture.shippingNumber, billing: fixture.billingNumber }; + const { panels } = boot(fixture); + 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. + 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 - // 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', @@ -281,35 +337,101 @@ 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, description) => { + const { panels } = boot(); + + panels[actor].adoptSoleTrader(NUMBERED_TRADER); + + const wrap = document.querySelector(`${FORMS[actor]} .two-company-field-wrap`); + expect(wrap).not.toBeNull(); + 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(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, description) => { + const { panels } = boot(); + panels[actor].adoptSoleTrader(NUMBERED_TRADER); + + panels[actor].renderChrome(); + + const wrap = document.querySelector(`${FORMS[actor]} .two-company-field-wrap`); + 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)); + }); + + 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', () => { - 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]); @@ -317,6 +439,237 @@ 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]])); + }); +}); + +/* + * TWO-25554: an address-lookup failure is the panel's own, rendered at the + * 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.'; + 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. */ + 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); + }); +}); + +/* + * 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' }; + + 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 into a flow it does not host', 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/Test/Js/company-panel-independence.test.js b/Test/Js/company-panel-independence.test.js index 2e28227f..6ee0bc1a 100644 --- a/Test/Js/company-panel-independence.test.js +++ b/Test/Js/company-panel-independence.test.js @@ -28,7 +28,10 @@ const { defaultMocks, brandConfigMock, installAsyncSimulation, - tagged + tagged, + quoteAddress, + quoteAddressValue, + makeObservable } = require('./amd-harness'); const SEARCH = 'view/frontend/web/js/model/company-search.js'; @@ -123,7 +126,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 +155,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; } } ); @@ -355,9 +359,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', () => { @@ -387,8 +388,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(''); @@ -409,7 +410,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'); @@ -461,7 +462,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); @@ -474,6 +475,233 @@ 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, '')); + }); +}); + +/* + * 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'); + }); +}); + +/* + * 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', @@ -506,11 +734,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 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 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, @@ -521,8 +748,26 @@ 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.identities.shipping.companyName()).toBe('Saved Billing Co'); expect(booted.capture.identity.companyId()).toBe('555'); expect(booted.identities.billing.companyId()).toBe(''); }); @@ -652,7 +897,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-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..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(); @@ -469,7 +470,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 +502,23 @@ 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); + 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); + 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. @@ -502,17 +530,24 @@ 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([ [ - 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..8615369d 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'); @@ -529,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'], diff --git a/Test/Js/company-search-country-switch.test.js b/Test/Js/company-search-country-switch.test.js index da40fd55..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'; @@ -268,7 +276,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: {} }; } @@ -369,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; } }); @@ -410,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 })); + } }; } @@ -429,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.$, @@ -445,13 +458,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 +473,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 +495,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 +526,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/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/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/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-company-selection.test.js b/Test/Js/gateway-method-company-selection.test.js index 89abb49a..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', () => { @@ -472,27 +485,49 @@ 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. - const { renderer, billingAddress } = loadWithSections({}); + // (TWO-25554). + const { renderer, billingAddress, capture } = loadWithSections({}); renderer.fillCustomerData(); billingAddress({ - getCacheKey: () => 'k3', + getCacheKey: () => 'k', countryId: 'GB', telephone: '+47 123 45 678', company: 'Billing Example Ltd', 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/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/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-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/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/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 61884c6a..e35d86e5 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'; @@ -386,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. @@ -422,7 +429,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' }) } ) }) ); @@ -455,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/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php b/Test/Unit/Plugin/Model/Checkout/LayoutProcessorPluginTest.php index 9c15fb56..0c90704a 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 { @@ -579,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'], ]; } 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/css/style.css b/view/frontend/web/css/style.css index 0b3c0256..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 @@ -489,8 +491,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 70e08ae1..0416f8ae 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. @@ -113,6 +122,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 @@ -304,10 +316,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); }; @@ -441,6 +456,11 @@ 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) { // Neither host is on the page any more. Forgetting where the control // was is what stops `adjacentCountry()` answering for a form that has @@ -553,15 +573,55 @@ * * @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(); this.renderSoleTraderLink(); + this.renderAddressNotice(); + }; + + /** + * 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; + [COMPANY_NUMBER_CLASS, SOLE_TRADER_LINK_CLASS, ADDRESS_NOTICE_CLASS].forEach(function (className) { + const node = self._chromeNode(className); + if (node) node.remove(); + }); }; /** @@ -584,8 +644,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,17 +655,34 @@ 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. - if (found.length === 1) return found[0].value || ''; + if (found.length) return found[0].value || ''; node = node.parentElement; } 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. * @@ -614,9 +691,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; @@ -624,7 +702,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); }; /** @@ -634,9 +712,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(); @@ -656,7 +735,40 @@ 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); + }; + + /** + * 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-capture.js b/view/frontend/web/js/model/company-capture.js index 0b75a713..7d5e920c 100644 --- a/view/frontend/web/js/model/company-capture.js +++ b/view/frontend/web/js/model/company-capture.js @@ -65,15 +65,25 @@ 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]"]'; /** @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; @@ -87,20 +97,64 @@ 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'); } - /** 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 = 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 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 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); + if ($all.length < 2) 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. + * + * @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(); } /** @@ -297,9 +351,14 @@ 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; + // `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) return null; + return $form.find(ADDRESS_CITY_SELECTOR).length ? $form : null; } /** @@ -325,14 +384,45 @@ 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 + * 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 */ function shippingWriteTarget() { const root = shippingWriteRoot(); - if (!root) companySearch.announceAddressUnavailable(shippingIdentity); + if (!root) companySearch.announceAddressUndeliverable(shippingIdentity); + 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 nowhere 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; } @@ -350,6 +440,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; @@ -367,10 +458,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()); @@ -388,12 +479,12 @@ 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: '', fieldExists: function (selector) { if (!selector) return false; - // See isVisible() and billingIsDistinct() above. return isVisible($(selector)); }, getAdjacentCountry: function () { @@ -403,18 +494,18 @@ define([ companySearch.lookupCompanyAddress( billingComponent.config(), selectedItem, - $(BILLING_FORM_ROOT), + billingWriteRoot(), billingIdentity ); }, revertAutofilledAddress: function () { - companySearch.revertAutofilledAddress($(BILLING_FORM_ROOT)); + companySearch.revertAutofilledAddress(billingWriteRoot(), billingIdentity); }, applyBuyerAddress: function (source) { - companySearch.applyAddress(source, $(BILLING_FORM_ROOT)); + 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)); @@ -443,6 +534,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); } }); diff --git a/view/frontend/web/js/model/company-identity.js b/view/frontend/web/js/model/company-identity.js index b8f4a604..084a5071 100644 --- a/view/frontend/web/js/model/company-identity.js +++ b/view/frontend/web/js/model/company-identity.js @@ -199,12 +199,27 @@ notify(); }, - /** Abandon both halves — a country change invalidates the registry. */ + /** + * 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). + * + * `soleTraderAvailable` is a property of the country rather than 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(); }, @@ -224,14 +239,21 @@ }, /** - * Every field, as one plain object — 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. + * 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). + * + * One object rather than per-field reads because the mirror + * (`company-source-resolver.js`) must land in a single notify. * * @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-search-panel.js b/view/frontend/web/js/model/company-search-panel.js index 804a1625..bc249154 100644 --- a/view/frontend/web/js/model/company-search-panel.js +++ b/view/frontend/web/js/model/company-search-panel.js @@ -69,6 +69,21 @@ // 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 is not there and points `aria-controls` at a removed popover + * (TWO-25554). + */ + const COMBOBOX_ATTRIBUTES = ['role', 'aria-haspopup', 'aria-controls', 'aria-expanded']; + + 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 +330,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 +970,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 +1094,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; diff --git a/view/frontend/web/js/model/company-search.js b/view/frontend/web/js/model/company-search.js index 5aecc75b..c4895c73 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 */ @@ -111,29 +112,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: 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(); /** - * @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 +602,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 +631,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( @@ -792,7 +794,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(''); } @@ -849,10 +864,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); @@ -976,8 +992,8 @@ define([ apiClientParams: apiClientParams, unwrapProxyResponse: unwrapProxyResponse, - /** @see announceAddressUnavailable */ - announceAddressUnavailable: announceAddressUnavailable, + /** @see announceAddressUndeliverable */ + announceAddressUndeliverable: announceAddressUndeliverable, /** * Run one company search and hand back rows the panel can render. @@ -999,8 +1015,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}>} */ @@ -1114,7 +1130,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; } @@ -1150,7 +1166,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 +1214,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 +1365,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/model/company-source-resolver.js b/view/frontend/web/js/model/company-source-resolver.js index 719ad65d..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. @@ -38,8 +39,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/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. 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..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 @@ -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); }); @@ -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" > - - - -