Skip to content

Commit

Permalink
Merge pull request #3036 from weaveworks/3029-change-paused-at-timest…
Browse files Browse the repository at this point in the history
…amp-back-to-string

Change pausedAt format from moment() back to ISO string
  • Loading branch information
fbarl authored Jan 22, 2018
2 parents ea0b549 + 382b0c6 commit c56ff7e
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 45 deletions.
3 changes: 1 addition & 2 deletions client/app/scripts/components/node-details.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { clickCloseDetails, clickShowTopologyForNode } from '../actions/app-acti
import { brightenColor, getNeutralColor, getNodeColorDark } from '../utils/color-utils';
import { isGenericTable, isPropertyList } from '../utils/node-details-utils';
import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils';
import { timestampsEqual } from '../utils/time-utils';

import Overlay from './overlay';
import MatchedText from './matched-text';
Expand Down Expand Up @@ -307,7 +306,7 @@ class NodeDetails extends React.Component {
function mapStateToProps(state, ownProps) {
const currentTopologyId = state.get('currentTopologyId');
return {
transitioning: !timestampsEqual(state.get('pausedAt'), ownProps.timestamp),
transitioning: state.get('pausedAt') !== ownProps.timestamp,
nodeMatches: state.getIn(['searchNodeMatches', currentTopologyId, ownProps.id]),
nodes: state.get('nodes'),
selectedNodeId: state.get('selectedNodeId'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { appendTime } from '../node-details-health-link-item';

describe('NodeDetailsHealthLinkItem', () => {
describe('appendTime', () => {
const time = moment.unix(1496275200);
const time = '2017-06-01T00:00:00Z';
const timeUnix = moment(time).unix();

it('returns url for empty url or time', () => {
expect(appendTime('', time)).toEqual('');
Expand All @@ -15,10 +16,10 @@ describe('NodeDetailsHealthLinkItem', () => {

it('appends as json for cloud link', () => {
const url = appendTime('/prom/:orgid/notebook/new/%7B%22cells%22%3A%5B%7B%22queries%22%3A%5B%22go_goroutines%22%5D%7D%5D%7D', time);
expect(url).toContain(time.unix());
expect(url).toContain(timeUnix);

const payload = JSON.parse(decodeURIComponent(url.substr(url.indexOf('new/') + 4)));
expect(payload.time.queryEnd).toEqual(time.unix());
expect(payload.time.queryEnd).toEqual(timeUnix);
});

it('appends as GET parameter', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import moment from 'moment';
import { connect } from 'react-redux';

import NodeDetailsHealthItem from './node-details-health-item';
Expand All @@ -9,7 +10,7 @@ import { trackAnalyticsEvent } from '../../utils/tracking-utils';

/**
* @param {string} url
* @param {Moment} time
* @param {string} time
* @returns {string}
*/
export function appendTime(url, time) {
Expand All @@ -18,12 +19,13 @@ export function appendTime(url, time) {
// rudimentary check whether we have a cloud link
const cloudLinkPathEnd = 'notebook/new/';
const pos = url.indexOf(cloudLinkPathEnd);
const timeUnix = moment(time).unix();
if (pos !== -1) {
let payload;
const json = decodeURIComponent(url.substr(pos + cloudLinkPathEnd.length));
try {
payload = JSON.parse(json);
payload.time = { queryEnd: time.unix() };
payload.time = { queryEnd: timeUnix };
} catch (e) {
return url;
}
Expand All @@ -32,9 +34,9 @@ export function appendTime(url, time) {
}

if (url.indexOf('?') !== -1) {
return `${url}&time=${time.unix()}`;
return `${url}&time=${timeUnix}`;
}
return `${url}?time=${time.unix()}`;
return `${url}?time=${timeUnix}`;
}

class NodeDetailsHealthLinkItem extends React.Component {
Expand Down
14 changes: 4 additions & 10 deletions client/app/scripts/components/time-travel-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,13 @@ class TimeTravelWrapper extends React.Component {
constructor(props, context) {
super(props, context);

this.changeTimestamp = this.changeTimestamp.bind(this);
this.trackTimestampEdit = this.trackTimestampEdit.bind(this);
this.trackTimelinePanButtonClick = this.trackTimelinePanButtonClick.bind(this);
this.trackTimelineLabelClick = this.trackTimelineLabelClick.bind(this);
this.trackTimelineZoom = this.trackTimelineZoom.bind(this);
this.trackTimelinePan = this.trackTimelinePan.bind(this);
}

changeTimestamp(timestamp) {
this.props.jumpToTime(moment(timestamp).utc());
}

trackTimestampEdit() {
trackAnalyticsEvent('scope.time.timestamp.edit', {
layout: this.props.topologyViewMode,
Expand Down Expand Up @@ -82,7 +77,7 @@ class TimeTravelWrapper extends React.Component {
<TimeTravel
timestamp={this.props.timestamp}
earliestTimestamp={this.props.earliestTimestamp}
onChangeTimestamp={this.changeTimestamp}
onChangeTimestamp={this.props.jumpToTime}
onTimestampInputEdit={this.trackTimestampEdit}
onTimelinePanButtonClick={this.trackTimelinePanButtonClick}
onTimelineLabelClick={this.trackTimelineLabelClick}
Expand All @@ -96,23 +91,22 @@ class TimeTravelWrapper extends React.Component {

function mapStateToProps(state, { params }) {
const scopeState = state.scope || state;
const pausedAt = scopeState.get('pausedAt');
let firstSeenConnectedAt;

// If we're in the Weave Cloud context, use firstSeeConnectedAt as the earliest timestamp.
if (state.root && state.root.instances) {
const serviceInstance = state.root.instances[params && params.orgId];
if (serviceInstance && serviceInstance.firstSeenConnectedAt) {
firstSeenConnectedAt = moment(serviceInstance.firstSeenConnectedAt);
firstSeenConnectedAt = moment(serviceInstance.firstSeenConnectedAt).utc().format();
}
}

return {
visible: scopeState.get('showingTimeTravel'),
topologyViewMode: scopeState.get('topologyViewMode'),
currentTopology: scopeState.get('currentTopology'),
earliestTimestamp: firstSeenConnectedAt && firstSeenConnectedAt.utc().format(),
timestamp: pausedAt && pausedAt.utc().format(),
earliestTimestamp: firstSeenConnectedAt,
timestamp: scopeState.get('pausedAt'),
};
}

Expand Down
10 changes: 4 additions & 6 deletions client/app/scripts/reducers/root.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import {
} from '../selectors/topology';
import { isPausedSelector } from '../selectors/time-travel';
import { activeTopologyZoomCacheKeyPathSelector } from '../selectors/zooming';
import { timestampsEqual } from '../utils/time-utils';
import { applyPinnedSearches } from '../utils/search-utils';
import { deserializeTimestamp } from '../utils/web-api-utils';
import {
findTopologyById,
setTopologyUrlsById,
Expand Down Expand Up @@ -381,13 +379,13 @@ export function rootReducer(state = initialState, action) {
case ActionTypes.PAUSE_TIME_AT_NOW: {
state = state.set('showingTimeTravel', false);
state = state.set('timeTravelTransitioning', false);
return state.set('pausedAt', moment().utc());
return state.set('pausedAt', moment().utc().format());
}

case ActionTypes.START_TIME_TRAVEL: {
state = state.set('showingTimeTravel', true);
state = state.set('timeTravelTransitioning', false);
return state.set('pausedAt', action.timestamp || moment().utc());
return state.set('pausedAt', action.timestamp || moment().utc().format());
}

case ActionTypes.JUMP_TO_TIME: {
Expand Down Expand Up @@ -555,7 +553,7 @@ export function rootReducer(state = initialState, action) {
case ActionTypes.RECEIVE_NODE_DETAILS: {
// Ignore the update if paused and the timestamp didn't change.
const setTimestamp = state.getIn(['nodeDetails', action.details.id, 'timestamp']);
if (isPausedSelector(state) && timestampsEqual(action.requestTimestamp, setTimestamp)) {
if (isPausedSelector(state) && action.requestTimestamp === setTimestamp) {
return state;
}

Expand Down Expand Up @@ -695,7 +693,7 @@ export function rootReducer(state = initialState, action) {
});
state = state.set('topologyViewMode', action.state.topologyViewMode);
if (action.state.pausedAt) {
state = state.set('pausedAt', deserializeTimestamp(action.state.pausedAt));
state = state.set('pausedAt', action.state.pausedAt);
}
if (action.state.gridSortedBy) {
state = state.set('gridSortedBy', action.state.gridSortedBy);
Expand Down
3 changes: 1 addition & 2 deletions client/app/scripts/utils/__tests__/web-api-utils-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import moment from 'moment';
import { Map as makeMap, OrderedMap as makeOrderedMap } from 'immutable';

import { buildUrlQuery, basePath, getApiPath, getWebsocketUrl } from '../web-api-utils';
Expand Down Expand Up @@ -38,7 +37,7 @@ describe('WebApiUtils', () => {
});

it('should combine multiple options with a timestamp', () => {
state = state.set('pausedAt', moment('2015-06-14T21:12:05.275Z'));
state = state.set('pausedAt', '2015-06-14T21:12:05.275Z');
expect(buildUrlQuery(makeOrderedMap([
['foo', 2],
['bar', 4]
Expand Down
3 changes: 1 addition & 2 deletions client/app/scripts/utils/router-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { each } from 'lodash';

import { route } from '../actions/app-actions';
import { storageGet, storageSet } from './storage-utils';
import { serializeTimestamp } from './web-api-utils';

//
// page.js won't match the routes below if ":state" has a slash in it, so replace those before we
Expand Down Expand Up @@ -51,7 +50,7 @@ export function getUrlState(state) {
const urlState = {
controlPipe: cp ? cp.toJS() : null,
nodeDetails: nodeDetails.toJS(),
pausedAt: serializeTimestamp(state.get('pausedAt')),
pausedAt: state.get('pausedAt'),
topologyViewMode: state.get('topologyViewMode'),
pinnedMetricType: state.get('pinnedMetricType'),
pinnedSearches: state.get('pinnedSearches').toJS(),
Expand Down
6 changes: 0 additions & 6 deletions client/app/scripts/utils/time-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,3 @@ export function timer(fn) {
};
return timedFn;
}

export function timestampsEqual(timestampA, timestampB) {
const stringifiedTimestampA = timestampA ? timestampA.toISOString() : '';
const stringifiedTimestampB = timestampB ? timestampB.toISOString() : '';
return stringifiedTimestampA === stringifiedTimestampB;
}
11 changes: 1 addition & 10 deletions client/app/scripts/utils/web-api-utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import debug from 'debug';
import moment from 'moment';
import reqwest from 'reqwest';
import { defaults } from 'lodash';
import { Map as makeMap, List } from 'immutable';
Expand Down Expand Up @@ -50,17 +49,9 @@ let firstMessageOnWebsocketAt = null;
let continuePolling = true;


export function serializeTimestamp(timestamp) {
return timestamp ? timestamp.toISOString() : null;
}

export function deserializeTimestamp(str) {
return str ? moment(str) : null;
}

export function buildUrlQuery(params = makeMap(), state) {
// Attach the time travel timestamp to every request to the backend.
params = params.set('timestamp', serializeTimestamp(state.get('pausedAt')));
params = params.set('timestamp', state.get('pausedAt'));

// Ignore the entries with values `null` or `undefined`.
return params.map((value, param) => {
Expand Down

0 comments on commit c56ff7e

Please sign in to comment.