MWPW-159257 refactor mas enablement #3012

npeltier · 2024-10-04T09:21:54Z

move of wcms-commerce -> mas-commerce-service
move of some public functions to mas-commerce-service,
remove mas.js code for initialization, everything needs to go through the component,
split of settings code with locale settings, make milo locale settings an explicit function,
unit tests,
updates of docs, especially mas.js.html with that component

Test URLs:

- move of wcms-commerce -> mas-commerce-service - move of some public functions to mas-commerce-service, - remove mas.js code for initialization, everything needs to go through the component, - split of settings code with locale settings, make milo locale settings an explicit function, - unit tests, - updates of docs, especially mas.js.html with that component

yesil · 2024-10-16T06:53:18Z

@npeltier for instance checkout link element inherits from the native HTMLAnchorElement.
I find AnchorElement is a good suffix what all custom elements inheriting from it should be using.
That way, when you look at the stack trace, not only you know it is CheckoutLinkAnchorElement, but also you know it is a custom element that is a link that renders content.
Since we already have npm modules(hopefully we can combine commerce & web-components) and folders to organise files, I think giving long file names is not necessary.

not documenting it as i'm not sure it's needed by consumers

yesil · 2024-10-17T11:14:09Z

libs/blocks/merch/merch.js

@@ -3,7 +3,7 @@ import {
 } from '../../utils/utils.js';
 import { replaceKey } from '../../features/placeholders.js';

-export const CHECKOUT_LINK_CONFIG_PATH = '/commerce/checkout-link.json'; // relative to libs.
+export const CHECKOUT_LINK_CONFIG_PATH = '/commerce/checkout-link-anchor-element.json'; // relative to libs.


@npeltier this is an excel file, I don't think we should rename it, especially now.

yesil · 2024-10-17T11:17:55Z

libs/features/mas/commerce/src/mas-commerce-service.js

