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

Using ACDL to call WebSDK #447

Closed
wants to merge 28 commits into from
Closed

Using ACDL to call WebSDK #447

wants to merge 28 commits into from

Conversation

cuzuco2
Copy link

@cuzuco2 cuzuco2 commented Jan 31, 2024

Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):

Fix #442 and Fix#441

Test URLs:

Copy link

aem-code-sync bot commented Jan 31, 2024

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

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link

aem-code-sync bot commented Jan 31, 2024

Page Scores Audits Google
/solutions/ Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 421) PSI

@cuzuco2 cuzuco2 changed the title Pull 433 Using ACDL to call WebSDK Jan 31, 2024
openUrlForOs,
} from './scripts.js';
import { loadBreadcrumbs } from './breadcrumbs.js';

// Core Web Vitals RUM collection
sampleRUM('cwv');

const LANGUAGE_COUNTRY = getLanguageCountryFromPath(window.location.pathname);
/* const LANGUAGE_COUNTRY = getLanguageCountryFromPath(window.location.pathname);
Copy link
Collaborator

@vtsaplin vtsaplin Jan 31, 2024

Choose a reason for hiding this comment

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

@cuzuco2 Let's uncomment this and reference the new tag by adding an entry to the placeholders file.

Copy link
Author

Choose a reason for hiding this comment

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

@vtsaplin you mean something like this?

const LANGUAGE_COUNTRY = getLanguageCountryFromPath(window.location.pathname);
// const LAUNCH_URL = 'https://assets.adobedtm.com';
const ENVIRONMENT = getEnvironment(window.location.hostname, LANGUAGE_COUNTRY.country);
// Load Adobe Experience platform data collection (Launch) script
// const { launchProdScript, launchStageScript, launchDevScript } = await fetchPlaceholders();
switch (ENVIRONMENT) {
case 'prod':
loadScript('https://assets.adobedtm.com/8a93f8486ba4/e7dc9e6549e5/launch-42a4d88f6f31.min.js'); break;
case 'stage':
loadScript('https://assets.adobedtm.com/8a93f8486ba4/e7dc9e6549e5/launch-d0a308d8ad78-staging.min.js'); break;
default:
loadScript('https://assets.adobedtm.com/8a93f8486ba4/e7dc9e6549e5/launch-aef7ddf31563-development.min.js'); break;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

@cuzuco2 Let's keep the code as it was before but make a tmp change in the scripts.js:

export function getEnvironment(hostname, country) {
  if (hostname.startsWith('pull_433')) {
    return 'dev';
  }
  if (hostname.includes('hlx.page') || hostname.includes('hlx.live')) {
    return 'stage';
  }
  if (hostname.includes(`.${country}`)) {
    return 'prod';
  }
  return 'dev';
}

You'd also need to change the dev ID in placeholders.

@cuzuco2
Copy link
Author

cuzuco2 commented Feb 1, 2024

@vtsaplin please review the changes, not sure why the ghost inspector is failing, I can't execute the test on my local machine

@@ -20,18 +20,19 @@ import { loadBreadcrumbs } from './breadcrumbs.js';
sampleRUM('cwv');

const LANGUAGE_COUNTRY = getLanguageCountryFromPath(window.location.pathname);
const LAUNCH_URL = 'https://assets.adobedtm.com';
// const LAUNCH_URL = 'https://assets.adobedtm.com'; */
Copy link
Collaborator

@vtsaplin vtsaplin Feb 1, 2024

Choose a reason for hiding this comment

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

@cuzuco2 The code here needs to be exactly as it was before. The change of URL should be done in the placeholder file on SharePoint. Here are the docs: https://www.aem.live/developer/placeholders

We'll fix the Ghost Inspector later. It has been broken for a while and is sensitive to changes because it relies on image comparison.

@cuzuco2
Copy link
Author

cuzuco2 commented Feb 1, 2024

@vtsaplin ok the function is now as it was before we began testing

* Customer's XDM schema namespace
* @type {string}
*/
const CUSTOM_SCHEMA_NAMESPACE = '_sitesinternal';
Copy link

Choose a reason for hiding this comment

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

This should be the namespace for bitdefender

Copy link
Collaborator

Choose a reason for hiding this comment

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

@iuliag - we change it to _bitdefender. It is ok?

const pageViewPromise = analyticsTrackPageViews(document); // track page view early

await import('./adobe-client-data-layer.min.js');
await import('./alloy.min.js');
Copy link
Collaborator

Choose a reason for hiding this comment

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

@cuzuco2 We probably need to loas alloy.js first

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.

Send current state of data layer to AEP
4 participants