Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MWPW-161289 Catalog CTA IMS country used in commerce calls #3136

Merged
merged 24 commits into from
Nov 13, 2024
Merged

MWPW-161289 Catalog CTA IMS country used in commerce calls #3136

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9f8b990
MWPW-161289 Catalog CTA IMS country used in commerce calls
Nov 4, 2024
cc9dc12
Trigger Build
Nov 4, 2024
dd8a4cc
MWPW-161289 Catalog CTA IMS country used in commerce calls
Nov 4, 2024
2ba5ab3
Merge branch 'stage' into mwpw161289ctaims
Nov 5, 2024
9f9a24d
MWPW-161289 Catalog CTA IMS country used in commerce calls - commerce…
Nov 5, 2024
79c4d23
Trigger Build
Nov 5, 2024
0b3c70c
Trigger Build
Nov 5, 2024
5c5064b
MWPW-161289 Catalog CTA IMS country used in commerce calls
Nov 6, 2024
242c5ae
MWPW-161289 Catalog CTA IMS country used in commerce calls
Nov 6, 2024
6035cac
Trigger Build
Nov 6, 2024
f6bf603
Expired test promo fixed
Nov 6, 2024
9d8116e
Expired test promo fixed
Nov 6, 2024
ff0ad3d
Trigger Build
Nov 6, 2024
df6a693
Trigger Build
Nov 6, 2024
5367e28
Merge branch 'stage' into mwpw161289ctaims
Nov 6, 2024
3a3688e
New commerce build
Nov 6, 2024
53c8870
Expired test promo rollback
Nov 6, 2024
d053704
Merge branch 'stage' into mwpw161289ctaims
Nov 8, 2024
64586cd
New commerce build
Nov 8, 2024
20d376b
Trigger Build
Nov 8, 2024
c1ffe9f
Trigger Build
Nov 8, 2024
b0aec3a
New mas build
Nov 8, 2024
f4b66c5
Merge branch 'stage' into mwpw161289ctaims
Nov 12, 2024
7a29c6c
New mas and commerce build
Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libs/deps/mas/commerce.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion libs/features/mas/commerce/src/checkout-link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export class CheckoutLink extends HTMLAnchorElement {
'data-checkout-workflow',
'data-checkout-workflow-step',
'data-extra-options',
'data-ims-country',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should stay, it will cause a re-render which is expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, if re-render is expected, I returned it back.
On re-render the line overrides.imsCountry = null will override the country from IMS, and the page country will be used and since offers for the page country are already loaded on the first render, we will not have another WCS call.

'data-perpetual',
'data-promotion-code',
'data-quantity',
Expand Down Expand Up @@ -121,6 +120,7 @@ export class CheckoutLink extends HTMLAnchorElement {
if (countryCode) this.dataset.imsCountry = countryCode;
}, ignore);
}
overrides.imsCountry = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yesil Yes, this will annul the IMS country in options and then the "page country" will be used, the same country that was used in the first render. And since we already have offers loaded for that country, no new WCS call will be triggered.

const options = service.collectCheckoutOptions(overrides, this);
if (!options.wcsOsi.length) return false;
let extraOptions;
Expand Down
4 changes: 2 additions & 2 deletions libs/features/mas/commerce/test/checkout.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,15 @@ describe('class "CheckoutLink"', () => {
);
});

it('renders link with ims country', async () => {
it('renders link with page country, not ims country', async () => {
bozojovicic marked this conversation as resolved.
Show resolved Hide resolved
mockIms('CH');
const service = await initMasCommerceService();
const checkoutLink = mockCheckoutLink('abm');
await service.imsCountryPromise;
await delay(1);
await checkoutLink.onceSettled();
expect(checkoutLink.href).to.equal(
'https://commerce.adobe.com/store/email?items%5B0%5D%5Bid%5D=632B3ADD940A7FBB7864AA5AD19B8D28&cli=adobe_com&ctx=fp&co=CH&lang=en',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CH is expected here, what is NOT expect is an extra WCS call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are fully reverted. So what I did is :
On initial render we use page country and there are no problems.
When IMS country is set in data-ims-country we have re-render and then before WCS offers are loaded I need to override IMS country with page country overrides.imsCountry = null
After WCS offers are loaded (no new WCS call is triggered since the WCS url is identical as in the first render)
and before checkout link is generated I need to set the country back to IMS country
options.country = this.dataset.imsCountry || options.country

Is it ok now?

'https://commerce.adobe.com/store/email?items%5B0%5D%5Bid%5D=632B3ADD940A7FBB7864AA5AD19B8D28&cli=adobe_com&ctx=fp&co=US&lang=en',
);
});

Expand Down
Loading