Skip to content

Fix bug: Search optional params are not applied when they are a list of strings #87

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

Merged
merged 2 commits into from
Jun 2, 2020

Conversation

eskombro
Copy link
Member

@eskombro eskombro commented Jun 1, 2020

When a search parameter like attributesToHighlight or attributesToRetrieve are given as a list of strings, they should be encoded as a comma separated string into the url:

attributesToRetrieve = ["some", "string"]

should become

... &attributesToRetrieve=some,string ...

This encoding is not the default behaviour of the url lib encoding and it was introducing the [ and ] characters in the url (and some +) so MeiliSearch was ignoring those parameters.

Before encoding this parameters, I added a few lines:

for key in opt_params:
    if isinstance(opt_params[key], list):
        opt_params[key] = ",".join(opt_params[key])

this will transform the lists of strings in the expected format.

A test was added to the meilisearch/tests/index/test_index_document_meilisearch.py file to check this behavior

Closes #85

@eskombro eskombro marked this pull request as ready for review June 1, 2020 15:43
@eskombro eskombro requested a review from a team June 1, 2020 15:43
@eskombro eskombro self-assigned this Jun 1, 2020
@eskombro eskombro force-pushed the fix_search_params_can_be_lists branch from dc84075 to 73559b8 Compare June 1, 2020 16:01
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Top!
Could you put single quotes everwhere to stay consistent? 🙂

@curquiza curquiza requested a review from bidoubiwa June 2, 2020 12:17
@eskombro
Copy link
Member Author

eskombro commented Jun 2, 2020

Done @curquiza ! (and for all the tests too)

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

👍

@eskombro eskombro merged commit 5233c97 into master Jun 2, 2020
@eskombro eskombro deleted the fix_search_params_can_be_lists branch June 2, 2020 14:54
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.

Search parametters don't accept lists of strings (and should)
2 participants