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

Add method to use faceting sub routes #44

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

curquiza
Copy link
Member

@curquiza curquiza commented Jun 3, 2020

NB: tests do not run yet because the v0.11 is not out.
If you want to run the tests with the last version of MeiliSearch, use cargo in the MeiliSearch repo, and go on my branch.

Copy link
Member

@eskombro eskombro left a comment

Choose a reason for hiding this comment

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

I test your PR and get 7 failing tests.

My testing steps:

For MeiliSearch

  1. Pull origin master du repo meilisearch
  2. cargo build --release
  3. ./target/release/meilisearch --master-key=masterKey

For meilisearch-ruby

  1. git pull origin master
  2. git checkout add-facets-settings-routes
  3. bundle exec rspec
Finished in 8.91 seconds (files took 0.42274 seconds to load)
115 examples, 7 failures

Failed examples:

rspec ./spec/meilisearch/index/documents_spec.rb:65 # MeiliSearch::Index - Documents All basic tests with primary-key inference browses documents with query parameters
rspec ./spec/meilisearch/index/settings_spec.rb:43 # MeiliSearch::Index - Settings On global settings routes gets default values of settings
rspec ./spec/meilisearch/index/settings_spec.rb:410 # MeiliSearch::Index - Settings On attributes-for-faceting sub-routes gets default values of attributes for faceting
rspec ./spec/meilisearch/index/settings_spec.rb:416 # MeiliSearch::Index - Settings On attributes-for-faceting sub-routes updates attributes for faceting
rspec ./spec/meilisearch/index/settings_spec.rb:423 # MeiliSearch::Index - Settings On attributes-for-faceting sub-routes resets attributes for faceting
rspec ./spec/meilisearch/index/settings_spec.rb:442 # MeiliSearch::Index - Settings Index with primary-key gets the default values of settings
rspec ./spec/meilisearch/index/stats_spec.rb:35 # MeiliSearch::Index - Stats gets the distribution of fields

Coverage report generated for RSpec to /Users/esk/Meili/meilisearch-ruby/coverage. 869 / 892 LOC (97.42%) covered.
SimpleCov failed with exit 1

Seems like basic tests like doing a simple search request with an offset is not working 😞

@curquiza
Copy link
Member Author

curquiza commented Jun 3, 2020

@eskombro git fetch origin?
Only 3 tests should fail for the moment

@eskombro
Copy link
Member

eskombro commented Jun 3, 2020

@eskombro git fetch origin?

Nothing, same :/

@eskombro
Copy link
Member

eskombro commented Jun 3, 2020

Ok, got 3 Failures 🥳 :

Failures:

  1) MeiliSearch::Client - Keys fails to get settings if public key used
     Failure/Error: expect { new_client.index(@uid).settings }.to raise_meilisearch_http_error_with(403)
     
       expected Exception with an instance of MeiliSearch::HTTPError and having attributes {:code => 403}, got #<MeiliSearch::HTTPError: 401: Unauthorized - Invalid api key: 3b3bf839485f90453acc6159ba18fbed673ca88523093def11a9b4f4320e44a5.> with backtrace:
         # ./lib/meilisearch/http_request.rb:69:in `validate'
         # ./lib/meilisearch/http_request.rb:26:in `http_get'
         # ./lib/meilisearch/index.rb:121:in `settings'
         # ./spec/meilisearch/client/keys_spec.rb:24:in `block (3 levels) in <top (required)>'
         # ./spec/meilisearch/client/keys_spec.rb:24:in `block (2 levels) in <top (required)>'
     # ./spec/meilisearch/client/keys_spec.rb:24:in `block (2 levels) in <top (required)>'

  2) MeiliSearch::Client - Keys fails to get keys if private key used
     Failure/Error: expect { new_client.keys }.to raise_meilisearch_http_error_with(403)
     
       expected Exception with an instance of MeiliSearch::HTTPError and having attributes {:code => 403}, got #<MeiliSearch::HTTPError: 401: Unauthorized - Invalid api key: 8dcbb482663333d0280fa9fedf0e0c16d52185cb67db494ce4cd34da32ce2092.> with backtrace:
         # ./lib/meilisearch/http_request.rb:69:in `validate'
         # ./lib/meilisearch/http_request.rb:26:in `http_get'
         # ./lib/meilisearch/client.rb:49:in `keys'
         # ./spec/meilisearch/client/keys_spec.rb:30:in `block (3 levels) in <top (required)>'
         # ./spec/meilisearch/client/keys_spec.rb:30:in `block (2 levels) in <top (required)>'
     # ./spec/meilisearch/client/keys_spec.rb:30:in `block (2 levels) in <top (required)>'

  3) MeiliSearch::Index fails to manipulate index object after deletion
     Failure/Error: expect { @index2.delete }.to raise_meilisearch_http_error_with(404)
       expected Exception with an instance of MeiliSearch::HTTPError and having attributes {:code => 404} but nothing was raised
     # ./spec/meilisearch/index/base_spec.rb:61:in `block (2 levels) in <top (required)>'

Finished in 8.01 seconds (files took 0.39283 seconds to load)
115 examples, 3 failures

Failed examples:

rspec ./spec/meilisearch/client/keys_spec.rb:21 # MeiliSearch::Client - Keys fails to get settings if public key used
rspec ./spec/meilisearch/client/keys_spec.rb:27 # MeiliSearch::Client - Keys fails to get keys if private key used
rspec ./spec/meilisearch/index/base_spec.rb:57 # MeiliSearch::Index fails to manipulate index object after deletion

Coverage report generated for RSpec to /Users/esk/Meili/meilisearch-ruby/coverage. 892 / 892 LOC (100.0%) covered.
SimpleCov failed with exit 1

As you said, those arre waiting for fixes from MeiliSearch. Your code and tests look good to me tho 🎉

@eskombro
Copy link
Member

eskombro commented Jun 3, 2020

I put those issues here to keep track:

meilisearch/meilisearch#742
meilisearch/meilisearch#741

@curquiza correct me if I'm wrong please :)

@curquiza curquiza force-pushed the add-facets-settings-routes branch from 432c1b1 to d299941 Compare June 4, 2020 07:44
@curquiza curquiza requested a review from eskombro June 4, 2020 07:44
Copy link
Member

@eskombro eskombro left a comment

Choose a reason for hiding this comment

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

👍

@curquiza curquiza merged commit 51c901e into v0.11-meilisearch Jun 4, 2020
@curquiza curquiza deleted the add-facets-settings-routes branch June 4, 2020 09:10
curquiza added a commit that referenced this pull request Jun 15, 2020
* 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]>
bidoubiwa pushed a commit that referenced this pull request Mar 8, 2021
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants