Skip to content

Commit

Permalink
Fix incorrect dates at range ends
Browse files Browse the repository at this point in the history
Because of a mistake, the end date of time ranges as shown in the
dashboard was not copied by value but by reference. As an effect,
displayed dates would be repeatedly decremented by one second every time
users hovered data points. For this reason, displayed dates could vary
when hovering them multiple times.

This is easily fixed by copying the affected variable by value. Multiple
test cases ensure that no off-by-one errors or local time zone issues
occur.
  • Loading branch information
pluehne authored and larsxschneider committed Apr 11, 2018
1 parent 7eceacd commit 9366cee
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 7 deletions.
19 changes: 13 additions & 6 deletions docs/assets/js/charts.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,18 @@ function formatDate(date)
return moment(date).utc().format('D MMM YYYY');
}

function formatDateRange(dateRange)
{
const t0 = dateRange[0];
let t1 = new Date(dateRange[1].valueOf());

// The date range is of the form [t0, t1), inclusive of t0 but exclusive of t1.
// Hence, subtract one second from t1 to obtain the previous date in UTC
t1.setSeconds(t1.getSeconds() - 1);

return formatDate(t0) + ' to ' + formatDate(t1);
}

function hasConfig(element, key)
{
return element.data('config') && (key in element.data('config'));
Expand Down Expand Up @@ -427,12 +439,7 @@ function createHistoryChart(canvas, actionBar)
if (dateRange[0] == dateRange[1])
return formatDate(dateRange[0]) + suffix;

// The date range is of the form [t0, t1), inclusive of t0 but exclusive of t1.
// Hence, subtract one second from t1 to obtain the previous date in UTC
let t1 = dateRange[1];
t1.setSeconds(t1.getSeconds() - 1);

return formatDate(dateRange[0]) + ' to ' + formatDate(t1) + suffix;
return formatDateRange(dateRange) + suffix;
};

let options = jQuery.extend({}, timeSeriesChartDefaults);
Expand Down
4 changes: 3 additions & 1 deletion docs/spec/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
"jquery": true
},
"globals": {
"d3": false
"d3": false,
"moment": false,
"Date": false
}
}
66 changes: 66 additions & 0 deletions docs/spec/charts.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
createList,
createTable,
createSpinner,
formatDate,
formatDateRange,
*/

describe('global charts.js', function()
Expand Down Expand Up @@ -197,5 +199,69 @@ describe('global charts.js', function()
expect(aggregatedData[10]['value']).toEqual(4);
expect(aggregatedData[11]['value']).toEqual(4);
});

it('should format dates correctly', function()
{
// The local time zone should not affect the displayed dates, which are to be interpreted as UTC
const testCases =
[
{localTimeZone: 'Etc/GMT-1', date: '2018-12-31T00:00:00.000Z', expectedResult: '31 Dec 2018'},
{localTimeZone: 'Etc/GMT-1', date: '2018-12-31T23:59:59.000Z', expectedResult: '31 Dec 2018'},
{localTimeZone: 'Etc/GMT-1', date: '2019-01-01T00:30:00.000Z', expectedResult: '1 Jan 2019'},
{localTimeZone: 'Etc/GMT', date: '2018-12-31T00:00:00.000Z', expectedResult: '31 Dec 2018'},
{localTimeZone: 'Etc/GMT', date: '2018-12-31T23:59:59.000Z', expectedResult: '31 Dec 2018'},
{localTimeZone: 'Etc/GMT', date: '2019-01-01T00:30:00.000Z', expectedResult: '1 Jan 2019'},
{localTimeZone: 'Etc/GMT+1', date: '2018-12-31T00:00:00.000Z', expectedResult: '31 Dec 2018'},
{localTimeZone: 'Etc/GMT+1', date: '2018-12-31T23:59:59.000Z', expectedResult: '31 Dec 2018'},
{localTimeZone: 'Etc/GMT+1', date: '2019-01-01T00:30:00.000Z', expectedResult: '1 Jan 2019'}
];

for (let i = 0; i < testCases.length; i++)
{
moment.tz.setDefault(testCases[i].localTimeZone);

expect(formatDate(testCases[i].date)).toEqual(testCases[i].expectedResult);
}
});

it('should format date ranges correctly', function()
{
// The local time zone should not affect the displayed dates, which are to be interpreted as UTC
// Ranges are exclusive of the end date and should handle this correctly
const testCases =
[
{localTimeZone: 'Etc/GMT-1', dateRange: ['2018-06-01T00:00:00.000Z', '2018-06-08T00:00:00.000Z'],
expectedResult: '1 Jun 2018 to 7 Jun 2018'},
{localTimeZone: 'Etc/GMT', dateRange: ['2018-06-01T00:00:00.000Z', '2018-06-08T00:00:00.000Z'],
expectedResult: '1 Jun 2018 to 7 Jun 2018'},
{localTimeZone: 'Etc/GMT+1', dateRange: ['2018-06-01T00:00:00.000Z', '2018-06-08T00:00:00.000Z'],
expectedResult: '1 Jun 2018 to 7 Jun 2018'}
];

for (let i = 0; i < testCases.length; i++)
{
moment.tz.setDefault(testCases[i].localTimeZone);

expect(formatDateRange(testCases[i].dateRange)).toEqual(testCases[i].expectedResult);
}
});

it('formatting date ranges repeatedly should not modify the data', function()
{
moment.tz.setDefault('Etc/GMT');

const date1 = new Date('2018-06-01T00:00:00.000Z');
const date2 = new Date('2018-06-08T00:00:00.000Z');
const date3 = new Date('2018-06-15T00:00:00.000Z');

const dateRange1 = [date1, date2];
const dateRange2 = [date2, date3];

for (let i = 0; i < 20; i++)
{
expect(formatDateRange(dateRange1)).toEqual('1 Jun 2018 to 7 Jun 2018');
expect(formatDateRange(dateRange2)).toEqual('8 Jun 2018 to 14 Jun 2018');
}
});
});
});
1 change: 1 addition & 0 deletions docs/spec/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module.exports = function(config)
'assets/js/vendor/jquery-3.2.1.min.js',
'assets/js/vendor/d3.v4.min.js',
'assets/js/vendor/moment-with-locales-2.21.0.min.js',
'assets/js/vendor/moment-timezone-0.5.14-2017c.min.js',
'assets/js/vendor/Chart-2.7.1.min.js',
'assets/js/vendor/spin-2.3.2.min.js',
'assets/js/charts.js',
Expand Down

0 comments on commit 9366cee

Please sign in to comment.