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

Suggestion text needs to be a string #24526

Merged
merged 3 commits into from Oct 24, 2018

Conversation

Bargs
Copy link
Contributor

@Bargs Bargs commented Oct 24, 2018

Addresses the issue @trevan brought up here

@trevan
Copy link
Contributor

trevan commented Oct 24, 2018

Just noticed that you can get duplicates in the recent searches which causes another complaint from React. This happens if you have a search that is an object in your history and then type in the same string. You now have two items in the history, one is an object, the other is a string, but after toUser(), they are both the same string.

I hope that makes sense.

the same query could both exist in the persisted log, resulting in
identical strings after toUser runs
@Bargs
Copy link
Contributor Author

Bargs commented Oct 24, 2018

Great catch, added deduplication as well

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

LGTM, one semi-related issue is that we probably don't want empty strings to be put in the recent search history, and I think that wasn't happening before.

image

@Bargs
Copy link
Contributor Author

Bargs commented Oct 24, 2018

Ah yeah that's pretty silly, and an easy fix, went ahead and changed it.

@Bargs Bargs merged commit e690991 into elastic:master Oct 24, 2018
Bargs added a commit to Bargs/kibana that referenced this pull request Oct 24, 2018
* suggestion text needs to be a string

* deduplicate after calling toUser since a string and object version of
the same query could both exist in the persisted log, resulting in
identical strings after toUser runs

* don't put empty strings in the recent search history
Bargs added a commit that referenced this pull request Oct 24, 2018
* suggestion text needs to be a string

* deduplicate after calling toUser since a string and object version of
the same query could both exist in the persisted log, resulting in
identical strings after toUser runs

* don't put empty strings in the recent search history
@Bargs
Copy link
Contributor Author

Bargs commented Oct 24, 2018

Forgot to mention, that previous failure was unrelated to this PR and the build before that passed.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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

Successfully merging this pull request may close these issues.

None yet

4 participants