+        });
+        //commerce parameters
+        ['checkoutWorkflowStep', 'forceTaxExclusive', 'checkoutClientId'].forEach((attribute) => {
+            const value = this.getAttribute(attribute);


I think we should adopt html friendly attribute names: checkout-workflow-step, force-tax-exclusive checkout-client-id

yesil · 2024-10-17T11:20:01Z

libs/features/mas/mas/dist/mas.js.map

might be a unintentional commit, let's remove this.

yesil · 2024-10-17T11:21:52Z

libs/features/mas/web-components/src/global.css.js

@@ -25,6 +24,8 @@ styles.innerHTML = `
    --consonant-merch-card-cta-font-size: 15px;

    /* headings */
+    --merch-card-heading-xxs-font-size: 16px;


these changes that are supposed to be shipped as part of @Axelcureno's PRs, if shipped earlier, might cause regression.
I recommend to met his PRs first.

the other way around i would say

yesil · 2024-10-17T12:50:13Z

libs/features/mas/commerce/src/mas-commerce-service.js

rename to mas-commerce-service-element ?

i would rather not, a web component is an element, we know it

narcis-radu · 2024-10-18T07:53:34Z

libs/blocks/merch/merch.js

+  cis_ru: 'AZ_ru',
+  sea: 'SG_en',
+  th_en: 'TH_en',
+  th_th: 'TH_th',


I see there are missing locales here, is this intentional?

this is the locales we are living with at the moment, i just moved them to a different place. Do you have an example?

narcis-radu · 2024-10-18T07:59:41Z

libs/blocks/merch/merch.js

+      document.head.append(service);
+      await service.readyPromise;
+      service.imsSignedInPromise?.then((isSignedIn) => {
+        if (isSignedIn) {


Suggested change

if (isSignedIn) {

if (isSignedIn) fetchEntitlements();

also, we might not need to wrap the outside function

narcis-radu · 2024-10-18T08:06:15Z

libs/features/mas/commerce/src/inline-price.js

+    'MY_ms',
+    'NZ_en',
+    'TH_en',
+    'TH_th',


Locales missing

these are locales where tax is displayed for all segments by default, not all of them

afmicka · 2024-10-18T10:51:44Z

@npeltier

commerce.env=stage is making requests to www.adobe.com instead of www.stage.adobe.com
https://mwpw-159257--milo--npeltier.hlx.page/drafts/nala/features/commerce/promo-placeholders?commerce.env=stage

commerce.landscape does not work (has effect) on your branch,
https://mwpw-159257--milo--npeltier.hlx.page/drafts/nala/features/commerce/promo-placeholders?commerce.landscape=draft

on main branch: https://main--milo--adobecom.hlx.page/drafts/nala/features/commerce/promo-placeholders?commerce.landscape=draft

the rest of the regression looks OK but after fixing this, it needs another round. @Roycethan could you follow-up after the fix? I will be away starting this afternoon until Thursday morning.

Roycethan · 2024-10-18T23:57:21Z

Regressions covered for this branch - Once above mentioned issues get fixed let me know @npeltier @3ch023 @yesil

Milo only consumes WCS prod. However with this PR, on local and stage environments, `allow-override` parameter will be added to `mas-commerce-service` allowing to override the commerce env/landscape via query parameters.

Roycethan · 2024-10-21T18:16:43Z

Reported issues fixed and - regressions covered

Axelcureno and others added 27 commits September 16, 2024 17:02

initial commit

29a16b0

slots, styles and basic layout

cd361b9

more changes

d47b6ff

Update merch-card.ccd-suggested.test.html

1894a7a

Update merch-twp-d2p.test.html

eb40fb4

unit tests, strip slot and more

329be3a

fixed review comments

0b1aef8

review comments fixed

e20123a

Merge branch 'stage' into MWPW-158473

f545c53

Merge branch 'stage' into MWPW-158473

254ef7a

RTL background image support

937713f

Initial @adobecom/mas-js version.

59285a0

a

1dd8ea9

Update merch-card.ccd-suggested.test.html

5e5924d

a

eb156cc

dark mode, unit tests and rtl

5302051

simplified styles

bdf845d

Merge branch 'stage' into MWPW-158473

d2b50c4

updated deps

bdd6322

Update variant-layout.js

d8eb9e9

dark theme ut

d1ba823

Update merch-card.ccd-suggested.test.html.js

fed743d

Update variant-layout.js

48d5fc4

card mapping

a7ac03d

MWPW-158200: Merch card Dark mode

0aa1042

updated deps

521ac4e

npeltier requested a review from a team as a code owner October 4, 2024 09:21

npeltier requested review from yesil and removed request for a team October 4, 2024 09:21

npeltier added 7 commits October 16, 2024 15:14

MWPW-159257 various fixes

5d1e6a1

rename promise -> readyPromise

bcb7502

not documenting it as i'm not sure it's needed by consumers

MWPW-159257 enhance coverage

96ded50

MWPW-159257 yet more coverage

d462fb2

MWPW-159257 final?

52bd80f

MWPW-159257 non reported cov issues

983cf7c

prob. some issue in cov reporting as its used several times

b71a421

Axelcureno approved these changes Oct 16, 2024

View reviewed changes

yesil reviewed Oct 17, 2024

View reviewed changes

3ch023 added the high priority Why is this a high priority? Blocker? Critical? Dependency? label Oct 17, 2024

npeltier added 2 commits October 17, 2024 22:18

MWPW-159257 review fixes

a7d2b54

yet some over c8 ignore

061805c

narcis-radu reviewed Oct 18, 2024

View reviewed changes

yesil approved these changes Oct 18, 2024

View reviewed changes

npeltier added 2 commits October 18, 2024 10:27

MWPW-159257 removing it.only & built docs

dd21fdb

MWPW-159257 unwrap single condition block

4262369

aem-code-sync bot deployed to MWPW-159257 October 18, 2024 14:00 View deployment

yesil added 4 commits October 21, 2024 14:42

MWPW-159257: [M@S] Support override env/landscape

d1b2124

Milo only consumes WCS prod. However with this PR, on local and stage environments, `allow-override` parameter will be added to `mas-commerce-service` allowing to override the commerce env/landscape via query parameters.

Merge branch 'stage' of github.com:adobecom/milo into MWPW-159257

b41aa54

PR feedback

4523f0e

reduce codecov requirement

d0c0d92

Roycethan added verified PR has been E2E tested by a reviewer Ready for Stage labels Oct 21, 2024

milo-pr-merge bot merged commit 9bd25e6 into adobecom:stage Oct 22, 2024
22 checks passed

milo-pr-merge bot mentioned this pull request Oct 22, 2024

[Release] Stage to Main #3072

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MWPW-159257 refactor mas enablement #3012

MWPW-159257 refactor mas enablement #3012

npeltier commented Oct 4, 2024

yesil commented Oct 16, 2024

yesil Oct 17, 2024

yesil Oct 17, 2024

yesil Oct 17, 2024

yesil Oct 17, 2024

npeltier Oct 17, 2024

yesil Oct 17, 2024

npeltier Oct 17, 2024

narcis-radu Oct 18, 2024

npeltier Oct 18, 2024

narcis-radu Oct 18, 2024

narcis-radu Oct 18, 2024

npeltier Oct 18, 2024

afmicka commented Oct 18, 2024

Roycethan commented Oct 18, 2024

Roycethan commented Oct 21, 2024

MWPW-159257 refactor mas enablement #3012

MWPW-159257 refactor mas enablement #3012

Conversation

npeltier commented Oct 4, 2024

yesil commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afmicka commented Oct 18, 2024

Roycethan commented Oct 18, 2024

Roycethan commented Oct 21, 2024