Clean up time range handling in embeddables (#17718) (#18981)

* Convert all inner time range formats to utc for consistency

- pass down container state on render rather than rely on specific
function call orders.

* remove accidentally left in return jsdoc line
This commit is contained in:
Stacey Gammon 2018-05-10 16:42:53 -04:00 committed by GitHub
parent c5c8c631c0
commit bea616976a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 59 additions and 24 deletions

View file

@ -139,7 +139,7 @@ app.directive('dashboardApp', function ($injector) {
courier.fetch(...args);
};
$scope.timefilter = timefilter;
dashboardStateManager.handleTimeChange($scope.timefilter);
dashboardStateManager.handleTimeChange($scope.timefilter.time);
$scope.expandedPanel = null;
$scope.dashboardViewMode = dashboardStateManager.getViewMode();
@ -220,7 +220,7 @@ app.directive('dashboardApp', function ($injector) {
$scope.$watch('model.query', $scope.updateQueryAndFetch);
$scope.$listen(timefilter, 'fetch', () => {
dashboardStateManager.handleTimeChange($scope.timefilter);
dashboardStateManager.handleTimeChange($scope.timefilter.time);
// Currently discover relies on this logic to re-fetch. We need to refactor it to rely instead on the
// directly passed down time filter. Then we can get rid of this reliance on scope broadcasts.
$scope.refresh();

View file

@ -112,10 +112,19 @@ export class DashboardStateManager {
/**
* Time is part of global state so we need to deal with it outside of _pushAppStateChangesToStore.
* @param {Object} newTimeFilter
* @param {String|Object} newTimeFilter.to -- either a string representing an absolute time in utc format,
* or a relative time (now-15m), or a moment object
* @param {String|Object} newTimeFilter.from - either a string representing an absolute or a relative time, or a
* moment object
* @param {String} newTimeFilter.mode
*/
handleTimeChange(newTimeFilter) {
store.dispatch(updateTimeRange(newTimeFilter));
const timeFilter = {
from: FilterUtils.convertTimeToUTCString(newTimeFilter.from),
to: FilterUtils.convertTimeToUTCString(newTimeFilter.to),
mode: newTimeFilter.mode,
};
store.dispatch(updateTimeRange(timeFilter));
}
/**

View file

@ -54,12 +54,18 @@ export class FilterUtils {
}
/**
* Converts the time to a string, if it isn't already.
* Converts the time to a utc formatted string. If the time is not valid (e.g. it might be in a relative format like
* 'now-15m', then it just returns what it was passed).
* @param time {string|Moment}
* @returns {string}
* @returns {string} the time represented in utc format, or if the time range was not able to be parsed into a moment
* object, it returns the same object it was given.
*/
static convertTimeToString(time) {
return typeof time === 'string' ? time : moment(time).toString();
static convertTimeToUTCString(time) {
if (moment(time).isValid()) {
return moment(time).utc();
} else {
return time;
}
}
/**
@ -71,7 +77,7 @@ export class FilterUtils {
* @returns {boolean}
*/
static areTimesEqual(timeA, timeB) {
return this.convertTimeToString(timeA) === this.convertTimeToString(timeB);
return this.convertTimeToUTCString(timeA) === this.convertTimeToUTCString(timeB);
}
/**

View file

@ -9,10 +9,10 @@ export function updateSavedDashboard(savedDashboard, appState, timeFilter, toJso
savedDashboard.optionsJSON = toJson(appState.options);
savedDashboard.timeFrom = savedDashboard.timeRestore ?
FilterUtils.convertTimeToString(timeFilter.time.from)
FilterUtils.convertTimeToUTCString(timeFilter.time.from)
: undefined;
savedDashboard.timeTo = savedDashboard.timeRestore ?
FilterUtils.convertTimeToString(timeFilter.time.to)
FilterUtils.convertTimeToUTCString(timeFilter.time.to)
: undefined;
const timeRestoreObj = _.pick(timeFilter.refreshInterval, ['display', 'pause', 'section', 'value']);
savedDashboard.refreshInterval = savedDashboard.timeRestore ? timeRestoreObj : undefined;

View file

@ -35,8 +35,7 @@ export class DashboardPanel extends React.Component {
if (this.mounted) {
this.embeddable = embeddable;
embeddableIsInitialized(embeddable.metadata);
this.embeddable.onContainerStateChanged(this.props.containerState);
this.embeddable.render(this.panelElement);
this.embeddable.render(this.panelElement, this.props.containerState);
} else {
embeddable.destroy();
}

View file

@ -128,9 +128,10 @@ export const getHidePanelTitles = dashboard => dashboard.view.hidePanelTitles;
* @return {string|undefined}
*/
export const getMaximizedPanelId = dashboard => dashboard.view.maximizedPanelId;
/**
* @param dashboard {DashboardState}
* @return {string|undefined}
* @return {{ from: {string}, to: {string}, mode: {string} }}
*/
export const getTimeRange = dashboard => dashboard.view.timeRange;
@ -160,7 +161,8 @@ export const getDescription = dashboard => dashboard.metadata.description;
* This state object is specifically for communicating to embeddables and it's structure is not tied to
* the redux tree structure.
* @typedef {Object} ContainerState
* @property {Object} timeRange
* @property {String} timeRange.to - either an absolute time range in utc format or a relative one (e.g. now-15m)
* @property {String} timeRange.from - either an absolute time range in utc format or a relative one (e.g. now-15m)
* @property {Object} embeddableCustomization
* @property {boolean} hidePanelTitles
*/
@ -171,12 +173,18 @@ export const getDescription = dashboard => dashboard.metadata.description;
* @param panelId {string}
* @return {ContainerState}
*/
export const getContainerState = (dashboard, panelId) => ({
timeRange: _.cloneDeep(getTimeRange(dashboard)),
embeddableCustomization: _.cloneDeep(getEmbeddableCustomization(dashboard, panelId) || {}),
hidePanelTitles: getHidePanelTitles(dashboard),
customTitle: getPanel(dashboard, panelId).title,
});
export const getContainerState = (dashboard, panelId) => {
const time = getTimeRange(dashboard);
return {
timeRange: {
to: time.to,
from: time.from,
},
embeddableCustomization: _.cloneDeep(getEmbeddableCustomization(dashboard, panelId) || {}),
hidePanelTitles: getHidePanelTitles(dashboard),
customTitle: getPanel(dashboard, panelId).title,
};
};
/**
*

View file

@ -103,7 +103,13 @@ export class SearchEmbeddable extends Embeddable {
};
}
render(domNode) {
/**
*
* @param {Element} domNode
* @param {ContainerState} containerState
*/
render(domNode, containerState) {
this.onContainerStateChanged(containerState);
this.domNode = domNode;
this.initializeSearchScope();
this.searchInstance = this.$compile(searchTemplate)(this.searchScope);

View file

@ -93,7 +93,13 @@ export class VisualizeEmbeddable extends Embeddable {
}
}
render(domNode) {
/**
*
* @param {Element} domNode
* @param {ContainerState} containerState
*/
render(domNode, containerState) {
this.onContainerStateChanged(containerState);
this.domNode = domNode;
this.handler = this.loader.embedVisualizationWithSavedObject(
domNode,

View file

@ -41,8 +41,9 @@ export class Embeddable {
/**
* @param {Element} domNode - the dom node to mount the rendered embeddable on
* @param {ContainerState} containerState
*/
render(/*domNode*/) {}
render(/*domNode, containerState*/) {}
destroy() {}
}