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

Fix visualization individual timeranges #17123

Merged
merged 12 commits into from
Mar 25, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,12 @@ Using 'none' as your request handles means your visualization does not require a

[[development-custom-request-handler]]
==== custom request handler
You can define your custom request handler by providing a function with the following definition:
`function (vis, appState, uiState, searchSource) { ... }`
You can define your custom request handler by providing a function with the following signature:
`function (vis, { uiState, appState, timeRange }) { ... }`

The `timeRange` will be an object with a `from` and `to` key, that can contain
datemath expressions, like `now-7d`. You can use the `datemath` library to parse
them.

This function must return a promise, which should get resolved with new data that will be passed to responseHandler.

Expand All @@ -306,7 +310,7 @@ It's up to function to decide when it wants to issue a new request or return pre
-----------
import { VisFactoryProvider } from 'ui/vis/vis_factory';

const myRequestHandler = async (vis, appState, uiState, searchSource) => {
const myRequestHandler = async (vis, { appState, uiState, timeRange }) => {
const data = ... parse ...
return data;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,10 @@ function discoverController(
return $scope.vis.getAggConfig().toDsl();
});
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the date histogram in the chart looks into this variable expected to be set by visualize.js we need to set it explicitly in the places we use <visualization> directly. This is currently only discover. I think in the long run with the new chart component in EUI we should rather use the chart component directly and not any part of the visualize stack anymore in Discover, since discover and visualize already have a ton of special handling just for this one chart.

$scope.vis.filters = {
timeRange: timefilter.time
};
}

function resolveIndexPatternLoading() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
app-state="state"
editor-mode="chrome.getVisible()"
show-spy-panel="chrome.getVisible()"
time-range="timeRange"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pass down the timeRange explicitly from the editor app.

>

</visualize>
Expand Down
8 changes: 8 additions & 0 deletions src/core_plugins/kibana/public/visualize/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ function VisEditor($scope, $route, timefilter, AppState, $window, kbnUrl, courie
$scope.isAddToDashMode = () => addToDashMode;

$scope.timefilter = timefilter;
$scope.timeRange = timefilter.time;
$scope.opts = _.pick($scope, 'doSave', 'savedVis', 'shareData', 'timefilter', 'isAddToDashMode');

stateMonitor = stateMonitorFactory.create($state, stateDefaults);
Expand Down Expand Up @@ -216,6 +217,12 @@ function VisEditor($scope, $route, timefilter, AppState, $window, kbnUrl, courie
}
});

const updateTimeRange = () => {
$scope.timeRange = timefilter.time;
};

timefilter.on('update', updateTimeRange);

