MWPW-202435: Tech debt: avoid relocating mas-field CTA content outside the mas-field wrapper - #1185
Conversation
…k unwrap Generalize #stampContext to stamp fragment/variation/mask ids + promo code onto every commerce element mas-field renders (checkout anchors/buttons and inline prices), and make price/checkout option providers dataset-first so a CTA or price relocated out of <mas-field> by Milo's autoblock still resolves. mas is now the single source of truth; Milo only unwraps. MWPW-202435.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1185 +/- ##
==========================================
+ Coverage 90.62% 90.64% +0.01%
==========================================
Files 308 308
Lines 99175 99198 +23
==========================================
+ Hits 89877 89915 +38
+ Misses 9298 9283 -15
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
TsayAdobe
left a comment
There was a problem hiding this comment.
I would use an early return when element is undefined rather than relying on multiple optional chaining checks.
Axelcureno
left a comment
There was a problem hiding this comment.
Approving — nice fix. Went and checked the actual mechanism this depends on: the new global-ish data-promotion-code/fragment-id ownership check in priceOptionsProvider/checkoutOptionsProvider now runs on every price/checkout element, not just mas-field's own, but collectPriceOptions/collectCheckoutOptions always re-merge the element's own dataset after providers run, so it can't clobber merch-card's own promo (including cancel-context) resolution. Also cross-checked the companion milo#6586 diff — it deletes Milo's own re-stamping and fully trusts this, and checkReady() already guarantees #stampContext runs before Milo unwraps, so no race there either.
Small nit: #stampContext's selector is hardcoded (a[data-wcs-osi],button[is="checkout-button"],span[is="inline-price"]) instead of reusing SELECTOR_MAS_CHECKOUT_LINK/SELECTOR_MAS_INLINE_PRICE from constants.js — worth a follow-up so it doesn't drift.
yesil
left a comment
There was a problem hiding this comment.
@joaquinrivero I think the PR title is wrong, but looking at the JIRA, LGTM.
ee645e2 to
00d1df7
Compare
* origin/main: MWPW-205209 Merch Icon URLs Relative (#1188) MWPW-200943: load entitlements from the offer found by ID search (#1156) MWPW-206050: limit executions to only our runners (#1189) MWPW-204765 CTA data-key field in RTE (#1167) MWPW-204645: Reduce promotions editor Odin load for large projects (#1160) # Conflicts: # web-components/dist/commerce.js # web-components/dist/mas-field.js # web-components/dist/mas.js
|
@joaquinrivero Plz see the error in https://github.com/adobecom/mas/actions/runs/33438620327/job/99641065094?pr=1185 |
priceOptionsProvider read masField.settings without optional-chaining masField itself. masField is null for a CTA unwrapped from mas-field (fragment-id marker only, no ancestor), so the check threw instead of falling through.
* upstream/main: MWPW-204248 Wait for mas-commerce-service (#1183) # Conflicts: # web-components/dist/commerce.js # web-components/dist/mas.js
Resolves https://jira.corp.adobe.com/browse/MWPW-202435
mas-field now stamps its context onto the CTA and price directly (dropping the old copy helpers) and holds the CTA until the price resolves
Companion PR (milo): adobecom/milo#6586
Test URLs: