-
Notifications
You must be signed in to change notification settings - Fork 44
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
Implement faceting in search #46
Conversation
curquiza
commented
Jun 4, 2020
•
edited
Loading
edited
- Fixes Search parametters don't accept lists of strings (and should) #41 (close the PR Fix bug: Search params crash when they are a list of strings #42 in favor of this one)
- There are not enough tests for the search, I opened an issue Improve search tests #45
- There is still the 3 same tests failing since MeiliSearch is broken
19807d3
to
ad114ce
Compare
ad114ce
to
39b3099
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just added suggestions to avoid redundancy on tests because the new assertions you added to check the content of the response make it useless to check if a single field hits
exists in response. Otherwise, sseems good :)
Co-authored-by: Samuel Jimenez <[email protected]>
Co-authored-by: Samuel Jimenez <[email protected]>
Co-authored-by: Samuel Jimenez <[email protected]>
Co-authored-by: Samuel Jimenez <[email protected]>
Co-authored-by: Samuel Jimenez <[email protected]>
@eskombro you're completely right! I added my new test too fast 🙈! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM 🎉
* Rename fields_frequency to fields_distribution according to MeiliSearch v0.11 * Add method to use faceting sub routes (#44) * Implement faceting in search (#46) * Implement faceting in search * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> Co-authored-by: Samuel Jimenez <[email protected]> * Upd tests according to change in MeiliSearch about anthentication (#48) * Add debugging part in README * Add test with multiple facetFilters (#51) * Change create_index prototype (#50) Co-authored-by: Samuel Jimenez <[email protected]>
* Rename fields_frequency to fields_distribution according to MeiliSearch v0.11 * Add method to use faceting sub routes (#44) * Implement faceting in search (#46) * Implement faceting in search * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> * Update spec/meilisearch/index/search_spec.rb Co-authored-by: Samuel Jimenez <[email protected]> Co-authored-by: Samuel Jimenez <[email protected]> * Upd tests according to change in MeiliSearch about anthentication (#48) * Add debugging part in README * Add test with multiple facetFilters (#51) * Change create_index prototype (#50) Co-authored-by: Samuel Jimenez <[email protected]>