// update the searchSource when filters update
$scope.$listen(queryFilter, 'update', function () {
$state.save();
Expand All @@ -230,6 +237,7 @@ function VisEditor($scope, $route, timefilter, AppState, $window, kbnUrl, courie
$scope.$on('$destroy', function () {
savedVis.destroy();
stateMonitor.destroy();
timefilter.off('update', updateTimeRange);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,25 @@ const MetricsRequestHandlerProvider = function (Private, Notifier, config, timef

return {
name: 'metrics',
handler: function (vis, appState, uiState) {
handler: function (vis, { uiState, timeRange }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make TSVB request handler only use the passed timeRange and not looking into timefilter directly anymore.

const timezone = Private(timezoneProvider)();
return new Promise((resolve) => {
const panel = vis.params;
const uiStateObj = uiState.get(panel.type, {});
const timeRange = vis.params.timeRange || timefilter.getBounds();
const parsedTimeRange = timefilter.calculateBounds(timeRange);
const scaledDataFormat = config.get('dateFormat:scaled');
const dateFormat = config.get('dateFormat');
if (panel && panel.id) {
const params = {
timerange: { timezone, ...timeRange },
timerange: { timezone, ...parsedTimeRange },
filters: [dashboardContext()],
panels: [panel],
state: uiStateObj
};

try {
const maxBuckets = config.get('metrics:max_buckets');
validateInterval(timeRange, panel, maxBuckets);
validateInterval(parsedTimeRange, panel, maxBuckets);
const httpResult = $http.post('../api/metrics/vis/data', params)
.then(resp => ({ dateFormat, scaledDataFormat, timezone, ...resp.data }))
.catch(resp => { throw resp.data; });
Expand Down
14 changes: 3 additions & 11 deletions src/core_plugins/timelion/public/vis/timelion_request_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _ from 'lodash';
import { dashboardContextProvider } from 'plugins/kibana/dashboard/dashboard_context';

import { timezoneProvider } from 'ui/vis/lib/timezone';
const TimelionRequestHandlerProvider = function (Private, Notifier, $http, $rootScope, timefilter) {
const TimelionRequestHandlerProvider = function (Private, Notifier, $http) {
const timezone = Private(timezoneProvider)();
const dashboardContext = Private(dashboardContextProvider);

Expand All @@ -12,28 +12,20 @@ const TimelionRequestHandlerProvider = function (Private, Notifier, $http, $root

return {
name: 'timelion',
handler: function (vis /*, appState, uiState, queryFilter*/) {
handler: function (vis, { timeRange }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make timelion request handler only use the explicitly passed timeRange instead of accessing timefilter.time directly.


return new Promise((resolve, reject) => {
const expression = vis.params.expression;
if (!expression) return;

let timeFilter = timefilter.time;
if (vis.params.timeRange) {
timeFilter = {
mode: 'absolute',
from: vis.params.timeRange.min.toJSON(),
to: vis.params.timeRange.max.toJSON()
};
}
const httpResult = $http.post('../api/timelion/run', {
sheet: [expression],
extended: {
es: {
filter: dashboardContext()
}
},
time: _.extend(timeFilter, {
time: _.extend(timeRange, {
interval: vis.params.interval,
timezone: timezone
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe(`TimeCache`, () => {
this._max = max;
}

getBounds() {
calculateBounds() {
this._accessCount++;
return {
min: { valueOf: () => this._min },
Expand Down
6 changes: 5 additions & 1 deletion src/core_plugins/vega/public/data_model/time_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,17 @@ export class TimeCache {
return this._cachedBounds;
}

setTimeRange(timeRange) {
this._timeRange = timeRange;
}

/**
* Get parsed min/max values
* @returns {{min: number, max: number}}
* @private
*/
_getBounds() {
const bounds = this._timefilter.getBounds();
const bounds = this._timefilter.calculateBounds(this._timeRange);
return {
min: bounds.min.valueOf(),
max: bounds.max.valueOf()
Expand Down
3 changes: 2 additions & 1 deletion src/core_plugins/vega/public/vega_request_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export function VegaRequestHandlerProvider(Private, es, timefilter, serviceSetti

name: 'vega',

handler(vis) {
handler(vis, { timeRange }) {
timeCache.setTimeRange(timeRange);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pass in the timeRange that the vega request handler got into the timecache, that is used.

const vp = new VegaParser(vis.params.spec, searchCache, timeCache, dashboardContext, serviceSettings);
return vp.parseAsync();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ describe('params', function () {
setTimeBounds = function (n, units) {
timefilter.enableAutoRefreshSelector();
timefilter.enableTimeRangeSelector();
timefilter.getBounds = _.constant({
min: now.clone().subtract(n, units),
max: now.clone()
});
paramWriter.vis.filters = {
timeRange: {
from: now.clone().subtract(n, units),
to: now.clone()
}
};
};
}));

Expand Down
7 changes: 2 additions & 5 deletions src/ui/public/agg_types/buckets/date_histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,9 @@ export function AggTypesBucketsDateHistogramProvider(timefilter, config, Private
}

function getBounds(vis) {
if (!vis.getTimeRange) {
return timefilter.getActiveBounds();
if (timefilter.isTimeRangeSelectorEnabled && vis.filters) {
return timefilter.calculateBounds(vis.filters.timeRange);
}

const timeRange = vis.getTimeRange();
return timefilter.calculateBounds(timeRange);
}

function setBounds(agg, force) {
Expand Down
42 changes: 39 additions & 3 deletions src/ui/public/vis/request_handlers/courier.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,46 @@ import { VisRequestHandlersRegistryProvider } from 'ui/registry/vis_request_hand
const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
const SearchSource = Private(SearchSourceProvider);

/**
* TODO: This code can be removed as soon as we got rid of inheritance in the
* searchsource and pass down every filter explicitly.
* we're only adding one range filter against the timeFieldName to ensure
* that our filter is the only one applied and override the global filters.
* this does rely on the "implementation detail" that filters are added first
* on the leaf SearchSource and subsequently on the parents.
*/
function removeSearchSourceParentTimefilter(searchSource) {
searchSource.addFilterPredicate((filter, state) => {
if (!filter.range) {
return true;
}

const index = searchSource.index() || searchSource.getParent().index();
const timeFieldName = index.timeFieldName;
if (!timeFieldName) {
return true;
}

// Only check if we need to filter out this filter if it's actual a range filter
// on our time field and not any other field.
if (!filter.range[timeFieldName]) {
return true;
}

return !(state.filters || []).find(f => f.range && f.range[timeFieldName]);
});

}

return {
name: 'courier',
handler: function (vis, appState, uiState, queryFilter, searchSource) {
handler: function (vis, { appState, queryFilter, searchSource, timeRange }) {

searchSource.filter(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move applying the timeRange filter on searchSource into the actual courier request handler.

In the long run we want to remove searchSource altogether from visualize, since this is courier request handler specific.

return timefilter.get(searchSource.index(), timeRange);
});

removeSearchSourceParentTimefilter(searchSource, timeRange);

if (queryFilter && vis.editorMode) {
searchSource.set('filter', queryFilter.getFilters());
Expand All @@ -31,7 +67,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
if (!_.isEqual(_.cloneDeep(searchSource.get('filter')), searchSource.lastQuery.filter)) return true;
if (!_.isEqual(_.cloneDeep(searchSource.get('query')), searchSource.lastQuery.query)) return true;
if (!_.isEqual(_.cloneDeep(copyAggs(vis.aggs)), searchSource.lastQuery.aggs)) return true;
if (!_.isEqual(_.cloneDeep(timefilter.time), searchSource.lastQuery.time)) return true;
if (!_.isEqual(_.cloneDeep(timeRange), searchSource.lastQuery.timeRange)) return true;

return false;
};
Expand All @@ -44,7 +80,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
filter: _.cloneDeep(searchSource.get('filter')),
query: _.cloneDeep(searchSource.get('query')),
aggs: _.cloneDeep(copyAggs(vis.aggs)),
time: _.cloneDeep(timefilter.time)
timeRange: _.cloneDeep(timeRange)
};

searchSource.rawResponse = resp;
Expand Down
45 changes: 18 additions & 27 deletions src/ui/public/visualize/visualize.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,32 +59,6 @@ uiModules

$scope.vis.description = $scope.savedObj.description;

if ($scope.timeRange) {
$scope.vis.getTimeRange = () => $scope.timeRange;

const searchSource = $scope.savedObj.searchSource;
searchSource.filter(() => {
return timefilter.get(searchSource.index(), $scope.timeRange);
});

// we're only adding one range filter against the timeFieldName to ensure
// that our filter is the only one applied and override the global filters.
// this does rely on the "implementation detail" that filters are added first
// on the leaf SearchSource and subsequently on the parents
searchSource.addFilterPredicate((filter, state) => {
if (!filter.range) {
return true;
}

const timeFieldName = searchSource.index().timeFieldName;
if (!timeFieldName) {
return true;
}

return !(state.filters || []).find(f => f.range && f.range[timeFieldName]);
});
}

$scope.editorMode = $scope.editorMode || false;
$scope.vis.editorMode = $scope.editorMode;

Expand All @@ -96,8 +70,25 @@ uiModules
// was still waiting for its debounce, in this case we don't want to start
// fetching new data and rendering.
if (!$scope.vis.initialized || !$scope.savedObj || destroyed) return;

// TODO: This should ALWAYS be passed into this component via the loader
// in the future. Unfortunately we need some refactoring in dashboard
// to make this working and correctly rerender, so for now we will either
// use the one passed in to us or look into the timefilter ourselfs (which
// will be removed in the future).
const timeRange = $scope.timeRange || timefilter.time;

$scope.vis.filters = { timeRange };

const handlerParams = {
appState: $scope.appState,
uiState: $scope.uiState,
queryFilter: queryFilter,
searchSource: $scope.savedObj.searchSource,
timeRange: timeRange,
};
// searchSource is only there for courier request handler
requestHandler($scope.vis, $scope.appState, $scope.uiState, queryFilter, $scope.savedObj.searchSource)
requestHandler($scope.vis, handlerParams)
.then(requestHandlerResponse => {

//No need to call the response handler when there have been no data nor has been there changes
Expand Down