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

Web search uses api endpoint #1822

Merged
merged 3 commits into from Jan 31, 2017

Conversation

Projects
None yet
3 participants
@jberger
Contributor

jberger commented Nov 24, 2016

NOTE! I want people to pay careful attention to this change, try it, try to break it. Search is one of the most important parts of the site and as such I'm cautious.

Hopefully, if I've done my job correctly the results should be exactly the same as currently. I have not exposed the boosts etc.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Nov 24, 2016

Coverage Status

Coverage remained the same at 70.025% when pulling d29d96e on jberger/web_search_endpoint into cf1b30f on master.

coveralls commented Nov 24, 2016

Coverage Status

Coverage remained the same at 70.025% when pulling d29d96e on jberger/web_search_endpoint into cf1b30f on master.

@jberger

This comment has been minimized.

Show comment
Hide comment
@jberger

jberger Nov 24, 2016

Contributor

Also, pay attention to the slight change in single_dist behavior (which defines if the "search in distribution" link is shown on the results). Before it checked to see if the munged query contained a distribution: limiter. This meant that the search was already within a limitation to just a distribution. As the web side no longer has access to the munged query some change had to be made.

The api currently (if not explicitly told) decides whether it should show expanded to collapsed results based on this regex which is similar. It then contains within the result a key saying whether the results were collapsed. It is this value that is then used to decide if the "search in distribution" link is shown. Is this sufficient or does it need tweaking?

Contributor

jberger commented Nov 24, 2016

Also, pay attention to the slight change in single_dist behavior (which defines if the "search in distribution" link is shown on the results). Before it checked to see if the munged query contained a distribution: limiter. This meant that the search was already within a limitation to just a distribution. As the web side no longer has access to the munged query some change had to be made.

The api currently (if not explicitly told) decides whether it should show expanded to collapsed results based on this regex which is similar. It then contains within the result a key saying whether the results were collapsed. It is this value that is then used to decide if the "search in distribution" link is shown. Is this sufficient or does it need tweaking?

jberger added some commits Nov 20, 2016

Use the "simple" endpoint in the module model's first method
Not only does this move one more method to using the api rather than ES
directly, it also fixes "lucky" redirection test that were broken by the
changes to the web search.
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jan 31, 2017

Coverage Status

Coverage remained the same at 70.025% when pulling cad0e22 on jberger/web_search_endpoint into b771f9b on master.

coveralls commented Jan 31, 2017

Coverage Status

Coverage remained the same at 70.025% when pulling cad0e22 on jberger/web_search_endpoint into b771f9b on master.

@oalders oalders merged commit 56d0372 into master Jan 31, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 70.025%
Details

@oalders oalders deleted the jberger/web_search_endpoint branch Jan 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment