Skip to content

MWPW-192308: Add ability to create a grouped variation of a collection#812

Open
Andrei4226 wants to merge 5 commits intomainfrom
MWPW-192308
Open

MWPW-192308: Add ability to create a grouped variation of a collection#812
Andrei4226 wants to merge 5 commits intomainfrom
MWPW-192308

Conversation

@Andrei4226
Copy link
Copy Markdown
Contributor

@Andrei4226 Andrei4226 commented Apr 30, 2026

Resolves https://jira.corp.adobe.com/browse/MWPW-192308
QA Checklist: https://wiki.corp.adobe.com/display/adobedotcom/M@S+Engineering+QA+Use+Cases

Adds support for grouped variations on collections in MAS Studio.
The collection CF model has no pznTags field in its schema (and no OSI).
For collections only, tags are now stored on metadata.tags instead of fields. A synthetic PAC (merch-card-collection / COLLECTION_GROUPED_VARIATION_PAC) is used to construct the target folder path (en_US/merch-card-collection/pzn/...) since collections have no offer selector.
Explicit COLLECTION_MODEL_PATH guards were added to handle collections separately from cards:

  • mas-repository.js — skips pznTags in fields, stores tags on metadata.tags (collections only)
  • mas-variation-dialog.js — returns synthetic PAC instead of calling WCS (collections only)
  • mas-fragment-editor.js / mas-fragment-variations.js — falls back to fragment.tags when fields.pznTags is empty (collections only)
  • customize.js / odinSchemaTransform.js — personalization matching reads tags from metadata.tags for collections, so grouped variations are correctly selected.

URL differs from cards only in the tags param:
mas:studio/content-type/merch-card-collection
vs
mas:studio/content-type/merch-card

Grouped variations of a collection: https://mwpw-192308--mas--adobecom.aem.live/studio.html#fragmentId=4bea6742-f4d5-4953-ac28-92ac1c42dc83&page=fragment-editor&path=sandbox&tags=mas%3Astudio%2Fcontent-type%2Fmerch-card-collection

Please do the steps below before submitting your PR for a code review or QA

  • C1. Cover code with Unit Tests
  • C2. Add a Nala test (double check with #fishbags if nala test is needed)
  • C3. Verify all Checks are green (unit tests, nala tests)
  • C4. PR description contains working Test Page link where the feature can be tested
  • C5: you are ready to do a demo from Test Page in PR (bonus: write a working demo script that you'll use on Thursday, you can eventually put in your PR)
  • C.6 read your Jira one more time to validate that you've addressed all AC's and nothing is missing

🧪 Nala E2E Tests

Nala tests run automatically when you open this PR.

To run Nala tests again:

  1. Add the run nala label to this PR (in the right sidebar)
  2. Tests will run automatically on the current commit
  3. Any future commits will also trigger tests as long as the label remains

To stop automatic Nala tests:

  • Remove the run nala label

Note: Tests only run on commits if the run nala label is present. Add the label whenever you need tests to run on new changes.

Test URLs:

Milo Page
https://main--cc--adobecom.aem.page/eg_en/drafts/mariia/coll?maslibs=MWPW-192308
https://main--cc--adobecom.aem.live/eg_en/drafts/mariia/coll?mas-io-url=https://14257-merchatscale-qa.adobeioruntime.net/api/v1/web/MerchAtScale

@Andrei4226 Andrei4226 self-assigned this Apr 30, 2026
@aem-code-sync
Copy link
Copy Markdown

aem-code-sync Bot commented Apr 30, 2026

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.47%. Comparing base (6431509) to head (fca7e5b).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #812      +/-   ##
==========================================
+ Coverage   87.45%   87.47%   +0.01%     
==========================================
  Files         214      214              
  Lines       63670    63702      +32     
==========================================
+ Hits        55682    55722      +40     
+ Misses       7988     7980       -8     
Files with missing lines Coverage Δ
studio/src/mas-fragment-editor.js 89.52% <100.00%> (+0.03%) ⬆️
studio/src/mas-fragment-variations.js 73.13% <100.00%> (+0.99%) ⬆️
studio/src/mas-repository.js 64.32% <100.00%> (+0.12%) ⬆️
studio/src/mas-variation-dialog.js 94.64% <100.00%> (+0.09%) ⬆️

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6431509...fca7e5b. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Andrei4226 Andrei4226 changed the title MWPW-192308: Add abillity to create a grouped variation of a collection MWPW-192308: Add ability to create a grouped variation of a collection Apr 30, 2026
Copy link
Copy Markdown
Contributor

@yesil yesil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments.


function pznTagIdsForMatching(variation) {
const fromField = variation.fields?.pznTags;
if (fromField != null && fromField !== '') {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No unit tests for pznTagIdsForMatching — the fromField !== '' guard, empty-array path, and string-vs-object tag fallback are all worth covering.

const REFERENCE_FIELDS = [...CF_REFERENCE_FIELDS, 'tags'];
const COLLECTION_MODEL_PATH = '/conf/mas/settings/dam/cfm/models/collection';

function collectPathToIdMap(references, map = {}) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COLLECTION_MODEL_PATH is already exported from studio/src/constants.js. Keeping two copies in sync means a path change needs two edits.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I duplicated the constant because io/www cannot import studio/src/constants.js, when running tests, that relative import is resolved incorrectly by the dev server, causing test failures.

const fields = transformFields(ref.fields, pathToIdMap);
const isCollection = ref.model?.path === COLLECTION_MODEL_PATH;
const value = {
name: ref.name,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isCollection tag-spreading branch in transformReferences has no unit tests.

if (isCollection) {
const mergedTags = [...(parentFragment.tags || []), ...(pznTags || [])];
if (mergedTags.length) {
await this.aem.sites.cf.fragments.copyFragmentTags(newFragment, mergedTags);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentFragment.tags entries are objects ({ id, path, ... }) while pznTags are plain strings — copyFragmentTags receives a heterogeneous array. Worth verifying it handles this mix, or normalizing to one type before merging.

return tags.join(',');
const fromField = pznTagsField?.values || [];
if (fromField.length) return fromField.join(',');
const seen = new Set();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same mas:locale//mas:pzn/ metadata fallback pattern is duplicated in mas-fragment-editor.js:displayGroupedVariationInfo. A shared helper would prevent drift.

Copy link
Copy Markdown
Contributor

@3ch023 3ch023 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return matchedTokens * 100 + (regionMatch ? 20 : 0) + (countryMatch ? 10 : 0);
}

function pznTagIdsForMatching(variation) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about it, is it needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pznTagIdsForMatching function is meant to extract tags from a variation
For cards, fields.pznTags is populated, so the new variation.tags fallback is unused
However, for collections, fields.pznTags doesn’t exist, so the function falls back to variation.tags
Should we need to have pznTags for grouped variations in collections (just like we do for cards), or should we want to using variation.tags?

Copy link
Copy Markdown
Contributor

@3ch023 3ch023 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pznTagIdsForMatching likely doesn't do any impact

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants