Ability to preserve primary key type

[Borzik]

Pull Request

According to MeiliSearch docs

The document id must be an integer or a string

However, currently meilisearch-rails always stringifies primary keys before sending them to MeiliSearch. As a result, sorting by primary key becomes impossible, because hits will be sorted alphabetically:

Color.create!(id: 1, name: 'purple', short_name: 'p')
Color.create!(id: 2, name: 'blue', short_name: 'b')
Color.create!(id: 10, name: 'yellow', short_name: 'l')
raw_search_results = Color.raw_search('*', sort: ['id:asc'])['hits']
raw_search_result_ids = raw_search_results.map { |h| h['id'].to_i }

raw_search_result_ids == [1, 2, 10] #> false
raw_search_result_ids #> [1, 10, 2]

What does this PR do?

• adds a stringify_primary_keys configuration which will preserve current behaviour
• if stringify_primary_keys is set to false, meilisearch-rails will not stringify primary keys when sending them to MeiliSearch, and will look up record by integers instead of strings

Ideally full test suite will have to be ran twice, with this config set to true and false; I did so manually when submitting this PR. But I wasn't sure if this PR will be considered so didn't expand specs, other than adding a test which confirms that primary-key-sorted docs will preserve order.

PR checklist

Please check if your PR fulfills the following requirements:

• [+] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [+] Have you read the contributing guidelines?
• [+] Have you made sure that the title is accurate and descriptive of the changes?

[ellnix]

I'm not sure about this change to be honest, it seems like a lot of complexity, and like you said it's hard to test thoroughly. I see that you did a lot of work toward this, but I feel that this gem is messy to begin with and the added ifs would not help in that regard...

I'll leave it to @brunoocasali to decide if we should go in this direction.

[codecov]

Codecov Report

Attention: Patch coverage is 57.89474% with 8 lines in your changes missing coverage. Please review.

Project coverage is 88.53%. Comparing base (5c408f1) to head (11db591).

Files with missing lines	Patch %	Lines
lib/meilisearch/rails/multi_search/result.rb	30.00%	7 Missing ⚠️
lib/meilisearch-rails.rb	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #390      +/-   ##
==========================================
- Coverage   89.13%   88.53%   -0.61%     
==========================================
  Files          13       13              
  Lines         764      776      +12     
==========================================
+ Hits          681      687       +6     
- Misses         83       89       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Borzik]

It didn't take that much of my time really, so no worries if it doesn't get merged — at least it will give some context to people who might experience the same issue. It is possible to overcome it by putting an id in a separate custom attribute (those don't get stringified and remain integers, so will sort correctly).

As far as I see, the whole stringified primary key came from Algolia, which I assume doesn't support integer primary keys. But since MeiliSearch itself has both string and integer in the spec for primary keys, I think it's better to have framework implementation match the spec.

Full test suite can be ran twice by doing two rspec calls with different config, and all tests must pass with this setting on and off, I can add that to github workflow.

And I can refactor it a bit to reduce the number of ifs in MultiSearchResult, if needed.