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

bozojovicic · 2024-11-04T14:46:25Z

IMS country should not be used in https://www.adobe.com/web_commerce_artifact calls

When data-ims-country is updated, the /web_commerce_artifact call is triggered again with IMS country in query parameters. By removing that data attribute from observedAttributes I prevented that.
On catalog pages I found one hidden CTA where /web_commerce_artifact is called with IMS country even after this change so I had to add overrides.imsCountry = null to fix that case as well.

Test URL :
https://main--cc--adobecom.hlx.page/products/catalog?milolibs=mwpw161289ctaims--milo--bozojovicic

Resolves: MWPW-161289

Test URLs:

aem-code-sync · 2024-11-04T14:46:48Z

	Page	Scores	Audits	Google
📱	/drafts/bozo/price-cta?martech=off
🖥️	/drafts/bozo/price-cta?martech=off

codecov · 2024-11-04T14:47:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.38%. Comparing base (45f953f) to head (7a29c6c).
Report is 5 commits behind head on stage.

Additional details and impacted files

@@            Coverage Diff             @@
##            stage    #3136      +/-   ##
==========================================
+ Coverage   96.37%   96.38%   +0.01%     
==========================================
  Files         245      245              
  Lines       56686    56688       +2     
==========================================
+ Hits        54630    54638       +8     
+ Misses       2056     2050       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

… re-generated

yesil · 2024-11-05T16:09:20Z

libs/features/mas/commerce/src/checkout-link.js

@@ -51,7 +51,6 @@ export class CheckoutLink extends HTMLAnchorElement {
            'data-checkout-workflow',
            'data-checkout-workflow-step',
            'data-extra-options',
-            'data-ims-country',


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

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.

yesil · 2024-11-05T16:09:41Z

libs/features/mas/commerce/test/checkout.test.js

        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',


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

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?

libs/features/mas/commerce/test/checkout.test.js

yesil

LGTM, but have a question.

yesil · 2024-11-06T11:04:33Z

libs/features/mas/commerce/src/checkout-link.js

@@ -121,6 +121,7 @@ export class CheckoutLink extends HTMLAnchorElement {
                if (countryCode) this.dataset.imsCountry = countryCode;
            }, ignore);
        }
+        overrides.imsCountry = null;


do we need this line?

@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.

afmicka · 2024-11-08T11:52:37Z

@yesil @3ch023 @npeltier @bozojovicic
Even though this seems to be fixed for catalog page, for studio I still get placeholder-failed and ims RS. Could you please check? Do we need to separate jiras for catalog and studio in this matter?
https://main--mas--adobecom.hlx.page/studio.html?milolibs=mwpw161289ctaims--milo--bozojovicic#path=ccd&query=ccd

yesil · 2024-11-08T15:28:45Z

@bozojovicic mas.js build is missing in your PR, can you rebuild and commit it please?

yesil · 2024-11-08T15:29:57Z

@afmicka thanks, @bozojovicic PR was missing mas.js build, asked him to rebuild it.
but on top, we need a small PR to mas to remove ims atribute at the end: https://github.com/adobecom/mas/blob/main/studio/src/aem/render-view.js#L51

bozojovicic · 2024-11-08T17:21:21Z

@yesil new mas.js build pushed
but who will do this part ?
we need a small PR to mas to remove ims atribute at the end: https://github.com/adobecom/mas/blob/main/studio/src/aem/render-view.js#L51
This is "uncharted territory" for me.

3ch023 · 2024-11-11T15:47:05Z

Checked on
https://main--cc--adobecom.hlx.live/products/photoshop?milolibs=mwpw161289ctaims--milo--bozojovicic
looks good with my DE user. WCS request country=US, CTA link country=DE

yesil · 2024-11-12T12:13:38Z

@bozojovicic @afmicka sorry, my message regarding the ims attribute, it was completely wrong, please discard it.

afmicka · 2024-11-12T13:16:44Z

