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

Central georouting support #2531

Merged
merged 11 commits into from
Jul 10, 2024
Merged

Central georouting support #2531

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7e281ea
Adding central georouting support from federal repo
bandana147 Jun 28, 2024
642e82e
Unit test case Update
bandana147 Jun 28, 2024
45aa8c0
Unit test case Update
bandana147 Jun 28, 2024
fa6c3d3
Remove console
bandana147 Jun 28, 2024
af705d1
Creating a helper file for feds utils function
bandana147 Jul 9, 2024
1da0a82
Removing blog from default allowed list
bandana147 Jul 9, 2024
44c6549
Updating georouting fetch calls
bandana147 Jul 9, 2024
cd751c7
Lint fix
bandana147 Jul 9, 2024
e2676b8
Test case update
bandana147 Jul 9, 2024
bc7410f
Renaming fedshelper file
bandana147 Jul 10, 2024
bd96b1c
Updating test file
bandana147 Jul 10, 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
33 changes: 1 addition & 32 deletions libs/blocks/global-navigation/utilities/utilities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
getConfig, getMetadata, loadStyle, loadLana, decorateLinks, localizeLink,
getConfig, getMetadata, loadStyle, loadLana, decorateLinks, localizeLink, getFederatedContentRoot,
} from '../../../utils/utils.js';
import { processTrackingLabels } from '../../../martech/attributes.js';
import { replaceText } from '../../../features/placeholders.js';
Expand All @@ -8,16 +8,6 @@ loadLana();

const FEDERAL_PATH_KEY = 'federal';

// TODO when porting this to milo core, we should define this on config level
// and allow consumers to add their own origins
const allowedOrigins = [
'https://www.adobe.com',
'https://business.adobe.com',
'https://blog.adobe.com',
'https://milo.adobe.com',
'https://news.adobe.com',
];

