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

Multi source support for commerce configs and placeholders #286

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

fnhipster
Copy link
Collaborator

@fnhipster fnhipster commented Jan 28, 2025

Multi Stores

Multistore refers to a streamlined architecture that enables managing multiple stores with unique catalogs and localization while leveraging a shared code base and unified content delivery.

Overrides Commerce endpoint configs and placeholders based on the root page metadata provided. Otherwise, it defaults to the base /.

Metadata are managed as a bulk (https://www.aem.live/docs/bulk-metadata) in metadata.xlsx.

image
  • root: path to where configs.xlsx files are located. default /](root: Path to the directory where configs.xlsx files are located. Default is /.)
  • placeholders: path to the placeholders.json filer to use. Default /placeholders.json. Adding a path in a new line in this value adds an override file where both paths will be fetched, and the values of the second will override the first.](placeholders: Path to the placeholders.json file to use. The default is /placeholders.json. Adding a new line with another path in this value specifies an override file. Both files will be fetched, and the values in the second file will override those in the first.)

☑️ TODO

  • Fallback placeholders
  • Load fragments relative to root
  • Localized links @anthoula
  • Copy over the rest of the pages. /drafts/multistore/* only have the index at the moment. @anthoula
  • 404
  • Remove decorateLinks @anthoula
  • Fork a "demo instance" and setup folder mapping in /products/: /products/default @fnhipster
  • Review indexes /helix-query.yaml

https://multistore--aem-boilerplate-commerce--hlxsites.hlx.page/
image

https://multistore--aem-boilerplate-commerce--hlxsites.hlx.page/drafts/multistore/en/
image

https://multistore--aem-boilerplate-commerce--hlxsites.hlx.page/drafts/multistore/en_ca/
image

Test URLs:

Copy link

aem-code-sync bot commented Jan 28, 2025

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 28, 2025

Page Scores Audits Google
📱 / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
📱 /drafts/multistore/en_ca/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /drafts/multistore/en_ca/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
📱 /drafts/multistore/en/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /drafts/multistore/en/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@fnhipster fnhipster changed the title refactor: update fetchPlaceholders import path and enhance multi-stor… Multi source support for commerce configs and placeholders Jan 28, 2025
@fnhipster fnhipster marked this pull request as draft January 28, 2025 21:18
scripts/configs.js Outdated Show resolved Hide resolved
… as the default

- decorate links for EDS pages
- localize links for Blocks
scripts/scripts.js Outdated Show resolved Hide resolved
… as the default

- localize links for header block
- localize links for product links
@herzog31
Copy link

herzog31 commented Feb 4, 2025

Adding a new line with another path in this value specifies an override file. Both files will be fetched, and the values in the second file will override those in the first.

This is a content management use case and IMO should not be done in code. For document-based authoring, you can use Excel formula to create a reference to another file and then overwrite values you want to change. Both AEM and DA support language copy rollouts (MSM).

@@ -18,6 +18,42 @@
</script>
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta property="og:title" content="Page not found">
<script type="module">
Copy link

Choose a reason for hiding this comment

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

You can skip this part and hardcode the paths. For multi store we likely would need to have localized 404 pages which then get assigned via CDN. Something like /en/404 and /fr/404 documents which get mapped to the 404 status code using Fastly VCL.

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'm trying to avoid the need for code changes to add a new language and have that all be driven by the CMS, hence why the root is set at the metadata level. Aside from the folder mapping used for PDP, it's almost possible, but I digress.

What's the harm in having this dynamically set here other than optimizing the 404 page? Perhaps we can add a condition so the fetch is skipped completely if the metadata is present, i.e., already optimized at the CDN level.

Copy link

Choose a reason for hiding this comment

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

For multi language the 404s would need to be driven by the CMS yes.

What's the harm in having this dynamically set here other than optimizing the 404 page?

No harm, but that 404 wouldn't be localized. Edge Delivery delivers the same 404.html page regardless of the current path which makes it unfeasible for production use cases.

@@ -503,20 +503,35 @@ function decorateSections(main) {
*/
// eslint-disable-next-line import/prefer-default-export
async function fetchPlaceholders(prefix = 'default') {
const overrides = getMetadata('placeholders') || '';
const [fallback, override] = overrides.split('\n');
Copy link

Choose a reason for hiding this comment

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

See my comment on placeholder fallbacks.

scripts/configs.js Show resolved Hide resolved
scripts/scripts.js Outdated Show resolved Hide resolved
* Decorates links.
* @param {Element} main The main element
*/
export function decorateLinks(main) {
Copy link

Choose a reason for hiding this comment

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

This should not be needed once the content was updated accordingly. There shouldn't be any links in the content that point to a URL that does not include the root.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What about links added to the document? How would those be relative to the root defined?

Copy link

Choose a reason for hiding this comment

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

You would link to a document like /us/en/welcome.docx, so it matches the actual content structure.

Copy link
Collaborator Author

@fnhipster fnhipster Feb 4, 2025

Choose a reason for hiding this comment

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

That's a good point. @anthoula, can you try removing decorateLinks? Document links, such as those in the nav fragment, should be updated and localized.

Co-authored-by: Mark J. Becker <[email protected]>
@fnhipster
Copy link
Collaborator Author

Adding a new line with another path in this value specifies an override file. Both files will be fetched, and the values in the second file will override those in the first.

This is a content management use case and IMO should not be done in code. For document-based authoring, you can use Excel formula to create a reference to another file and then overwrite values you want to change. Both AEM and DA support language copy rollouts (MSM).

Interesting. Can you expand some more on this? Do you have any reference I can look at on how to achieved this in a doc-based setup?

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.

4 participants