Skip to content

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

Merged
eskombro merged 2 commits into
masterfrom
fix_search_params_can_be_lists
Jun 2, 2020
Merged

Fix bug: Search optional params are not applied when they are a list of strings#87
eskombro merged 2 commits into
masterfrom
fix_search_params_can_be_lists

Conversation

@eskombro

@eskombro eskombro commented Jun 1, 2020

Copy link
Copy Markdown
Contributor

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

@curquiza curquiza left a comment

Copy link
Copy Markdown
Member

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

eskombro commented Jun 2, 2020

Copy link
Copy Markdown
Contributor Author

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

@curquiza curquiza left a comment

Copy link
Copy Markdown
Member

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