MWPW-202563: Validate promo variation geo tags on save in Card Editor - #1182
MWPW-202563: Validate promo variation geo tags on save in Card Editor#1182Axelcureno wants to merge 15 commits into
Conversation
Extends editable geo tags on promo variations with save-time validation against sibling conflicts, the single no-geo-variation rule, and the promotion project's configured geos, blocking the save with a toast on conflict. Also fixes getPromotionTagFromFragment/fragmentIsPromoVariation, which preferred a fragment's tags field over its metadata tags array even when the field held unrelated tags (offer_type, plan_type) - this silently broke promotion-project matching for real Promotion/Card fragments.
Reuse getPromotionTagFromFragment in fragmentIsPromoVariation instead of re-inlining the same fallback expression, and extract getAttachedFragmentPaths to mirror the existing getProjectGeos accessor pattern.
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
|
SNOW Change Request Transaction ID [STUDIO]: 0000019c1ef37582-1fc7dd34 |
|
SNOW Change Request Transaction ID [IO]: 0000019c1ef37582-1fc7dd8b |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1182 +/- ##
==========================================
+ Coverage 90.74% 90.80% +0.06%
==========================================
Files 319 319
Lines 101832 101922 +90
==========================================
+ Hits 92410 92555 +145
+ Misses 9422 9367 -55
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Andrei4226
left a comment
Andrei4226
left a comment
There was a problem hiding this comment.
I noticed that when I open a promo variation, it no longer opens in Fragments > Editor, but in Promotions > Edit promotion project > Edit promotion variation (which already allows editing geos from the Geos tags field).
Based on the requirements, I think we want to keep the existing route when opening a promo variation (meaning Fragments > Editor) and create a connection between the geos from the promo project and the Geos tags field from the card editor, so we can discuss about it.
One more thing: now, if I open a promo variation and press the back button once, nothing happens. If I press it one more time, I get redirected to the home page: https://mwpw-202563--mas--adobecom.aem.live/
|
@Andrei4226 Good catch, but that routing/breadcrumb behavior is pre-existing (not something this PR touches), and it's more of a product decision than a bug, so I think that's one for Lucy/Frauke to weigh in on rather than something to fix here. This ticket's scope was specifically to stop saves that create conflicting geo tags across promo variations, that's built and tested. Happy to file a follow-up ticket for the routing/back-button behavior if you want to track it separately. |
seanchoi0
left a comment
seanchoi0
left a comment
There was a problem hiding this comment.
Nice work. The getPromotionTagFromFragment fix is the right call — preferring the tags metadata array over the field value was silently breaking promotion-project matching for real Card fragments. Good catch, and the extracted assertPromoVariationGeoTagsValid is a clean shape for reuse.
Two small things, neither blocking:
- Dead arg:
resolveDefaultFragmentForPromoVariationis called with 4 args but only accepts 3 —attachedFragmentPathsis silently ignored. Either drop the arg or wire it through to avoid the misleading pre-fetch. - Validation reads pre-
prepareVariationForSavepznTags (fragment.getFieldValues) while AEM receivesfragmentToSave— semantically these should match. In practice it won't misfire since promo variation pznTags are always explicitly set, butfragmentToSave.getFieldValues('pznTags')would be more correct.
Worth adding tests for the !promoTagId and !defaultFragment early-exit branches in assertPromoVariationGeoTagsValid too — the happy paths are well covered but those guards are untested.
Approving.
yesil
left a comment
yesil
left a comment
There was a problem hiding this comment.
One thing worth checking (not a blocker): the saveFragment hook validates geos on every promo-variation save, not just when pznTags changed. With pre-existing conflicting sibling data (legacy geo-less/overlapping geos — cf. MWPW-205737), editing an unrelated field like price would block the save. Intended, or gate on pznTags actually changing?
Non-blocking: the !promoTagId and !defaultFragment early-returns are untested, and the "no promotion tag" test trips the path guard, not the tag guard.
|
@Axelcureno can we connect about these changes? I am not able to add geo tag on my promo variation, the menu is empty. Also the existing variation has only one geo tag out of 2 that project has. I should be able to add at least the second one if not any. |
5 participants
Adds save-time validation for a promo variation's geo tags in the Card Editor: blocks the save with an error toast if the tags conflict with a sibling variation, violate the single no-geo-variation rule, or fall outside the promotion project's configured geos.
Also fixes
getPromotionTagFromFragment/fragmentIsPromoVariation, which preferred a fragment'stagsfield over its metadatatagsarray even when the field held unrelated tags (offer_type,plan_type) — this silently broke promotion-project matching for real Promotion/Card fragments, which would have made the project-containment check dead code in production.Resolves https://jira.corp.adobe.com/browse/MWPW-202563
QA Checklist: https://wiki.corp.adobe.com/display/adobedotcom/M@S+Engineering+QA+Use+Cases
Please do the steps below before submitting your PR for a code review or QA
NALA E2E coverage (C2) is deferred: it requires minting a new
mas:promotion/<name>taxonomy tag in Odin, which isn't available through any current Odin MCP tool (only applying existing tags is). Will follow up once that's created via Studio UI.Test URLs: