search

mitodl / ocw-www

Other
0 stars 0 forks source link

Parse URL to fix section handling #99

Closed noisecapella closed 3 years ago

noisecapella commented 3 years ago

Pre-Flight checklist

What are the relevant tickets?

Fixes #93

What's this PR do?

Instead of using short_url, this parses url which is the legacy url in order to get the section parts. Ideally this would be done in ocw-data-parser but at this point it seems simpler to fix it here.

How should this be manually tested?

Search for "water oceans" with the quotes. Click on "resources". Hover over the link in the first result. It should be something like http://localhost:8063/courses/ec-719-d-lab-water-climate-change-and-health-spring-2019/sections/additional-resources/water-oceans/, depending on your open-discussions instance

github-actions[bot] commented 3 years ago

🚀 Deployed on https://ocw-www-pr-99--ocw-next.netlify.app

github-actions[bot] commented 3 years ago

Lighthouse results:

results for https://ocw-www-pr-99--ocw-next.netlify.app/:

Accessibility Best Practices Performance Progressive Web App SEO
72 🙂 80 😄 75 🙂 42 😨 62 😐

results for https://ocw-www-pr-99--ocw-next.netlify.app/search/:

Accessibility Best Practices Performance Progressive Web App SEO
71 🙂 93 🎉 77 🙂 42 😨 56 😟
pdpinch commented 3 years ago

Should we still fix this in ocw-data-parser?

If we don’t fix it in ocw-data-parser, does that mean we will be importing incorrect data into ocw-studio?

Ideally this would be done in ocw-data-parser but at this point it seems simpler to fix it here.

noisecapella commented 3 years ago

For the second question, no, we are using correct urls in ocw-studio already. We are updating these links in ocw-to-hugo when we export to markdown. ocw-studio imports from the markdown, not from the parsed JSON, so it is dealing with links which are already updated.

For the first question, possibly but it's a cost/benefit question. open-discussions imports from the plone JSON, using ocw-data-parser to convert the plone JSON files to a single parsed JSON. That means the ContentFile objects have incorrect urls in the search results, which this PR fixes in Javascript (which we are already doing for other links). This is all needlessly complicated but if we are getting rid of ocw-data-parser and ocw-to-hugo anyway in the near future it might not be worth trying to fix it at that level.

gumaerc commented 3 years ago

Just a heads up, we might want to move the code in this PR over to ocw-hugo-themes as the search app will live there going forward. https://github.com/mitodl/ocw-www/pull/101 will be merged as part of the cutover and will remove the search app from this repo.