Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MWPW-161845: basic analytics on mas cards #3206

Open
wants to merge 4 commits into
base: ccd
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libs/deps/mas/commerce.js

Large diffs are not rendered by default.

74 changes: 37 additions & 37 deletions libs/deps/mas/mas.js

Large diffs are not rendered by default.

112 changes: 56 additions & 56 deletions libs/deps/mas/merch-card.js

Large diffs are not rendered by default.

74 changes: 37 additions & 37 deletions libs/features/mas/mas/dist/mas.js

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions libs/features/mas/web-components/src/hydrate.js
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ const DEFAULT_BADGE_COLOR = '#000000';
const DEFAULT_BADGE_BACKGROUND_COLOR = '#F8D904';
const CHECKOUT_LINK_STYLE_PATTERN =
/(accent|primary|secondary)(-(outline|link))?/;
export const ANALYTICS_TAG = 'mas:product_code/';
export const ANALYTICS_LINK_ATTR = 'daa-ll';
export const ANALYTICS_SECTION_ATTR = 'daa-lh';

function processFragment(fragmentData) {
return fragmentData.fields.reduce(
Expand Down Expand Up @@ -222,6 +225,17 @@ export function processCTAs(fragment, merchCard, aemFragmentMapping, variant) {
}
}

export function processAnalytics(fragment, merchCard) {
const { tags } = fragment;
const cardAnalyticsId = tags?.find(tag => tag.startsWith(ANALYTICS_TAG))?.split('/').pop();
if(cardAnalyticsId) {
merchCard.setAttribute(ANALYTICS_SECTION_ATTR, cardAnalyticsId);
merchCard.querySelectorAll(`a[data-analytics-id]`).forEach((link, index) => {
link.setAttribute(ANALYTICS_LINK_ATTR, `${link.dataset.analyticsId}-${index + 1}`);
});
}
}

export async function hydrate(fragmentData, merchCard) {
const fragment = processFragment(fragmentData);
const { variant } = fragment;
Expand All @@ -237,6 +251,7 @@ export async function hydrate(fragmentData, merchCard) {
merchCard.removeAttribute('badge-color');
merchCard.removeAttribute('badge-text');
merchCard.removeAttribute('size');
merchCard.removeAttribute(ANALYTICS_SECTION_ATTR);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, i finally understood that it is here to reflect actual state of the fragment.
However i find it incomplete to have cleanage being done in bulk for every type of processing, with no specification of what is at stake at the beginning of generic hydrate, and have then processAnalytics that is the one amending that attribute (not specific to analytics, same stands true).
I would have prefer specific code to do everything, probably this part

    const { aemFragmentMapping } = merchCard.variantLayout;
    if (!aemFragmentMapping) return;

in between basically implies that we need, for each field, 2 places (preprocess & process ?), may be we need subfile / class that has those 2 methods. May be not for this PR :)

cc @yesil


merchCard.variant = variant;
await merchCard.updateComplete;
Expand All @@ -260,4 +275,5 @@ export async function hydrate(fragmentData, merchCard) {
processPrices(fragment, merchCard, aemFragmentMapping.prices);
processDescription(fragment, merchCard, aemFragmentMapping.description);
processCTAs(fragment, merchCard, aemFragmentMapping, variant);
processAnalytics(fragment, merchCard);
}
3 changes: 2 additions & 1 deletion libs/features/mas/web-components/src/merch-card.js
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ [eslint] reported by reviewdog 🐶
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
EVENT_MAS_ERROR,
} from './constants.js';
import { VariantLayout } from './variants/variant-layout.js';
import { hydrate } from './hydrate.js';
import { hydrate, ANALYTICS_SECTION_ATTR } from './hydrate.js';

export const MERCH_CARD_NODE_NAME = 'MERCH-CARD';
export const MERCH_CARD = 'merch-card';
Expand Down Expand Up @@ -90,6 +90,7 @@ export class MerchCard extends LitElement {
reflect: true,
},
merchOffer: { type: Object },
analyticsId: { type: String, attribute: ANALYTICS_SECTION_ATTR, reflect: true },
};

static styles = [styles, getVariantStyles(), ...sizeStyles()];
Expand Down
57 changes: 57 additions & 0 deletions libs/features/mas/web-components/test/hydrate.test.js
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import {
processBackgroundImage,
processCTAs,
processSubtitle,
processAnalytics,
ANALYTICS_TAG,
ANALYTICS_LINK_ATTR,
ANALYTICS_SECTION_ATTR
} from '../src/hydrate.js';

import { mockFetch } from './mocks/fetch.js';
Expand Down Expand Up @@ -318,3 +322,56 @@ describe('processBackgroundImage', () => {
expect(merchCard.outerHTML).to.equal('<div></div>');
});
});

describe('processAnalytics', () => {
let merchCard;

beforeEach(() => {
merchCard = mockMerchCard();
//merchCard.innerHTML = '<a href="/see-terms.html" data-analytics-id="see-terms">See terms</a>'
});

afterEach(() => {
sinon.restore();
});

it('should not set analytics attributes if no fragment.tags', () => {
const fragment = {};
processAnalytics(fragment, merchCard);
expect(merchCard.hasAttribute(ANALYTICS_SECTION_ATTR)).to.be.false;
expect(merchCard.querySelectorAll(`a[${ANALYTICS_LINK_ATTR}]`).length).to.equal(0);
});

it(`should not set analytics attributes when no tags start with ${ANALYTICS_TAG}`, () => {
const fragment = { tags: ['mas:term/montly'] };
processAnalytics(fragment, merchCard);
expect(merchCard.hasAttribute(ANALYTICS_SECTION_ATTR)).to.be.false;
expect(merchCard.querySelectorAll(`a[${ANALYTICS_LINK_ATTR}]`).length).to.equal(0);
});

it('should set analytics-section attribute on merchCard', () => {
const fragment = { tags: ['mas:product_code/phsp'] };
processAnalytics(fragment, merchCard);
expect(merchCard.getAttribute(ANALYTICS_SECTION_ATTR)).to.equal('phsp');
});

it('should set analytics-link attributes on links inside merchCard', () => {
const fragment = { tags: ['mas:term/montly', 'mas:product_code/ccsn'] };

const seeTerms = document.createElement('a');
seeTerms.setAttribute('data-analytics-id', 'see-terms');
const buyNow = document.createElement('a');
buyNow.setAttribute('data-analytics-id', 'buy-now');
const noAnalytics = document.createElement('a');
merchCard.appendChild(seeTerms);
merchCard.appendChild(buyNow);
merchCard.appendChild(noAnalytics);

processAnalytics(fragment, merchCard);
expect(merchCard.getAttribute(ANALYTICS_SECTION_ATTR)).to.equal('ccsn');
expect(seeTerms.getAttribute(ANALYTICS_LINK_ATTR)).to.equal('see-terms-1');
expect(buyNow.getAttribute(ANALYTICS_LINK_ATTR)).to.equal('buy-now-2');
// should handle only links with data-analytics-id attribute
expect(merchCard.querySelectorAll(`a[${ANALYTICS_LINK_ATTR}]`).length).to.equal(2);
});
});