Fix for CMS that may not have .html extensions #1029
Conversation
Error: Cms pages without .html in the url key will never resolve Any Other Pages without .html wont resolve Fix: Add a check count, and try and resolve both .html and not .html urls
|
|
|
This pull request is automatically deployed with Now. |
Check if extension has html to stop infinate loop Remove .html extension if it has and check if fetch resolves
|
Related to and maybe dependent on #936 . |
| if (resolve === null) { | ||
| if (opts.route.endsWith('.html')) { | ||
| opts.route = opts.route.replace('.html', ''); | ||
| resolve = await fetchRoute(opts) |
There was a problem hiding this comment.
Let's say the user clicks on an external ink to venia.com/accessories.html. If that catalog page no longer exists, this function will attempt to look up the entity at accessories. If that's a CMS page, it will display the CMS page without changing the URL in the browser location. Is this what we want?
I think there may be a better place to remove the URL key than this.
There was a problem hiding this comment.
Catalog categories have .html aswell, so unless a cms page is called /checkout then thats the only time i think this wont work.
There was a problem hiding this comment.
What if there used to be an /accessories.html, but that is no longer a category, and instead there is a CMS page called /accessories? That's where I'm worried about a bug. This may need to just redirect instead of silently removing the .html from the active route. Am I missing something?
|
@zetlen can you take a look in review? Thanks! |
|
Gotta close this in favor of UPWARD-driven and GraphQL |
Error:
Cms pages without .html in the url key will never resolve
Any Other Pages without .html wont resolve
Fix:
Add a check count, and try and resolve both .html and not .html urls