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

feat: Make Sitemap data available via GraphQL query #4927

Merged
merged 4 commits into from
Jan 24, 2019

Conversation

impactmass
Copy link
Contributor

@impactmass impactmass commented Jan 18, 2019

Resolves #4796
Impact: major
Type: feature

To be tested along with reactioncommerce/example-storefront#488

Issue

#4796
The sitemaps are generated in the core API app (ie. the reaction service), and currently available on the API domain/url. We need to make the sitemap information available through GraphQL so standalone storefronts can use it.

Solution

Details

  • The query takes 2 params: handle and shopUrl. The handle is a string used when the sitemap was created. It can have values like sitemap.xml, sitemap-tags-1.xml.
  • Sitemaps are saved with the shopId they belong to. Storefront apps like starkerkit get shopId value later on in the rendering process. To get sitemap information without depending on the rendering process, the shopUrl is passed as param.
  • The shopUrl is confirmed in the GQL implementation to ensure the domain requested is for a known shop domain Shops.findOne({ domains: domain })
  • The shopUrl param is also used to replace the xml placeholder: sitemap.xml.replace(/BASE_URL/g, shopUrl)

Testing

[ @kieckhafer as the primary reviewer. @dancastellon I added you to take a pass based on the fact that you worked previously on sitemap. ]

@impactmass impactmass force-pushed the feat-4796-impactmass-sitemap-query branch from 81be91f to 732eb84 Compare January 22, 2019 21:35
@impactmass impactmass changed the title WIP 4796 Add Sitemap data GQL query feat: Make Sitemap data available via GraphQL query Jan 22, 2019
Signed-off-by: Erik Kieckhafer <[email protected]>
@kieckhafer kieckhafer force-pushed the feat-4796-impactmass-sitemap-query branch from 9b1cbbe to 147143d Compare January 23, 2019 06:55
Copy link
Member

@kieckhafer kieckhafer left a comment

Choose a reason for hiding this comment

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

Works as described. IMO can be merged once @dancastellon takes a look.

@impactmass impactmass force-pushed the feat-4796-impactmass-sitemap-query branch from 7d9f183 to dd7b7f7 Compare January 24, 2019 16:37
Copy link
Contributor

@dancastellon dancastellon left a comment

Choose a reason for hiding this comment

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

Looks good!

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