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.

Sign up for GitHub

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-147206 making promo schedule geo aware #2438

Merged
merged 7 commits into from
Jun 17, 2024
Merged

MWPW-147206 making promo schedule geo aware #2438

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cd09233
MWPW-147206 making promo schedule geo aware
npeltier Jun 5, 2024
df29e84
lint & ut fix
npeltier Jun 7, 2024
0c52771
MWPW-147206 removing non matching manifests
npeltier Jun 7, 2024
70ba20d
lint fix
npeltier Jun 7, 2024
e0de285
review fixes
npeltier Jun 7, 2024
5c15e53
MWPW-147206 rares changes
npeltier Jun 13, 2024
47c7384
Merge branch 'stage' of github.com:adobecom/milo into MWPW-147206
npeltier Jun 17, 2024
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
42 changes: 38 additions & 4 deletions libs/features/personalization/promo-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,70 @@
import { getMetadata } from '../../utils/utils.js';
import { getMetadata, getConfig } from '../../utils/utils.js';

const GMTStringToLocalDate = (gmtString) => new Date(`${gmtString}+00:00`);

const APAC = ['au', 'cn', 'hk_en', 'hk_zh', 'id_en', 'id_id', 'in', 'in_hi', 'kr', 'my_en', 'my_ms', 'nz', 'ph_en', 'ph_fil', 'sg', 'th_en', 'th_th', 'tw', 'vn_en', 'vn_vi'];
const EMEA = ['ae_en', 'ae_ar', 'africa', 'at', 'be_en', 'be_fr', 'be_nl', 'bg', 'ch_de', 'ch_fr', 'ch_it', 'cis_en', 'cis_ru', 'cz', 'de', 'dk', 'ee', 'eg_ar', 'eg_en', 'es', 'fi', 'fr', 'gr_el', 'gr_en', 'hu', 'ie', 'il_en', 'il_he', 'iq', 'is', 'it', 'kw_ar', 'kw_en', 'lt', 'lu_de', 'lu_en', 'lu_fr', 'lv', 'mena_ar', 'mena_en', 'ng', 'nl', 'no', 'pl', 'pt', 'qa_ar', 'qa_en', 'ro', 'ru', 'sa_en', 'sa_ar', 'se', 'si', 'sk', 'tr', 'ua', 'uk', 'za'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to get these mappings from a json? I'm thinking about geo-expansion and the need to update the list of supported languages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i understand but what is the added value to separate it beside clarity for developers? Those lists are region mapping for promotion country, for now not used elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

and the need to update the list of supported languages.

for this use case its probably better to keep it in code similar to the list of locales in scripts.js.
hopefully geoexpansion PR will include changes to both lists at the same time, worst case we can always update it later.
json will not make it much easier

const AMERICAS = ['us', 'ar', 'br', 'ca', 'ca_fr', 'cl', 'co', 'cr', 'ec', 'gt', 'la', 'mx', 'pe', 'pr'];
const JP = ['jp'];
const REGIONS = { APAC, EMEA, AMERICAS, JP };

const getLocaleCode = () => {
const getLocale = () => getConfig()?.locale?.prefix?.substring(1) || 'us';
return (getLocale()?.split('-')?.pop() || 'us').toLowerCase();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be a function? Looks like it could be a simple constant.

};

const localeCode = getLocaleCode();
const regionCode = Object.keys(REGIONS)
.find((r) => REGIONS[r]?.includes(localeCode))?.toLowerCase() || null;

export const isDisabled = (event, searchParams) => {
if (!event) return false;
if (!event?.locales?.includes(localeCode)) return true;
const currentDate = searchParams?.get('instant') ? new Date(searchParams.get('instant')) : new Date();
if ((!event.start && event.end) || (!event.end && event.start)) return true;
return Boolean(event.start && event.end
&& (currentDate < event.start || currentDate > event.end));
};

export default function getPromoManifests(manifestNames, searchParams) {
const getRegionalPromoManifests = (manifestNames, region, searchParams) => {
const attachedManifests = manifestNames
? manifestNames.split(',')?.map((manifest) => manifest?.trim())
: [];
const schedule = getMetadata('schedule');

const schedule = getMetadata(region ? `${region}_schedule` : 'schedule');
if (!schedule) {
return [];
}
return schedule.split(',')
.map((manifest) => {
const [name, start, end, manifestPath] = manifest.trim().split('|').map((s) => s.trim());
const [name, start, end, manifestPath, locales] = manifest.trim().split('|').map((s) => s.trim());
if (attachedManifests.includes(name)) {
const event = {
name,
start: GMTStringToLocalDate(start),
end: GMTStringToLocalDate(end),
};
if (locales) {
event.locales = locales?.split(';').map((locale) => locale.trim());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we should check here if current page is not in target locales.
and if its not, it should not have an event.
currently you create a disabled event, which means it would show on all unaffected GEOs in the MEP Preview.
instead we could just have:

const localesList = locales?.split(';').map((locale) => locale.trim().toLowerCase());
if (!localesList.includes(localeCode)) return true;

(I added toLowerCase, it doesn't hurt)
I created https://mwpw-147206--milo--npeltier.hlx.page/de/npeltier/geopromos/promo to demonstrate what i mean, but it doesn't show preview because of another problem
jp promotions file have wrong column names (emea) and rewrite metadata for emea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think i do agree with you. Fix the file you created and this is weird to see on de page
image
i fixed it, please have another look @3ch023

const disabled = isDisabled(event, searchParams);
return { manifestPath, disabled, event };
}
return null;
})
.filter((manifest) => manifest != null);
};

export default function getPromoManifests(manifestNames, searchParams) {
const promoManifests = getRegionalPromoManifests(
manifestNames[`${regionCode}_manifestnames`],
Copy link
Contributor

@3ch023 3ch023 Jun 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

maybe we want to wrap the first method call in if (region !== null) to avoid calling getRegionalPromoManifests twice with null value if geo is not listed in the regions map yet

regionCode,
searchParams,
);
const globalPromoManifests = getRegionalPromoManifests(
manifestNames.manifestnames,
null,
searchParams,
);
return [...promoManifests, ...globalPromoManifests];
}
34 changes: 30 additions & 4 deletions libs/utils/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ export const MILO_EVENTS = { DEFERRED: 'milo:deferred' };
const LANGSTORE = 'langstore';
const PAGE_URL = new URL(window.location.href);

const PROMO_PARAM = 'promo';

function getEnv(conf) {
const { host } = window.location;
const query = PAGE_URL.searchParams.get('env');
Expand Down Expand Up @@ -873,12 +875,36 @@ const getMepValue = (val) => {
return finalVal;
};

const getMdValue = (key) => {
const value = getMetadata(key);
if (value) {
return getMepValue(value);
}
return false;
};

const getPromoMepEnablement = () => {
const mds = ['apac_manifestnames', 'emea_manifestnames', 'americas_manifestnames', 'jp_manifestnames', 'manifestnames'];
const mdObject = mds.reduce((obj, key) => {
const val = getMdValue(key);
if (val) {
obj[key] = getMdValue(key);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
obj[key] = getMdValue(key);
obj[key] = val;

}
return obj;
}, {});
if (Object.keys(mdObject).length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (Object.keys(mdObject).length > 0) {
if (Object.keys(mdObject).length) {

return mdObject;
}
return false;
};

export const getMepEnablement = (mdKey, paramKey = false) => {
const paramValue = PAGE_URL.searchParams.get(paramKey || mdKey);
if (paramValue) return getMepValue(paramValue);
const mdValue = getMetadata(mdKey);
if (!mdValue) return false;
return getMepValue(mdValue);
if (PROMO_PARAM === paramKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (PROMO_PARAM === paramKey) {
if (PROMO_PARAM === paramKey) return getPromoMepEnablement();

return getPromoMepEnablement();
}
return getMdValue(mdKey);
};

export const combineMepSources = async (persEnabled, promoEnabled, mepParam) => {
Expand Down Expand Up @@ -922,7 +948,7 @@ async function checkForPageMods() {
const { mep: mepParam } = Object.fromEntries(PAGE_URL.searchParams);
if (mepParam === 'off') return;
const persEnabled = getMepEnablement('personalization');
const promoEnabled = getMepEnablement('manifestnames', 'promo');
const promoEnabled = getMepEnablement('manifestnames', PROMO_PARAM);
const targetEnabled = getMepEnablement('target');
const mepEnabled = persEnabled || targetEnabled || promoEnabled || mepParam;
if (!mepEnabled) return;
Expand Down
6 changes: 5 additions & 1 deletion test/features/personalization/mocks/head-schedule.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
<meta name="manifestnames" content="pre-black-friday-global,black-friday-global,cyber-monday">
<meta name="manifestnames" content="pre-black-friday-global,black-friday-global,cyber-monday,bf-us">
<meta name="emea_manifestnames" content="bf-de">
<meta name="americas_manifestnames" content="bf-us">
<meta name="schedule" content="pre-black-friday-global | 2000-11-01T00:00:00 | 2300-12-15T00:00:00 | https://main--milo--adobecom.hlx.page/promos/2023/black-friday/pre-black-friday/manifest-global.json, black-friday-global | 2000-12-15T00:00:00 | 2000-12-31T00:00:00 | https://main--milo--adobecom.hlx.page/promos/2023/black-friday/black-friday/manifest-global.json">
<meta name="emea_schedule" content="bf-de | 2000-11-01T00:00:00 | 2300-12-15T00:00:00 | https://main--milo--adobecom.hlx.page/promos/2023/black-friday/bf-de.json | de;ch, black-friday-de | 2000-12-15T00:00:00 | 2300-12-31T00:00:00 | https://main--milo--adobecom.hlx.page/promos/2023/black-friday/black-friday/manifest-de.json">
<meta name="americas_schedule" content="bf-us | 2000-11-01T00:00:00 | 2300-12-15T00:00:00 | https://main--milo--adobecom.hlx.page/promos/2023/black-friday/bf-us.json | ca;us, bf-ca | 2000-11-01T00:00:00 | 2300-12-15T00:00:00 | https://main--milo--adobecom.hlx.page/promos/2023/black-friday/bf-ca.json | ca,black-friday-us | 2000-12-15T00:00:00 | 2300-12-31T00:00:00 | https://main--milo--adobecom.hlx.page/promos/2023/black-friday/black-friday/manifest-us.json">
77 changes: 55 additions & 22 deletions test/features/personalization/promo-utils.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,24 @@
expect(isDisabled(event, new URLSearchParams())).to.be.false;
});

it('should be enabled if current time is within range, and current locale corresponds to event locale', () => {
const event = {
start: new Date('2000-11-01T00:00:00'),
end: new Date('2300-11-01T00:00:00'),
locales: ['us'],
};
expect(isDisabled(event, new URLSearchParams())).to.be.false;
});

it('should be disabled if current time is within range, but current locale does not correspond to event locale', () => {
const event = {
start: new Date('2000-11-01T00:00:00'),
end: new Date('2300-11-01T00:00:00'),
locales: ['de'],
};
expect(isDisabled(event, new URLSearchParams())).to.be.true;
});

it('should be enabled if no event exist', () => {
expect(isDisabled(null, new URLSearchParams())).to.be.false;
});
Expand Down Expand Up @@ -49,31 +67,46 @@
});

describe('getPromoManifests', () => {
const expectedManifests = [
{
manifestPath: 'https://main--milo--adobecom.hlx.page/promos/2023/black-friday/pre-black-friday/manifest-global.json',
disabled: false,
event: {
name: 'pre-black-friday-global',
start: new Date('2000-11-01T00:00:00.000Z'),
end: new Date('2300-12-15T00:00:00.000Z'),
it('should return an array of promo manifests', async () => {
const expectedManifests = [
{
manifestPath: 'https://main--milo--adobecom.hlx.page/promos/2023/black-friday/bf-us.json',
disabled: false,
event: {
name: 'bf-us',
start: new Date('2000-11-01T00:00:00.000Z'),
end: new Date('2300-12-15T00:00:00.000Z'),
locales: [
'ca',
'us',
],
},
},
},
{
manifestPath: 'https://main--milo--adobecom.hlx.page/promos/2023/black-friday/black-friday/manifest-global.json',
disabled: true,
event: {
name: 'black-friday-global',
start: new Date('2000-12-15T00:00:00.000Z'),
end: new Date('2000-12-31T00:00:00.000Z'),
{
manifestPath: 'https://main--milo--adobecom.hlx.page/promos/2023/black-friday/pre-black-friday/manifest-global.json',
disabled: false,
event: {
name: 'pre-black-friday-global',
start: new Date('2000-11-01T00:00:00.000Z'),
end: new Date('2300-12-15T00:00:00.000Z'),
},
},
},
];

it('should return an array of promo manifests', async () => {
{
manifestPath: 'https://main--milo--adobecom.hlx.page/promos/2023/black-friday/black-friday/manifest-global.json',
disabled: true,
event: {
name: 'black-friday-global',
start: new Date('2000-12-15T00:00:00.000Z'),
end: new Date('2000-12-31T00:00:00.000Z'),
},
},
];
document.head.innerHTML = await readFile({ path: './mocks/head-schedule.html' });
const manifestNames = 'pre-black-friday-global,black-friday-global,cyber-monday';
expect(getPromoManifests(manifestNames, new URLSearchParams())).to.deep.eq(expectedManifests);
const manifestnames = 'pre-black-friday-global,black-friday-global,cyber-monday';
const emea = 'bf-de';
const americas = 'bf-us';
expect(getPromoManifests({ manifestnames, emea_manifestnames: emea, americas_manifestnames: americas }, new URLSearchParams()))

Check failure on line 108 in test/features/personalization/promo-utils.test.js

View workflow job for this annotation

GitHub Actions / Running eslint 

[eslint] reported by reviewdog 🐶
This line has a length of 131. Maximum allowed is 100.

Raw Output:
{"ruleId":"max-len","severity":2,"message":"This line has a length of 131. Maximum allowed is 100.","line":108,"column":1,"nodeType":"Program","messageId":"max","endLine":108,"endColumn":132}
.to.deep.eq(expectedManifests);
});

it('should return an empty array if no schedule', async () => {
Expand Down
4 changes: 4 additions & 0 deletions test/utils/mocks/mep/head-promo.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
<meta name="personalization" content="https://main--milo--adobecom.hlx.page/products/special-offers-manifest.json">
<meta name="manifestnames" content="pre-black-friday-global,black-friday-global">
<meta name="americas_manifestnames" content="us-special-promo">
<meta name="apac_manifestnames" content="kr-special-promo">
<meta name="schedule" content="pre-black-friday-global | 2023-11-01T00:00:00 | 2023-12-15T00:00:00 | /pre-black-friday.json, black-friday-global | 2023-12-15T00:00:00 | 2023-12-31T00:00:00 | /black-friday.json">
<meta name="americas_schedule" content="us-special-promo | 2023-11-01T00:00:00 | 2023-12-15T00:00:00 | /us-special-promo.json | us;ca">
<meta name="apac_schedule" content="kr-special-promo | 2023-11-01T00:00:00 | 2023-12-15T00:00:00 | /us-special-promo.json | kr">
<title>Document Title</title>
<link rel="icon" href="data:,">
+
10 changes: 7 additions & 3 deletions test/utils/utils-mep.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
});
it('combines promos and personalization', async () => {
document.head.innerHTML = await readFile({ path: './mocks/mep/head-promo.html' });
const manifests = await combineMepSources('/pers/manifest.json', 'pre-black-friday-global,black-friday-global', undefined);
const manifests = await combineMepSources('/pers/manifest.json', { manifestnames: 'pre-black-friday-global,black-friday-global' }, undefined);
expect(manifests.length).to.equal(3);
expect(manifests[0].manifestPath).to.equal('/pers/manifest.json');
expect(manifests[1].manifestPath).to.equal('/pre-black-friday.json');
Expand All @@ -20,7 +20,7 @@
document.head.innerHTML = await readFile({ path: './mocks/mep/head-promo.html' });
const manifests = await combineMepSources(
'/pers/manifest.json',
'pre-black-friday-global,black-friday-global',
{ manifestnames: 'pre-black-friday-global,black-friday-global' },
'/pers/manifest.json--var1---/mep-param/manifest1.json--all---/mep-param/manifest2.json--all',
);
expect(manifests.length).to.equal(5);
Expand Down Expand Up @@ -52,7 +52,11 @@
const persEnabled = getMepEnablement('personalization');
const promoEnabled = getMepEnablement('manifestnames', 'promo');
const targetEnabled = getMepEnablement('target');
expect(promoEnabled).to.equal('pre-black-friday-global,black-friday-global');
expect(promoEnabled).to.deep.equal({
'apac_manifestnames': 'kr-special-promo',

Check failure on line 56 in test/utils/utils-mep.test.js

View workflow job for this annotation

GitHub Actions / Running eslint 

[eslint] reported by reviewdog 🐶
Unnecessarily quoted property 'apac_manifestnames' found.

Raw Output:
{"ruleId":"quote-props","severity":2,"message":"Unnecessarily quoted property 'apac_manifestnames' found.","line":56,"column":9,"nodeType":"Property","messageId":"unnecessarilyQuotedProperty","endLine":56,"endColumn":49,"fix":{"range":[3132,3152],"text":"apac_manifestnames"}}
'americas_manifestnames': 'us-special-promo',

Check failure on line 57 in test/utils/utils-mep.test.js

View workflow job for this annotation

GitHub Actions / Running eslint 

[eslint] reported by reviewdog 🐶
Unnecessarily quoted property 'americas_manifestnames' found.

Raw Output:
{"ruleId":"quote-props","severity":2,"message":"Unnecessarily quoted property 'americas_manifestnames' found.","line":57,"column":9,"nodeType":"Property","messageId":"unnecessarilyQuotedProperty","endLine":57,"endColumn":53,"fix":{"range":[3182,3206],"text":"americas_manifestnames"}}
manifestnames: 'pre-black-friday-global,black-friday-global',
});
expect(persEnabled).to.equal('https://main--milo--adobecom.hlx.page/products/special-offers-manifest.json');
expect(targetEnabled).to.equal(false);
});
Expand Down
Loading