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

Prepend site.baseurl in search.json #829

Merged
merged 2 commits into from Sep 27, 2019
Merged

Conversation

danielhelfand
Copy link
Contributor

@danielhelfand danielhelfand commented Sep 26, 2019

Signed-off-by: danielhelfand helfand.4@gmail.com

What does this PR do?

#828 removed the stripping of the url from search.json as part of an effort to fix eclipse-che/che#14647. While running locally with changes in #828 fixes the issue, it does not solve the problem for the che-docs site.

There appears to be a difference in how search works locally versus the site itself. The site requires /che/docs as part of the url while it does not locally.

This pull request prepends /che/docs in search.json as part of the url.

Considerations

This change will mean that search will not work locally for the site.

If there is a staging environment for the site, it would make sense to test there as opposed to verifying locally.

Signed-off-by: danielhelfand <helfand.4@gmail.com>
@danielhelfand
Copy link
Contributor Author

This does not appear to be the issue actually. I'll do some further digging.

@danielhelfand
Copy link
Contributor Author

In reviewing this, it does look like the issue seems to be related to /che/docs not being included with the base url. If you go to the site now, you'll get a request url of https://www.eclipse.org/che-7/installing-che-on-openshift-4-from-operatorhub/ if searching for the OpenShift 4 installation doc.

So it refers to https://www.eclipse.org/ as the base url. If there's a more elegant way to change site.baseurl, please let me know, but I do believe this should address the search issue.

Copy link

@rkratky rkratky left a comment

Choose a reason for hiding this comment

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

@danielhelfand, thanks for digging into this. When merging the #828 PR, I didn't realize it would affect the various environments differently.

We need site.baseurl to be / for local previews, /che/docs/ for prod builds (because on eclipse.org, the docs are under /docs/che/, not in the webserver root), and finally /docs/ for WAR builds (which is yet to be fixed; see eclipse-che/che#14386).

I tested the solution I'm suggesting on both local and in a prod-like environment (you can do that by building with run.sh -prod and copying the contents of _site/ to a che/docs/ dir structure in your local web server's root). The clunky removal of the first / is needed because if we were to just prepend site.baseurl, the URL on local previews would start with //, which wouldn't work.

@@ -11,7 +11,7 @@ search: exclude
"title": "{{ page.title | escape }}",
"tags": "{{ page.tags }}",
"keywords": "{{page.keywords}}",
"url": "{{ page.url }}",
"url": "{{ page.url | prepend: '/che/docs' }}",
Copy link

Choose a reason for hiding this comment

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

Suggested change
"url": "{{ page.url | prepend: '/che/docs' }}",
"url": "{{ page.url | remove_first: '/' | prepend: site.baseurl }}",

@@ -24,7 +24,7 @@ search: exclude
"title": "{{ post.title | escape }}",
"tags": "{{ post.tags }}",
"keywords": "{{post.keywords}}",
"url": "{{ post.url }}",
"url": "{{ post.url | prepend: '/che/docs' }}",
Copy link

Choose a reason for hiding this comment

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

Suggested change
"url": "{{ post.url | prepend: '/che/docs' }}",
"url": "{{ page.url | remove_first: '/' | prepend: site.baseurl }}",

Signed-off-by: danielhelfand <helfand.4@gmail.com>
@danielhelfand
Copy link
Contributor Author

@rkratky Thanks for the detailed explanation and suggestions. I just pushed up a change, but let me know if there's anything else needed.

@danielhelfand danielhelfand changed the title Prepend /che/docs to search.json Prepend site.baseurl in search.json Sep 27, 2019
@rkratky rkratky merged commit c9c2f95 into eclipse-che:master Sep 27, 2019
@rkratky
Copy link

rkratky commented Sep 27, 2019

Merged. Thanks for investigating this, @danielhelfand.

@danielhelfand
Copy link
Contributor Author

@rkratky Thanks! How soon will the changes be live?

@danielhelfand
Copy link
Contributor Author

Never mind, I see that it's published and looks to be working. Thanks again for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants