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-153658 audit script #25

Merged
merged 2 commits into from
Jul 9, 2024
Merged

MWPW-153658 audit script #25

merged 2 commits into from
Jul 9, 2024

Conversation

npeltier
Copy link
Collaborator

@npeltier npeltier commented Jul 5, 2024

  • parse one or several url or sitemap files (can be added in manifest),
  • for each url from sitemap, parse page for personnalization fragment or fragment,
  • for each ost link found in html, keep usage,
  • for each osi, get wcs entry,
  • output everything as CSV

Test URLs:

- parse one or several url or sitemap files (can be added in manifest),
- for each url from sitemap, parse page for personnalization fragment or fragment,
- for each ost link found in html, keep usage,
- for each osi, get wcs entry,
- output everything as CSV
Copy link
Collaborator

@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.

It looks very good to me.

In #22, there is eslint/prettier lint rules. If possible I suggest to lint your code with it before the initial merge.

const prefixWcsKey = (key) => ('wcs ' + key);

async function getCommerceData(osi) {
const response = await fetch(wcsUrl(osi));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that okay if the OSI doesn't return an offer anymore via WCS? AOS can always deliver the parameters of an OSI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think i prefer usage of WCS as blank answer there is also something we should be worried about

@@ -0,0 +1,186 @@
https://www.adobe.com/cc-shared/assets/sitemap.xml
https://www.adobe.com/au/cc-shared/assets/sitemap.xml
https://www.adobe.com/cn/cc-shared/assets/sitemap.xml
Copy link
Collaborator

Choose a reason for hiding this comment

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

where does the list of sitemaps coming from?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i created it out of the locales we have in our code

Copy link
Collaborator

Choose a reason for hiding this comment

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

we should probably add a disclamer that these are not all Milo pages, wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this manifest is here for somebody (me for now) to use. Nothing say this manifest is here to get everything :) (i wait for discussion with business for this)

Copy link
Collaborator

@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.

approving, although sitemaps list is missing homepage, but we can add it later

Copy link

aem-code-sync bot commented Jul 9, 2024

Page Scores Audits Google
/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@npeltier npeltier merged commit 73e67d7 into adobecom:main Jul 9, 2024
5 checks passed
rohitsahu pushed a commit that referenced this pull request Jul 10, 2024
…into rosahu/MWPW-147172

* 'rosahu/MWPW-147172' of https://github.com/adobecom/mas:
  feat(MWPW-142267): Merch What's Included and Merch Mnemonic List (TwP) (#4)
  MWPW-153962: update /lib to /libs (#30)
  MWPW-153658 audit script (#25)
  MWPW-153962: support maslibs approach (#28)
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.

3 participants