@bozojovicic @yesil @3ch023
this time, CTA placeholders are resolved in studio but on catalog page if i log in with the user with FR as ims country, the ctas resolve to US (the site locale and not user's ims). Does this need to be fixed in dexter (since these are twp modals)?
https://main--cc--adobecom.hlx.page/products/catalog?milolibs=mwpw161289ctaims--milo--bozojovicic#miniplans-buy-acrobat

afmicka · 2024-11-13T11:59:04Z

will log a separate jira for the issue mentioned above. Approving this one.

MWPW-161289 Catalog CTA IMS country used in commerce calls

9f8b990

Bozo Jovicic added 2 commits November 4, 2024 15:57

Trigger Build

cc9dc12

MWPW-161289 Catalog CTA IMS country used in commerce calls

dd8a4cc

bozojovicic marked this pull request as ready for review November 4, 2024 16:38

bozojovicic requested review from npeltier, yesil and 3ch023 November 4, 2024 16:42

Bozo Jovicic added 4 commits November 5, 2024 14:19

Merge branch 'stage' into mwpw161289ctaims

2ba5ab3

MWPW-161289 Catalog CTA IMS country used in commerce calls - commerce…

9f9a24d

… re-generated

Trigger Build

79c4d23

Trigger Build

0b3c70c

yesil requested changes Nov 5, 2024

View reviewed changes

MWPW-161289 Catalog CTA IMS country used in commerce calls

5c5064b

yesil reviewed Nov 6, 2024

View reviewed changes

libs/features/mas/commerce/test/checkout.test.js Outdated Show resolved Hide resolved

Bozo Jovicic added 2 commits November 6, 2024 11:48

MWPW-161289 Catalog CTA IMS country used in commerce calls

242c5ae

Trigger Build

6035cac

yesil approved these changes Nov 6, 2024

View reviewed changes

Expired test promo fixed

f6bf603

yesil approved these changes Nov 6, 2024

View reviewed changes

Bozo Jovicic added 6 commits November 6, 2024 13:33

Expired test promo fixed

9d8116e

Trigger Build

ff0ad3d

Trigger Build

df6a693

Merge branch 'stage' into mwpw161289ctaims

5367e28

New commerce build

3a3688e

Expired test promo rollback

53c8870

Axelcureno approved these changes Nov 8, 2024

View reviewed changes

Bozo Jovicic added 2 commits November 8, 2024 12:41

Merge branch 'stage' into mwpw161289ctaims

d053704

New commerce build

64586cd

Trigger Build

20d376b

Trigger Build

c1ffe9f

New mas build

b0aec3a

3ch023 added the commerce label Nov 11, 2024

Bozo Jovicic added 2 commits November 12, 2024 13:23

Merge branch 'stage' into mwpw161289ctaims

f4b66c5

New mas and commerce build

7a29c6c

afmicka added verified PR has been E2E tested by a reviewer Ready for Stage labels Nov 13, 2024

milo-pr-merge bot merged commit 031b5d2 into adobecom:stage Nov 13, 2024
24 checks passed

milo-pr-merge bot mentioned this pull request Nov 13, 2024

[Release] Stage to Main #3187

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

bozojovicic commented Nov 4, 2024 •

edited

Loading

aem-code-sync bot commented Nov 4, 2024 •

edited

Loading

codecov bot commented Nov 4, 2024 •

edited

Loading

yesil Nov 5, 2024

bozojovicic Nov 6, 2024

yesil Nov 5, 2024

bozojovicic Nov 6, 2024

yesil left a comment

yesil Nov 6, 2024

bozojovicic Nov 6, 2024

afmicka commented Nov 8, 2024 •

edited

Loading

yesil commented Nov 8, 2024

yesil commented Nov 8, 2024

bozojovicic commented Nov 8, 2024

3ch023 commented Nov 11, 2024

yesil commented Nov 12, 2024

afmicka commented Nov 12, 2024 •

edited

Loading

afmicka commented Nov 13, 2024

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

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

Conversation

bozojovicic commented Nov 4, 2024 • edited Loading

aem-code-sync bot commented Nov 4, 2024 • edited Loading

codecov bot commented Nov 4, 2024 • edited Loading

Codecov Report

yesil Nov 5, 2024

Choose a reason for hiding this comment

bozojovicic Nov 6, 2024

Choose a reason for hiding this comment

yesil Nov 5, 2024

Choose a reason for hiding this comment

bozojovicic Nov 6, 2024

Choose a reason for hiding this comment

yesil left a comment

Choose a reason for hiding this comment

yesil Nov 6, 2024

Choose a reason for hiding this comment

bozojovicic Nov 6, 2024

Choose a reason for hiding this comment

afmicka commented Nov 8, 2024 • edited Loading

yesil commented Nov 8, 2024

yesil commented Nov 8, 2024

bozojovicic commented Nov 8, 2024

3ch023 commented Nov 11, 2024

yesil commented Nov 12, 2024

afmicka commented Nov 12, 2024 • edited Loading

afmicka commented Nov 13, 2024

bozojovicic commented Nov 4, 2024 •

edited

Loading

aem-code-sync bot commented Nov 4, 2024 •

edited

Loading

codecov bot commented Nov 4, 2024 •

edited

Loading

afmicka commented Nov 8, 2024 •

edited

Loading

afmicka commented Nov 12, 2024 •

edited

Loading