export const selectors = {
globalNav: '.global-navigation',
curtain: '.feds-curtain',
Expand Down Expand Up @@ -84,27 +74,6 @@ export function toFragment(htmlStrings, ...values) {
return fragment;
}

// TODO we might eventually want to move this to the milo core utilities
mokimo marked this conversation as resolved.
Show resolved Hide resolved
let federatedContentRoot;
export const getFederatedContentRoot = () => {
if (federatedContentRoot) return federatedContentRoot;

const { origin } = window.location;

federatedContentRoot = allowedOrigins.some((o) => origin.replace('.stage', '') === o)
? origin
: 'https://www.adobe.com';

if (origin.includes('localhost') || origin.includes('.hlx.')) {
// Akamai as proxy to avoid 401s, given AEM-EDS MS auth cross project limitations
federatedContentRoot = origin.includes('.hlx.live')
? 'https://main--federal--adobecom.hlx.live'
: 'https://www.stage.adobe.com';
}

return federatedContentRoot;
};

// TODO we should match the akamai patterns /locale/federal/ at the start of the url
// and make the check more strict.
export const getFederatedUrl = (url = '') => {
Expand Down
12 changes: 4 additions & 8 deletions libs/features/georouting/georouting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,24 +129,20 @@ async function showModal(details) {
return getModal(null, { class: 'locale-modal', id: 'locale-modal', content: details, closeEvent: 'closeModal' });
}

export default async function loadGeoRouting(config, createTag, getMetadata) {
export default async function loadGeoRouting(config, createTag, getMetadata, geoDetails) {
const { locale } = config;

const urlLocale = locale.prefix.replace('/', '');
const storedInter = sessionStorage.getItem('international') || getCookie('international');
const storedLocale = storedInter === 'us' ? '' : storedInter;

const resp = await fetch(`${config.contentRoot ?? ''}/georouting.json`);
if (!resp.ok) return;
const json = await resp.json();

const urlGeoData = json.data.find((d) => d.prefix === urlLocale);
const urlGeoData = geoDetails.data.find((d) => d.prefix === urlLocale);
Copy link
Contributor

Choose a reason for hiding this comment

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

would be great to use optional chaining to make sure it doesn't throw an error here

if (!urlGeoData) return;

if (storedLocale || storedLocale === '') {
// Show modal when url and cookie disagree
if (urlLocale.split('_')[0] !== storedLocale.split('_')[0]) {
const localeMatches = json.data.filter((d) => d.prefix === storedLocale);
const localeMatches = geoDetails.data.filter((d) => d.prefix === storedLocale);
const details = await getDetails(urlGeoData, localeMatches, config, createTag, getMetadata);
if (details) { await showModal(details); }
}
Expand All @@ -156,7 +152,7 @@ export default async function loadGeoRouting(config, createTag, getMetadata) {
// Show modal when derived countries from url locale and akamai disagree
const akamaiCode = await getAkamaiCode();
if (akamaiCode && !getCodes(urlGeoData).includes(akamaiCode)) {
const localeMatches = getMatches(json.data, akamaiCode);
const localeMatches = getMatches(geoDetails.data, akamaiCode);
const details = await getDetails(urlGeoData, localeMatches, config, createTag, getMetadata);
if (details) { await showModal(details); }
}
Expand Down
19 changes: 14 additions & 5 deletions libs/features/georoutingv2/georoutingv2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getFederatedContentRoot } from '../../utils/utils.js';

let config;
let createTag;
let getMetadata;
Expand Down Expand Up @@ -281,12 +283,19 @@ export default async function loadGeoRouting(
loadBlock = loadBlockFunc;
loadStyle = loadStyleFunc;

const resp = await fetch(`${config.contentRoot ?? ''}/georoutingv2.json`);
let resp = await fetch(`${config.contentRoot ?? ''}/georoutingv2.json`);
if (!resp.ok) {
// eslint-disable-next-line import/no-cycle
const { default: loadGeoRoutingOld } = await import('../georouting/georouting.js');
loadGeoRoutingOld(config, createTag, getMetadata);
return;
resp = await fetch(`${config.contentRoot ?? ''}/georouting.json`);
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're mixing scope here and this will eventually be hard to remove when we fully deprecate the old geo-routing feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To provide backward compatibility, we still need to ensure that fetch(${config.contentRoot ?? ''}/georoutingv2.json) doesn't exist before we go ahead and load the federal georoutingv2 file.
I am not seeing any other way to handle this case without introducing additional configuration from consumer. However, I am open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could always configure a project and say pick up the georouting from federal via metadata.

if (!resp.ok) {
resp = await fetch(`${getFederatedContentRoot()}/federal/georouting/georoutingv2.json`);
if (!resp.ok) return;
} else {
const json = await resp.json();
// eslint-disable-next-line import/no-cycle
const { default: loadGeoRoutingOld } = await import('../georouting/georouting.js');
loadGeoRoutingOld(config, createTag, getMetadata, json);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have the full context of why the old georouting is required, but here a suggestion that would provide a bit more clarity on the load order, avoiding a nested if/else structure:

const urls = [
  `${config.contentRoot ?? ''}/georoutingv2.json`,
  `${config.contentRoot ?? ''}/georouting.json`,
  `${getFederatedContentRoot()}/federal/georouting/georoutingv2.json`
];

for (const url of urls) {
  let resp = await fetch(url);
  if (resp.ok) {
    if (url.includes('georouting.json')) {
      const json = await resp.json();
      // eslint-disable-next-line import/no-cycle
      const { default: loadGeoRoutingOld } = await import('../georouting/georouting.js');
      loadGeoRoutingOld(config, createTag, getMetadata, json);
    }
    return;
  }
}

The way I understand it, we want to load georoutingv2, if that's not available the legacy georouting and then the federal georouting as backup?

Copy link
Contributor Author

@bandana147 bandana147 Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, correct. I agree it looks cleaner and more readable. I will update it.

}
const json = await resp.json();

Expand Down
28 changes: 28 additions & 0 deletions libs/utils/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1270,4 +1270,32 @@ export function loadLana(options = {}) {
window.addEventListener('unhandledrejection', lanaError);
}

const defaultAllowedOrigins = [
Copy link
Contributor

Choose a reason for hiding this comment

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

The allowed origins are strictly related to federal content, so we either rename the constant or we move it within the method.

'https://www.adobe.com',
'https://business.adobe.com',
'https://blog.adobe.com',
Copy link
Member

Choose a reason for hiding this comment

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

Please remove blog from this list. Blog does not use GeoRouting.

'https://milo.adobe.com',
];

let federatedContentRoot;
export const getFederatedContentRoot = () => {
const { allowedOrigins = [] } = getConfig();
if (federatedContentRoot) return federatedContentRoot;

const { origin } = window.location;

federatedContentRoot = [...allowedOrigins, ...defaultAllowedOrigins].some((o) => origin.replace('.stage', '') === o)
? origin
: 'https://www.adobe.com';

if (origin.includes('localhost') || origin.includes('.hlx.')) {
// Akamai as proxy to avoid 401s, given AEM-EDS MS auth cross project limitations
federatedContentRoot = origin.includes('.hlx.live')
? 'https://main--federal--adobecom.hlx.live'
: 'https://www.stage.adobe.com';
}

return federatedContentRoot;
};

Copy link
Contributor

@mokimo mokimo Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IMO we should have a dynamic import for the whole federated content or move it to its own separate module.

I would guess that most pages do not use federal content and thus shouldn't load it at all.

The blocks that do use the feature, should load it themselves.

export const reloadPage = () => window.location.reload();
10 changes: 0 additions & 10 deletions test/blocks/global-navigation/utilities/utilities.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
toFragment,
getFedsPlaceholderConfig,
federatePictureSources,
getFederatedContentRoot,
getAnalyticsValue,
decorateCta,
hasActiveLink,
Expand Down Expand Up @@ -49,15 +48,6 @@ describe('global navigation utilities', () => {
expect(fragment2.tagName).to.equal('SPAN');
});

// No tests for using the the live url and .hlx. urls
// as mocking window.location.origin is not possible
describe('getFedsContentRoot', () => {
it('should return content source for localhost', () => {
const contentSource = getFederatedContentRoot();
expect(contentSource).to.equal(baseHost);
});
});

describe('federatePictureSources', () => {
// The test scenarios tests decorated or non decorated links.
// https://adobe.com/media.png
Expand Down
29 changes: 15 additions & 14 deletions test/features/georouting/georouting.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { stub } from 'sinon';
import { expect } from '@esm-bundle/chai';

const { default: init, getCookie } = await import('../../../libs/features/georouting/georouting.js');
let { createTag, getMetadata } = await import('../../../libs/utils/utils.js');
let { getMetadata } = await import('../../../libs/utils/utils.js');
const { createTag } = await import('../../../libs/utils/utils.js');

const mockConfig = {
locales: {
Expand Down Expand Up @@ -136,7 +137,7 @@ describe('GeoRouting', () => {

it('Does create a modal if detected country from IP is CH and url prefix is US', async () => {
// prepare
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.not.be.null;
Expand All @@ -146,7 +147,7 @@ describe('GeoRouting', () => {
// prepare
setUserCountryFromIP('US');
sessionStorage.setItem('international', 'us');
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.be.null;
Expand All @@ -157,7 +158,7 @@ describe('GeoRouting', () => {
it('If aiming for CH page and IP in Switzerland no modal is shown', async () => {
// prepare
mockConfig.locale.prefix = 'ch_de';
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.be.null;
Expand All @@ -167,7 +168,7 @@ describe('GeoRouting', () => {

it('If aiming for US page but IP in Switzerland shows CH links and US continue', async () => {
// prepare
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.not.be.null;
Expand All @@ -192,7 +193,7 @@ describe('GeoRouting', () => {
mockConfig.locale.prefix = 'ch_fr';
setUserCountryFromIP('US');
sessionStorage.setItem('international', 'de');
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.not.be.null;
Expand All @@ -215,7 +216,7 @@ describe('GeoRouting', () => {
// prepare
mockConfig.locale.prefix = 'mena_en';
setUserCountryFromIP('BW');
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.not.be.null;
Expand All @@ -239,7 +240,7 @@ describe('GeoRouting', () => {
mockConfig.locale.prefix = 'mena_en';
setUserCountryFromIP('BW');
document.cookie = 'international=ch_de;path=/';
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.not.be.null;
Expand All @@ -262,7 +263,7 @@ describe('GeoRouting', () => {
// prepare
mockConfig.locale.prefix = 'ch_de';
sessionStorage.setItem('international', 'ch_fr');
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
mockConfig.locale.prefix = '';
Expand All @@ -272,7 +273,7 @@ describe('GeoRouting', () => {
it('If IP is from an unknown country no modal is show', async () => {
// prepare
setUserCountryFromIP('NOEXIST');
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.be.null;
Expand All @@ -284,7 +285,7 @@ describe('GeoRouting', () => {
// prepare
stubFallbackMetadata('off');
stubHeadRequestToReturnVal('/ch_it', false);
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.not.be.null;
Expand All @@ -308,7 +309,7 @@ describe('GeoRouting', () => {
it('Will show fallback links if fallbackrouting is on and page exist request fails', async () => {
// prepare
stubHeadRequestToReturnVal('/ch_it', false);
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.not.be.null;
Expand All @@ -328,7 +329,7 @@ describe('GeoRouting', () => {
stubHeadRequestToReturnVal('/ch_de', false);
stubHeadRequestToReturnVal('/ch_it', false);
stubHeadRequestToReturnVal('/ch_fr', false);
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
// assert
expect(modal).to.be.null;
Expand All @@ -341,7 +342,7 @@ describe('GeoRouting', () => {

it('Sets international and georouting_presented cookies on link click in modal', async () => {
// prepare
await init(mockConfig, createTag, getMetadata);
await init(mockConfig, createTag, getMetadata, mockGeoroutingJson);
const modal = document.querySelector('.dialog-modal');
const cookie = getCookie('international');
const storage = sessionStorage.getItem('international');
Expand Down
54 changes: 51 additions & 3 deletions test/features/georoutingv2/georoutingv2.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import { expect } from '@esm-bundle/chai';
import { setViewport } from '@web/test-runner-commands';

const { default: init, getCookie } = await import('../../../libs/features/georoutingv2/georoutingv2.js');

let { getMetadata } = await import('../../../libs/utils/utils.js');
const { createTag, loadStyle, loadBlock, setConfig } = await import('../../../libs/utils/utils.js');
const { createTag, loadStyle, loadBlock, setConfig, getFederatedContentRoot } = await import('../../../libs/utils/utils.js');

const mockConfig = {
locales: {
Expand Down Expand Up @@ -212,8 +213,38 @@ function stubHeadRequestToReturnVal(prefix, val) {
);
}

const stubFetchForGeorouting = () => {
const stubFetchForGeorouting = (val) => {
window.fetch.withArgs('/georoutingv2.json').returns(
new Promise((resolve) => {
resolve({
ok: val,
json: () => mockGeoroutingJson,
});
}),
);
mockGeoroutingJson.georouting.data.forEach((d) => {
const prefix = d.prefix ? `/${d.prefix}` : '';
stubHeadRequestToReturnVal(prefix, true);
});
};

const stubFetchForGeoroutingOld = (val) => {
window.fetch.withArgs('/georouting.json').returns(
new Promise((resolve) => {
resolve({
ok: val,
json: () => mockGeoroutingJson,
});
}),
);
mockGeoroutingJson.georouting.data.forEach((d) => {
const prefix = d.prefix ? `/${d.prefix}` : '';
stubHeadRequestToReturnVal(prefix, true);
});
};

const stubFetchForFederalGeorouting = () => {
window.fetch.withArgs(`${getFederatedContentRoot()}/federal/georouting/georoutingv2.json`).returns(
new Promise((resolve) => {
resolve({
ok: true,
Expand All @@ -240,7 +271,7 @@ const closeModal = () => {
describe('GeoRouting', () => {
before(() => {
setUserCountryFromIP();
stubFetchForGeorouting();
stubFetchForGeorouting(true);
setGeorouting();
});
after(() => {
Expand Down Expand Up @@ -597,4 +628,21 @@ describe('GeoRouting', () => {
// cleanup
setGeorouting('on');
});

it('Will load georouting even if georoutingv2 and georouting file is not found', async () => {
stubFetchForGeorouting(false);
stubFetchForGeoroutingOld(false);
stubFetchForFederalGeorouting();
await init(mockConfig, createTag, getMetadata, loadBlock, loadStyle);
const modal = document.querySelector('.dialog-modal');
expect(modal).to.not.be.null;
});

it('Will not load new georouting modal if georoutingv2 is not found', async () => {
stubFetchForGeorouting(false);
stubFetchForGeoroutingOld(true);
await init(mockConfig, createTag, getMetadata, loadBlock, loadStyle);
const tabs = document.querySelector('.tabs');
expect(tabs).to.be.null;
});
});
Loading
Loading