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

Controls vis - safely handle case where value can not be extracted from Kibana filter #22885

Merged
merged 2 commits into from
Sep 10, 2018

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Sep 10, 2018

fixes #22607

The original error can be created with the following steps:

  1. Create a new Controls visualization with an Options list control. Set index-pattern and field.
  2. Interact with the options list control and select a value and click Apply changes so a filter pill is added to Kibana.
  3. Change the field for the options list control. When the new field is applied then the error will be shown.

This resulted in passing [{value: undefined, label: undefined}] to EuiComboBox for the selectedOptions props.

This regression was introduced by PR #20002

@nreese nreese added bug Fixes for quality problems that affect the customer experience regression v7.0.0 v6.5.0 v6.4.1 labels Sep 10, 2018
@nreese nreese added :Sharing Feature:Input Control Input controls visualization labels Sep 10, 2018
@nreese nreese changed the title handle case where value can not be extracted from Kibana filter Controls vis - safely handle case where value can not be extracted from Kibana filter Sep 10, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@chrisdavies chrisdavies left a comment

Choose a reason for hiding this comment

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

LTGM w/ the tweaks I mentioned.

I pulled it down and tested in both master and this branch. Master reproduced the error, this branch did not. 🎉 🎂

.map((kbnFilter) => {
return this._getValueFromFilter(kbnFilter);
})
.filter(value => {
if (typeof value === 'undefined' || value == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The entire filter could become this: .filter(value => value != null), if you want to keep the != instead of !==. Alternatively, it could be: .filter(value => typeof value !== 'undefined' && value !== null).

return true;
});

if (values.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This if is unnecessary, as the filter will mean that reduce will be operating over an empty array in the test case, which should be just fie.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the filter removes all items from the array then reduce would return []. The function should return undefined if there are no selected options.

@@ -76,7 +76,10 @@ export class RangeFilterManager extends FilterManager {
range = _.get(kbnFilters[0], ['range', this.fieldName]);
}

return fromRange(range);
if (range == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double-checking that you wanted a == instead of a ===. Might want to slap a comment in here, if you do, as this is usually frowned upon, but it is a handy way to check for null and undefined simultaneously.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese nreese merged commit 9555b8f into elastic:master Sep 10, 2018
nreese added a commit to nreese/kibana that referenced this pull request Sep 10, 2018
…om Kibana filter (elastic#22885)

* handle case where value can not be extracted from Kibana filter

* review feedback
nreese added a commit to nreese/kibana that referenced this pull request Sep 10, 2018
…om Kibana filter (elastic#22885)

* handle case where value can not be extracted from Kibana filter

* review feedback
@Shifter2600
Copy link

Shifter2600 commented Sep 10, 2018

Great can't wait to use the new version and test this out in my system that errors currently. I use Docker Kibana 6.4 currently and can pull a new version when this is ready.

nreese added a commit that referenced this pull request Sep 10, 2018
…om Kibana filter (#22885) (#22900)

* handle case where value can not be extracted from Kibana filter

* review feedback
nreese added a commit that referenced this pull request Sep 10, 2018
…om Kibana filter (#22885) (#22899)

* handle case where value can not be extracted from Kibana filter

* review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Input Control Input controls visualization regression v6.4.1 v6.5.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Controls visualization passing undefined to EuiComboBox's selectedOptions prop resulting in fatal kibana error
4 participants