Skip to content

Update existing filters when negated via a visualization - #10778

Merged
Bargs merged 2 commits into
elastic:masterfrom
Bargs:inversionFilters
Mar 22, 2017
Merged

Bargs merged 2 commits into
elastic:masterfrom
Bargs:inversionFilters

Conversation

@Bargs

@Bargs Bargs commented Mar 16, 2017

Copy link
Copy Markdown
Contributor

Fixes #10769

Negating an existing filter via the data table wasn't working. It works in the discover doc table because it uses the filter_manager's add method, which does some of its own existence checks and inverts a filter if it already exists. The filter_bar_click_handler, which handles creation of filters in visualizations, uses the filter_bar module directly. So I added some generic existence checks to the filter_bar itself and added code to invert existing filters if necessary.

This should solve the problem for any new visualizations that might create negated filters in the future.

@Bargs

Bargs commented Mar 16, 2017

Copy link
Copy Markdown
Contributor Author

Tests passed, upload of artifacts to s3 just failed

@Bargs

Bargs commented Mar 20, 2017

Copy link
Copy Markdown
Contributor Author

@epixa should we try to get this into 5.3? It seems like we might have time, and I feel like it's a relatively bad bug for a new feature

@epixa

epixa commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

Works for me

@lukasolson lukasolson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works as expected now. LGTM

@epixa

epixa commented Mar 22, 2017

Copy link
Copy Markdown
Contributor

jenkins, test this

@Bargs
Bargs removed the request for review from weltenwort March 22, 2017 16:43
@Bargs
Bargs merged commit 4dd02e3 into elastic:master Mar 22, 2017
elastic-jasper added a commit that referenced this pull request Mar 22, 2017
Backports PR #10778

**Commit 1:**
Handle single filter scenario

* Original sha: ccd24cb
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:03:42Z

**Commit 2:**
Handle the multi filter scenario

* Original sha: c90535d
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:26:28Z
elastic-jasper added a commit that referenced this pull request Mar 22, 2017
Backports PR #10778

**Commit 1:**
Handle single filter scenario

* Original sha: ccd24cb
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:03:42Z

**Commit 2:**
Handle the multi filter scenario

* Original sha: c90535d
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:26:28Z
Bargs pushed a commit that referenced this pull request Mar 22, 2017
Backports PR #10778

**Commit 1:**
Handle single filter scenario

* Original sha: ccd24cb
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:03:42Z

**Commit 2:**
Handle the multi filter scenario

* Original sha: c90535d
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:26:28Z
Bargs pushed a commit that referenced this pull request Mar 22, 2017
Backports PR #10778

**Commit 1:**
Handle single filter scenario

* Original sha: ccd24cb
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:03:42Z

**Commit 2:**
Handle the multi filter scenario

* Original sha: c90535d
* Authored by Matthew Bargar <mbargar@gmail.com> on 2017-03-16T21:26:28Z
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
Negating an existing filter via the data table wasn't working. It works in the discover doc table because it uses the filter_manager's add method, which does some of its own existence checks and inverts a filter if it already exists. The filter_bar_click_handler, which handles creation of filters in visualizations, uses the filter_bar module directly. So I added some generic existence checks to the filter_bar itself and added code to invert existing filters if necessary.

This should solve the problem for any new visualizations that might create negated filters in the future.

Fixes elastic#10769
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.

4 participants