MWPW-201011: Geo Promo Variations - Follow up#1069
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1069 +/- ##
==========================================
+ Coverage 89.85% 89.90% +0.04%
==========================================
Files 298 299 +1
Lines 94917 95011 +94
==========================================
+ Hits 85287 85416 +129
+ Misses 9630 9595 -35
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
| * reactive property declaration and initial value. | ||
| * @param {typeof import('lit').LitElement} Base | ||
| */ | ||
| export const SearchableListMixin = (Base) => |
There was a problem hiding this comment.
Do we need a mixin? It looks to me we could have just plain utility functions, and pass this.searchQuery as a param to filterBySearchQuery. I think it will decrease complexity (debugging mixins is harder than plain functions)
| if (!variation?.id) break; | ||
| candidates.push({ path: targetPath, index }); | ||
| } | ||
| const variations = await Promise.all(candidates.map(({ path }) => getFragmentByPathOrNull(aem.sites.cf.fragments, path))); |
There was a problem hiding this comment.
Here we will be sending up to 50 parallel requests for every variation - can we consider sending one request to get all fragments in the folder, and then filter out variations? Or some other optimization
Resolves https://jira.corp.adobe.com/browse/MWPW-201011
QA Checklist: https://wiki.corp.adobe.com/display/adobedotcom/M@S+Engineering+QA+Use+Cases
Bug fixes — geo-scoped promo variations:
probePromoVariationsForFragmentto probe all variation indices instead of stopping at the first missing one, previously, deleting one variation could make higher-indexed sibling variations "disappear" from Studio's promo-variation list, even though they still existed in AEM.getNextAvailablePromoVariationIndexto fill index gaps left by individual deletions, instead of always assuming the next index is existingCount + 1.createPromoVariationto force-refresh the promotions store before reading attached fragment paths, so geo-collision checks don't silently no-op when the store hasn't loaded yet.resolveDefaultFragmentForPromoVariationto correctly prefer the non-suffixed candidate path over a coincidentally-named suffixed fragment.deleteAttachedPromoVariationsto delete all variations concurrently (bounded) instead of sequentially, and to throw if any deletion failed instead of silently logging and continuing.Code quality (review follow-ups):
geoMatchScorehelper, used by both the personalization (pzn) and promo-variation geo matching logic incustomize.js, removing duplicated scoring formulas.MAX_PROMO_VARIATIONS_PER_FRAGMENTbetween the Studio and io/www runtimes (separate deployables, no shared import path).search + select-all + indeterminate checkbox" list UI logic shared bymas-promo-variation-geos.jsandmas-translation-languages.jsinto a new shared module (studio/src/common/utils/selectable-list.js), with its own dedicated unit tests.displayPromoVariationGeos()method converted to a getter (promoVariationGeosTemplate), matching the siblingpromoVariationGeoTagsTemplategetter pattern.Test hygiene:
loadPromotionsmock on the repository stub).View promo variation" UI link.Nala test:
NalaEvergreenes_ARanddeandes_ARwas deleteddepromo variation: https://mas.adobe.com/studio.html#fragmentId=941b6280-0835-4b62-a8dd-bea443574264&page=fragment-editor&path=nala&promotionId=d966e2db-f2fa-4eb7-bece-a834e6c96e7eplans-collection.html?country=DE(correct variation-id, card visible)https://mas.adobe.com/web-components/docs/plans-collection.html?country=DE&mas.preview=off
https://mas.adobe.com/web-components/docs/plans-collection.html?country=DE&mas.preview=on
If you want to see the
depromo variation in Promotion column for the Photoshop parent fragment, please use the link of branch:https://mwpw-201011--mas--adobecom.aem.live/studio.html#page=content&path=nala&query=9c718c8b-8807-4f2b-b41a-ae7f87d69832
About the idea to combine
promotion-model.jsandpromotion-repository.js:promotion-model.js= pure functions, no state;promotions-repository.js= Store-aware orchestration.aem/fragment.js(a base file, used everywhere) would become dependent on Store plus the entire promotions feature, despite having nothing to do with it — any code using Fragment would risk breaking because of an unrelated feature.Please do the steps below before submitting your PR for a code review or QA
🧪 Nala E2E Tests
Nala tests run automatically when you open this PR.
To run Nala tests again:
run nalalabel to this PR (in the right sidebar)To stop automatic Nala tests:
run nalalabelTest URLs: