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

Avoid remove previously added params when adding a suggest to the query #726

Merged
merged 2 commits into from
Nov 20, 2014

Conversation

agallou
Copy link
Contributor

@agallou agallou commented Nov 19, 2014

When we add a suggest to the query (while wanting to keep the query part
of the query), the query and all the previoulsy added parameters where
removed.

Like in the unit test, if we add the query (or the size) then the suggest,
the query (or the size) is removed. But if we add the suggest then the query
(or the size), the query is still present.

So insted of removing all the params, we merge the suggest with the
existing parameters.

When we add a suggest to the query (while wanting to keep the query part
of the query), the query and all the previoulsy added parameters where
removed.

Like in the unit test, if we add the query (or the size) then the suggest,
the query (or the size) is removed. But if we add the suggest then the query
(or the size), the query is still present.

So insted of removing all the params, we merge the suggest with the
existing parameters.
@ruflin ruflin added the bug label Nov 19, 2014
@ruflin
Copy link
Owner

ruflin commented Nov 19, 2014

Can you update the changes.txt?

@agallou
Copy link
Contributor Author

agallou commented Nov 19, 2014

changes.txt updated

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 147c0a9 on agallou:suggest_query into 5fe627f on ruflin:master.

ruflin added a commit that referenced this pull request Nov 20, 2014
Avoid remove previously added params when adding a suggest to the query
@ruflin ruflin merged commit a780fe6 into ruflin:master Nov 20, 2014
@ruflin
Copy link
Owner

ruflin commented Nov 20, 2014

Thx. Merged.

Interesting that the coverage went down even though a test was added :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants