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

Visualization specific timeranges #15998

Merged
merged 10 commits into from
Jan 17, 2018
Merged

Visualization specific timeranges #15998

merged 10 commits into from
Jan 17, 2018

Conversation

kobelb
Copy link
Contributor

@kobelb kobelb commented Jan 11, 2018

The previous approach to visualization specific timeranges had a few deficiencies:

  • Breaking the SearchSource inheritance "chain" to ignore the global time filter also ended up ignoring the pinned filters
  • When a timeRange was set for specific visualization, it couldn't be updated over time.

This PR changes the approach, and we're no longer breaking the SearchSource inheritance. Instead, I've made the "filter filtering" a first class citizen of the SearchSource and added an extension point so that specific SearchSource instances are able to reject certain filters. This allows us to ensure that only one range filter is added to the SearchSource, and ignore the global time filter that was being added.

Additionally, we're no longer calculating the absolute min/max times from the timeRange when first "linking" the Visualization so that these values are able to change over time. The way that we're passing the custom timeRange to the DateHistogramAggregation has been revised as well to alleviate the same limitation of the values changing over time.

@kobelb kobelb added Feature:Dashboard Dashboard related features Feature:Visualizations Generic visualization features (in case no more specific feature label is available) :Sharing labels Jan 11, 2018
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

Looks great. How about some search source tests that filter out some filters

@kobelb
Copy link
Contributor Author

kobelb commented Jan 12, 2018

Looks great. How about some search source tests that filter out some filters

Good point, I'll add some unit tests for the _mergeProp function.

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

nice! LGTM

@kobelb kobelb merged commit cd5336f into elastic:master Jan 17, 2018
@kobelb kobelb deleted the timeranges branch January 17, 2018 16:52
kobelb added a commit to kobelb/kibana that referenced this pull request Jan 17, 2018
* No longer converting the $scope.timeRange to absolute values in
visualize.js

* Now using searchSource.filter as opposed to manual approach

* Adding extensible filters and only including the first timeRange filter

* Removing the forceNow stuff, we'll deal with this in another PR

* Adding omment about removing null/undefined filters

* Adding vis.getTimeRange and using it in the Date Histogram

* Removing the test hard-coded timeRange

* Adding tests for default filter "filters"

* Adding custom filter prediate test
@kobelb kobelb added the non-issue Indicates to automation that a pull request should not appear in the release notes label Jan 17, 2018
kobelb added a commit that referenced this pull request Jan 17, 2018
* No longer converting the $scope.timeRange to absolute values in
visualize.js

* Now using searchSource.filter as opposed to manual approach

* Adding extensible filters and only including the first timeRange filter

* Removing the forceNow stuff, we'll deal with this in another PR

* Adding omment about removing null/undefined filters

* Adding vis.getTimeRange and using it in the Date Histogram

* Removing the test hard-coded timeRange

* Adding tests for default filter "filters"

* Adding custom filter prediate test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features Feature:Visualizations Generic visualization features (in case no more specific feature label is available) non-issue Indicates to automation that a pull request should not appear in the release notes v6.2.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants