From 3b6f8871101fba69ccc737904817871276043562 Mon Sep 17 00:00:00 2001
From: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Date: Mon, 28 Oct 2019 19:05:47 -0400
Subject: [PATCH] Timezone aware scale (#11111)
* copy over code from pr
* remove chronological
* remove tick formatting
* remove chronological
* use reporting_timezone
* remove unneeded tests
* remove import
* add timeseriesScale tests/fixes
* eslint
* more timeseriesScale tests
* remove now unneeded rangeFn
* more timeseriesScale tests
* report_timezone to actual_timezone
* display warning if timezones are mismatched
* extract getTimezone
* don't error if series doesn't have a card (in tests)
* move expected/actual timezones into data, add warning when series have different timezones
* fixes
* make sure getTimezone works on transformed series
* update actual/expected to results/requested
* call getTimezone from LAB renderer rather than apply axis
* include timezone as part of a timeseries interval
* missed variable
* include timezone when updating timeseries interval
* remove "report_timezone"
* add timezone fields to DatasetData flow type
* make the fields optional
* Fill data using timezone scale (#11143)
---
frontend/src/metabase/meta/types/Dataset.js | 2 +
.../visualizations/lib/LineAreaBarRenderer.js | 25 +-
.../metabase/visualizations/lib/apply_axis.js | 33 +--
.../metabase/visualizations/lib/fill_data.js | 100 +++----
.../metabase/visualizations/lib/timeseries.js | 227 ++++++---------
.../metabase/visualizations/lib/warnings.js | 20 ++
.../LineAreaBarRenderer.unit.spec.js | 110 +++----
.../visualizations/lib/fill_data.unit.spec.js | 174 ++++++++++++
.../lib/timeseries.unit.spec.js | 268 ++++++++++++++++++
.../druid/test/metabase/driver/druid_test.clj | 7 +-
.../metabase/driver/googleanalytics_test.clj | 38 +--
.../mongo/test/metabase/driver/mongo_test.clj | 20 +-
package.json | 1 +
src/metabase/api/public.clj | 2 +-
.../middleware/add_settings.clj | 8 +-
test/metabase/api/dataset_test.clj | 20 +-
test/metabase/api/embed_test.clj | 13 +-
test/metabase/api/public_test.clj | 2 +-
test/metabase/driver/sql_jdbc/native_test.clj | 60 ++--
.../middleware/add_settings_test.clj | 20 +-
yarn.lock | 12 +
21 files changed, 773 insertions(+), 389 deletions(-)
create mode 100644 frontend/test/metabase/visualizations/lib/fill_data.unit.spec.js
diff --git a/frontend/src/metabase/meta/types/Dataset.js b/frontend/src/metabase/meta/types/Dataset.js
index 02ce487463d..674d6f1b4e9 100644
--- a/frontend/src/metabase/meta/types/Dataset.js
+++ b/frontend/src/metabase/meta/types/Dataset.js
@@ -36,6 +36,8 @@ export type DatasetData = {
cols: Column[],
rows: Row[],
rows_truncated?: number,
+ requested_timezone?: string,
+ results_timezone?: string,
};
export type Dataset = {
diff --git a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js
index fe58ace0cb6..9faf2198840 100644
--- a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js
+++ b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js
@@ -17,7 +17,11 @@ import {
colorShades,
} from "./utils";
-import { minTimeseriesUnit, computeTimeseriesDataInverval } from "./timeseries";
+import {
+ minTimeseriesUnit,
+ computeTimeseriesDataInverval,
+ getTimezone,
+} from "./timeseries";
import { computeNumericDataInverval } from "./numeric";
@@ -95,11 +99,16 @@ function checkSeriesIsValid({ series, maxSeries }) {
}
}
-function getXInterval({ settings, series }, xValues) {
+function getXInterval({ settings, series }, xValues, warn) {
if (isTimeseries(settings)) {
- // compute the interval
+ // We need three pieces of information to define a timeseries range:
+ // 1. interval - it's really the "unit": month, day, etc
+ // 2. count - how many intervals per tick?
+ // 3. timezone - what timezone are values in? days vary in length by timezone
const unit = minTimeseriesUnit(series.map(s => s.data.cols[0].unit));
- return computeTimeseriesDataInverval(xValues, unit);
+ const timezone = getTimezone(series, warn);
+ const { count, interval } = computeTimeseriesDataInverval(xValues, unit);
+ return { count, interval, timezone };
} else if (isQuantitative(settings) || isHistogram(settings)) {
// Get the bin width from binning_info, if available
// TODO: multiseries?
@@ -114,10 +123,10 @@ function getXInterval({ settings, series }, xValues) {
}
}
-function getXAxisProps(props, datas) {
+function getXAxisProps(props, datas, warn) {
const rawXValues = getXValues(props);
const isHistogram = isHistogramBar(props);
- const xInterval = getXInterval(props, rawXValues);
+ const xInterval = getXInterval(props, rawXValues, warn);
// For histograms we add a fake x value one xInterval to the right
// This compensates for the barshifting we do align ticks
@@ -804,10 +813,10 @@ export default function lineAreaBar(
}
let datas = getDatas(props, warn);
- let xAxisProps = getXAxisProps(props, datas);
+ let xAxisProps = getXAxisProps(props, datas, warn);
datas = fillMissingValuesInDatas(props, xAxisProps, datas);
- xAxisProps = getXAxisProps(props, datas);
+ xAxisProps = getXAxisProps(props, datas, warn);
if (isScalarSeries) {
xAxisProps.xValues = datas.map(data => data[0][0]);
diff --git a/frontend/src/metabase/visualizations/lib/apply_axis.js b/frontend/src/metabase/visualizations/lib/apply_axis.js
index 84c15f01f52..3b57de676ab 100644
--- a/frontend/src/metabase/visualizations/lib/apply_axis.js
+++ b/frontend/src/metabase/visualizations/lib/apply_axis.js
@@ -7,9 +7,8 @@ import moment from "moment";
import { datasetContainsNoResults } from "metabase/lib/dataset";
import { formatValue } from "metabase/lib/formatting";
-import { parseTimestamp } from "metabase/lib/time";
-import { computeTimeseriesTicksInterval } from "./timeseries";
+import { computeTimeseriesTicksInterval, timeseriesScale } from "./timeseries";
import { isMultipleOf, getModuloScaleFactor } from "./numeric";
import { getFriendlyName } from "./utils";
import { isHistogram } from "./renderer_utils";
@@ -96,10 +95,6 @@ export function applyChartTimeseriesXAxis(
// setup an x-axis where the dimension is a timeseries
let dimensionColumn = firstSeries.data.cols[0];
- // get the data's timezone offset from the first row
- const dataOffset =
- parseTimestamp(firstSeries.data.rows[0][0]).utcOffset() / 60;
-
// compute the data interval
const dataInterval = xInterval;
let tickInterval = dataInterval;
@@ -120,6 +115,9 @@ export function applyChartTimeseriesXAxis(
dimensionColumn = { ...dimensionColumn, unit: dataInterval.interval };
}
+ // extract xInterval timezone for updating tickInterval
+ const { timezone } = tickInterval;
+
// special handling for weeks
// TODO: are there any other cases where we should do this?
let tickFormatUnit = dimensionColumn.unit;
@@ -135,20 +133,15 @@ export function applyChartTimeseriesXAxis(
newTickInterval.count !== tickInterval.count
) {
tickFormatUnit = "month";
- tickInterval = { interval: "month", count: 1 };
+ tickInterval = { interval: "month", count: 1, timezone };
}
}
chart.xAxis().tickFormat(timestamp => {
- // timestamp is a plain Date object which discards the timezone,
- // so add it back in so it's formatted correctly
- const timestampFixed = moment(timestamp)
- .utcOffset(dataOffset)
- .format();
const { column, ...columnSettings } = chart.settings.column(
dimensionColumn,
);
- return formatValue(timestampFixed, {
+ return formatValue(timestamp, {
...columnSettings,
column: { ...column, unit: tickFormatUnit },
type: "axis",
@@ -157,21 +150,17 @@ export function applyChartTimeseriesXAxis(
});
// Compute a sane interval to display based on the data granularity, domain, and chart width
- tickInterval = computeTimeseriesTicksInterval(
- xDomain,
- tickInterval,
- chart.width(),
- );
- chart.xAxis().ticks(tickInterval.rangeFn, tickInterval.count);
- } else {
- chart.xAxis().ticks(0);
+ tickInterval = {
+ ...computeTimeseriesTicksInterval(xDomain, tickInterval, chart.width()),
+ timezone,
+ };
}
// pad the domain slightly to prevent clipping
xDomain = stretchTimeseriesDomain(xDomain, dataInterval);
// set the x scale
- chart.x(d3.time.scale.utc().domain(xDomain)); //.nice(d3.time[dataInterval.interval]));
+ chart.x(timeseriesScale(tickInterval).domain(xDomain));
// set the x units (used to compute bar size)
chart.xUnits((start, stop) =>
diff --git a/frontend/src/metabase/visualizations/lib/fill_data.js b/frontend/src/metabase/visualizations/lib/fill_data.js
index 34f0700fd1e..b8aa1417ff3 100644
--- a/frontend/src/metabase/visualizations/lib/fill_data.js
+++ b/frontend/src/metabase/visualizations/lib/fill_data.js
@@ -2,7 +2,6 @@
import { t } from "ttag";
import d3 from "d3";
-import moment from "moment";
import {
isTimeseries,
@@ -10,6 +9,7 @@ import {
isHistogram,
isHistogramBar,
} from "./renderer_utils";
+import { timeseriesScale } from "./timeseries";
// max number of points to "fill"
// TODO: base on pixel width of chart?
@@ -28,7 +28,9 @@ function fillMissingValues(rows, xValues, fillValue, getKey = v => v) {
const row = map.get(key);
if (row) {
map.delete(key);
- return [value, ...row.slice(1)];
+ const newRow = [value, ...row.slice(1)];
+ newRow._origin = row._origin;
+ return newRow;
} else {
return [value, ...fillValues];
}
@@ -46,60 +48,48 @@ function fillMissingValues(rows, xValues, fillValue, getKey = v => v) {
function fillMissingValuesInData(
props,
{ xValues, xDomain, xInterval },
- seriesSettings,
+ singleSeries,
rows,
) {
const { settings } = props;
- if (
- seriesSettings["line.missing"] === "zero" ||
- seriesSettings["line.missing"] === "none"
- ) {
- const fillValue = seriesSettings["line.missing"] === "zero" ? 0 : null;
- if (isTimeseries(settings)) {
- // $FlowFixMe
- const { interval, count } = xInterval;
- if (count <= MAX_FILL_COUNT) {
- // replace xValues with
- xValues = d3.time[interval]
- .range(xDomain[0], moment(xDomain[1]).add(1, "ms"), count)
- .map(d => moment(d));
- return fillMissingValues(
- rows,
- xValues,
- fillValue,
- m => d3.round(m.toDate().getTime(), -1), // sometimes rounds up 1ms?
- );
- }
+ const { "line.missing": lineMissing } = settings.series(singleSeries);
+
+ // return now if we're not filling with either 0 or null
+ if (!(lineMissing === "zero" || lineMissing === "none")) {
+ return rows;
+ }
+ let getKey;
+ const fillValue = lineMissing === "zero" ? 0 : null;
+ if (isTimeseries(settings)) {
+ const count = Math.abs(
+ xDomain[1].diff(xDomain[0], xInterval.interval) / xInterval.count,
+ );
+ if (count > MAX_FILL_COUNT) {
+ return rows;
}
- if (isQuantitative(settings) || isHistogram(settings)) {
- // $FlowFixMe
- const count = Math.abs((xDomain[1] - xDomain[0]) / xInterval);
- if (count <= MAX_FILL_COUNT) {
- let [start, end] = xDomain;
- if (isHistogramBar(props)) {
- // NOTE: intentionally add an end point for bar histograms
- // $FlowFixMe
- end += xInterval * 1.5;
- } else {
- // NOTE: avoid including endpoint due to floating point error
- // $FlowFixMe
- end += xInterval * 0.5;
- }
- xValues = d3.range(start, end, xInterval);
- return fillMissingValues(
- rows,
- xValues,
- fillValue,
- // NOTE: normalize to xInterval to avoid floating point issues
- v => Math.round(v / xInterval),
- );
- }
+
+ xValues = timeseriesScale(xInterval)
+ .domain(xDomain)
+ .ticks();
+ getKey = m => m.toISOString();
+ } else if (isQuantitative(settings) || isHistogram(settings)) {
+ const count = Math.abs((xDomain[1] - xDomain[0]) / xInterval);
+ if (count > MAX_FILL_COUNT) {
+ return rows;
+ }
+ let [start, end] = xDomain;
+ if (isHistogramBar(props)) {
+ // NOTE: intentionally add an end point for bar histograms
+ end += xInterval * 1.5;
} else {
- return fillMissingValues(rows, xValues, fillValue);
+ // NOTE: avoid including endpoint due to floating point error
+ end += xInterval * 0.5;
}
- } else {
- return rows;
+ xValues = d3.range(start, end, xInterval);
+ // NOTE: normalize to xInterval to avoid floating point issues
+ getKey = v => Math.round(v / xInterval);
}
+ return fillMissingValues(rows, xValues, fillValue, getKey);
}
export default function fillMissingValuesInDatas(
@@ -107,14 +97,12 @@ export default function fillMissingValuesInDatas(
{ xValues, xDomain, xInterval },
datas,
) {
- const { series, settings } = props;
- return datas.map((rows, index) => {
- const seriesSettings = settings.series(series[index]);
- return fillMissingValuesInData(
+ return datas.map((rows, index) =>
+ fillMissingValuesInData(
props,
{ xValues, xDomain, xInterval },
- seriesSettings,
+ props.series[index],
rows,
- );
- });
+ ),
+ );
}
diff --git a/frontend/src/metabase/visualizations/lib/timeseries.js b/frontend/src/metabase/visualizations/lib/timeseries.js
index d1204a59b78..b40e89e3f72 100644
--- a/frontend/src/metabase/visualizations/lib/timeseries.js
+++ b/frontend/src/metabase/visualizations/lib/timeseries.js
@@ -1,12 +1,14 @@
/* @flow weak */
import d3 from "d3";
-import moment from "moment";
+import moment from "moment-timezone";
import _ from "underscore";
import { isDate } from "metabase/lib/schema_metadata";
import { parseTimestamp } from "metabase/lib/time";
+import { unexpectedTimezoneWarning, multipleTimezoneWarning } from "./warnings";
+
const TIMESERIES_UNITS = new Set([
"minute",
"hour",
@@ -32,141 +34,31 @@ export function dimensionIsTimeseries({ cols, rows }, i = 0) {
// Use UTC methods to avoid issues with daylight savings
// NOTE: smaller modulos within an interval type must be multiples of larger ones (e.x. can't do both 2 days and 7 days i.e. week)
//
-// Count and time interval for axis.ticks() (see https://github.com/d3/d3-3.x-api-reference/blob/master/SVG-Axes.md#ticks)
-// is specified by rangeFn and count, e.g.
-//
-// xAxis.ticks(interval.rangeFn, interval.count) -> xAxis.ticks(d3.time.minutes, 15) // every 15 minutes
+// Count and time interval for axis.ticks()
//
-// TODO - I'm not sure what the appropriate thing to put for rangeFn for milliseconds is. This matches the previous
-// behavior, which may have been wrong in the first place. See https://github.com/d3/d3/issues/1529 for a similar issue
const TIMESERIES_INTERVALS = [
- { interval: "ms", count: 1, rangeFn: undefined, testFn: d => 0 }, // (0) millisecond
- {
- interval: "second",
- count: 1,
- rangeFn: d3.time.seconds,
- testFn: d => parseTimestamp(d).milliseconds(),
- }, // (1) 1 second
- {
- interval: "second",
- count: 5,
- rangeFn: d3.time.seconds,
- testFn: d => parseTimestamp(d).seconds() % 5,
- }, // (2) 5 seconds
- {
- interval: "second",
- count: 15,
- rangeFn: d3.time.seconds,
- testFn: d => parseTimestamp(d).seconds() % 15,
- }, // (3) 15 seconds
- {
- interval: "second",
- count: 30,
- rangeFn: d3.time.seconds,
- testFn: d => parseTimestamp(d).seconds() % 30,
- }, // (4) 30 seconds
- {
- interval: "minute",
- count: 1,
- rangeFn: d3.time.minutes,
- testFn: d => parseTimestamp(d).seconds(),
- }, // (5) 1 minute
- {
- interval: "minute",
- count: 5,
- rangeFn: d3.time.minutes,
- testFn: d => parseTimestamp(d).minutes() % 5,
- }, // (6) 5 minutes
- {
- interval: "minute",
- count: 15,
- rangeFn: d3.time.minutes,
- testFn: d => parseTimestamp(d).minutes() % 15,
- }, // (7) 15 minutes
- {
- interval: "minute",
- count: 30,
- rangeFn: d3.time.minutes,
- testFn: d => parseTimestamp(d).minutes() % 30,
- }, // (8) 30 minutes
- {
- interval: "hour",
- count: 1,
- rangeFn: d3.time.hours,
- testFn: d => parseTimestamp(d).minutes(),
- }, // (9) 1 hour
- {
- interval: "hour",
- count: 3,
- rangeFn: d3.time.hours,
- testFn: d => parseTimestamp(d).hours() % 3,
- }, // (10) 3 hours
- {
- interval: "hour",
- count: 6,
- rangeFn: d3.time.hours,
- testFn: d => parseTimestamp(d).hours() % 6,
- }, // (11) 6 hours
- {
- interval: "hour",
- count: 12,
- rangeFn: d3.time.hours,
- testFn: d => parseTimestamp(d).hours() % 12,
- }, // (12) 12 hours
- {
- interval: "day",
- count: 1,
- rangeFn: d3.time.days,
- testFn: d => parseTimestamp(d).hours(),
- }, // (13) 1 day
- {
- interval: "week",
- count: 1,
- rangeFn: d3.time.weeks,
- testFn: d => parseTimestamp(d).date() % 7,
- }, // (14) 7 days / 1 week
- {
- interval: "month",
- count: 1,
- rangeFn: d3.time.months,
- testFn: d => parseTimestamp(d).date(),
- }, // (15) 1 months
- {
- interval: "month",
- count: 3,
- rangeFn: d3.time.months,
- testFn: d => parseTimestamp(d).month() % 3,
- }, // (16) 3 months / 1 quarter
- {
- interval: "year",
- count: 1,
- rangeFn: d3.time.years,
- testFn: d => parseTimestamp(d).month(),
- }, // (17) 1 year
- {
- interval: "year",
- count: 5,
- rangeFn: d3.time.years,
- testFn: d => parseTimestamp(d).year() % 5,
- }, // (18) 5 year
- {
- interval: "year",
- count: 10,
- rangeFn: d3.time.years,
- testFn: d => parseTimestamp(d).year() % 10,
- }, // (19) 10 year
- {
- interval: "year",
- count: 50,
- rangeFn: d3.time.years,
- testFn: d => parseTimestamp(d).year() % 50,
- }, // (20) 50 year
- {
- interval: "year",
- count: 100,
- rangeFn: d3.time.years,
- testFn: d => parseTimestamp(d).year() % 100,
- }, // (21) 100 year
+ { interval: "ms", count: 1, testFn: d => 0 }, // (0) millisecond
+ { interval: "second", count: 1, testFn: d => d.milliseconds() }, // (1) 1 second
+ { interval: "second", count: 5, testFn: d => d.seconds() % 5 }, // (2) 5 seconds
+ { interval: "second", count: 15, testFn: d => d.seconds() % 15 }, // (3) 15 seconds
+ { interval: "second", count: 30, testFn: d => d.seconds() % 30 }, // (4) 30 seconds
+ { interval: "minute", count: 1, testFn: d => d.seconds() }, // (5) 1 minute
+ { interval: "minute", count: 5, testFn: d => d.minutes() % 5 }, // (6) 5 minutes
+ { interval: "minute", count: 15, testFn: d => d.minutes() % 15 }, // (7) 15 minutes
+ { interval: "minute", count: 30, testFn: d => d.minutes() % 30 }, // (8) 30 minutes
+ { interval: "hour", count: 1, testFn: d => d.minutes() }, // (9) 1 hour
+ { interval: "hour", count: 3, testFn: d => d.hours() % 3 }, // (10) 3 hours
+ { interval: "hour", count: 6, testFn: d => d.hours() % 6 }, // (11) 6 hours
+ { interval: "hour", count: 12, testFn: d => d.hours() % 12 }, // (12) 12 hours
+ { interval: "day", count: 1, testFn: d => d.hours() }, // (13) 1 day
+ { interval: "week", count: 1, testFn: d => d.date() % 7 }, // (14) 7 days / 1 week
+ { interval: "month", count: 1, testFn: d => d.date() }, // (15) 1 months
+ { interval: "month", count: 3, testFn: d => d.month() % 3 }, // (16) 3 months / 1 quarter
+ { interval: "year", count: 1, testFn: d => d.month() }, // (17) 1 year
+ { interval: "year", count: 5, testFn: d => d.year() % 5 }, // (18) 5 year
+ { interval: "year", count: 10, testFn: d => d.year() % 10 }, // (19) 10 year
+ { interval: "year", count: 50, testFn: d => d.year() % 50 }, // (20) 50 year
+ { interval: "year", count: 100, testFn: d => d.year() % 100 }, // (21) 100 year
];
// mapping from Metabase "unit" to d3 intervals above
@@ -208,7 +100,7 @@ function computeTimeseriesDataInvervalIndex(xValues, unit) {
// Only need to check more granular than the current interval
for (let i = 0; i < TIMESERIES_INTERVALS.length && i < index; i++) {
const interval = TIMESERIES_INTERVALS[i];
- const value = interval.testFn(xValue);
+ const value = interval.testFn(parseTimestamp(xValue));
if (values[i] === undefined) {
values[i] = value;
} else if (values[i] !== value) {
@@ -307,3 +199,68 @@ export function computeTimeseriesTicksInterval(xDomain, xInterval, chartWidth) {
maxTicksForChartWidth(chartWidth),
);
}
+
+// moment-timezone based d3 scale
+export const timeseriesScale = (
+ { count, interval, timezone },
+ linear = d3.scale.linear(),
+) => {
+ const ms = d =>
+ moment.isMoment(d) ? d.valueOf() : moment.isDate(d) ? d.getTime() : d;
+
+ const s = x => linear(ms(x));
+ s.domain = x => {
+ if (x === undefined) {
+ return linear.domain().map(t => moment(t).tz(timezone));
+ }
+ linear.domain(x.map(ms));
+ return s;
+ };
+ s.ticks = () => {
+ const [start, end] = s.domain();
+
+ const ticks = [];
+ let tick = start
+ .clone()
+ .tz(timezone)
+ .startOf(interval);
+
+ // We want to use "round" ticks for a given interval (unit). If we're
+ // creating ticks every 50 years, but and the start of the domain is in 1981
+ // we move it be on an even 50-year block. 1981 - (1981 % 50) => 1950;
+ const intervalMod = tick.get(interval);
+ tick.set(interval, intervalMod - (intervalMod % count));
+
+ while (!tick.isAfter(end)) {
+ if (!tick.isBefore(start)) {
+ ticks.push(tick);
+ }
+ tick = tick.clone().add(count, interval);
+ }
+ return ticks;
+ };
+ s.copy = () => timeseriesScale({ count, interval, timezone }, linear);
+ d3.rebind(s, linear, "range", "rangeRound", "interpolate", "clamp", "invert");
+ return s;
+};
+
+// We should always have results_timezone, but just in case we fallback to UTC
+const DEFAULT_TIMEZONE = "Etc/UTC";
+
+export function getTimezone(series, warn) {
+ series = series._raw || series;
+
+ // Dashboard multiseries cards might have series with different timezones.
+ const timezones = Array.from(
+ new Set(series.map(s => s.data.results_timezone)),
+ );
+ if (timezones.length > 1) {
+ warn(multipleTimezoneWarning(timezones));
+ }
+ // Warn if the query was run in an unexpected timezone.
+ const { results_timezone, requested_timezone } = series[0].data;
+ if (requested_timezone && requested_timezone !== results_timezone) {
+ warn(unexpectedTimezoneWarning({ results_timezone, requested_timezone }));
+ }
+ return results_timezone || DEFAULT_TIMEZONE;
+}
diff --git a/frontend/src/metabase/visualizations/lib/warnings.js b/frontend/src/metabase/visualizations/lib/warnings.js
index 95c2531f3eb..f7618c0cff2 100644
--- a/frontend/src/metabase/visualizations/lib/warnings.js
+++ b/frontend/src/metabase/visualizations/lib/warnings.js
@@ -27,3 +27,23 @@ export function unaggregatedDataWarning(col) {
)}" is an unaggregated field: if it has more than one value at a point on the x-axis, the values will be summed.`,
};
}
+
+export const UNEXPECTED_QUERY_TIMEZONE = "UNEXPECTED_QUERY_TIMEZONE";
+export function unexpectedTimezoneWarning({
+ results_timezone,
+ requested_timezone,
+}) {
+ return {
+ key: UNEXPECTED_QUERY_TIMEZONE,
+ text: t`The query for this chart was run in ${results_timezone} rather than ${requested_timezone} due to database or driver constraints.`,
+ };
+}
+
+export const MULTIPLE_TIMEZONES = "MULTIPLE_TIMEZONES";
+export function multipleTimezoneWarning(timezones) {
+ const tzList = timezones.join(", ");
+ return {
+ key: MULTIPLE_TIMEZONES,
+ text: t`This chart contains queries run in multiple timezones: ${tzList}`,
+ };
+}
diff --git a/frontend/test/metabase/visualizations/components/LineAreaBarRenderer.unit.spec.js b/frontend/test/metabase/visualizations/components/LineAreaBarRenderer.unit.spec.js
index f3984bf50ae..252ef80e633 100644
--- a/frontend/test/metabase/visualizations/components/LineAreaBarRenderer.unit.spec.js
+++ b/frontend/test/metabase/visualizations/components/LineAreaBarRenderer.unit.spec.js
@@ -1,9 +1,5 @@
import "__support__/mocks"; // included explicitly whereas with e2e tests it comes with __support__/e2e_tests
-import { formatValue } from "metabase/lib/formatting";
-
-import d3 from "d3";
-
import {
NumberColumn,
DateTimeColumn,
@@ -18,12 +14,6 @@ import lineAreaBarRenderer, {
getDimensionsAndGroupsAndUpdateSeriesDisplayNames,
} from "metabase/visualizations/lib/LineAreaBarRenderer";
-const formatTz = offset =>
- (offset < 0 ? "-" : "+") + d3.format("02d")(Math.abs(offset)) + ":00";
-
-const BROWSER_TZ = formatTz(-new Date().getTimezoneOffset() / 60);
-const ALL_TZS = d3.range(-1, 2).map(formatTz);
-
describe("LineAreaBarRenderer", () => {
let element;
@@ -71,77 +61,45 @@ describe("LineAreaBarRenderer", () => {
expect(warnings).toEqual(['We encountered an invalid date: "2019-W53"']);
});
- ["Z", ...ALL_TZS].forEach(tz =>
- it(
- "should display hourly data (in " +
- tz +
- " timezone) in X axis and tooltip consistently",
- () => {
- const onHoverChange = jest.fn();
-
- const rows = [
- ["2016-10-03T20:00:00.000" + tz, 1],
- ["2016-10-03T21:00:00.000" + tz, 1],
- ];
-
- renderTimeseriesLine({
- rowsOfSeries: [rows],
- unit: "hour",
- onHoverChange,
- });
-
- dispatchUIEvent(qs(".dot"), "mousemove");
-
- const expected = rows.map(row =>
- formatValue(row[0], {
- column: DateTimeColumn({ unit: "hour" }),
- }),
- );
- expect(getFormattedTooltips(onHoverChange.mock.calls[0][0])).toEqual([
- expected[0],
- "1",
- ]);
- expect(qsa(".axis.x .tick text").map(e => e.textContent)).toEqual(
- expected,
- );
- },
- ),
- );
-
- it("should display hourly data (in the browser's timezone) in X axis and tooltip consistently and correctly", () => {
- const onHoverChange = jest.fn();
- const tz = BROWSER_TZ;
- const rows = [
- ["2016-01-01T01:00:00.000" + tz, 1],
- ["2016-01-01T02:00:00.000" + tz, 1],
- ["2016-01-01T03:00:00.000" + tz, 1],
- ["2016-01-01T04:00:00.000" + tz, 1],
- ];
+ it("should warn if expected timezone doesn't match actual", () => {
+ const data = {
+ cols: [DateTimeColumn(), NumberColumn()],
+ rows: [["2019-01-01", 1]],
+ requested_timezone: "US/Pacific",
+ results_timezone: "US/Eastern",
+ };
+ const card = { display: "line", visualization_settings: {} };
+ const onRender = jest.fn();
- renderTimeseriesLine({
- rowsOfSeries: [rows],
- unit: "hour",
- onHoverChange,
- });
+ renderLineAreaBar(element, [{ data, card }], { onRender });
- dispatchUIEvent(qs(".dot"), "mousemove");
+ const [[{ warnings }]] = onRender.mock.calls;
+ expect(warnings).toEqual([
+ "The query for this chart was run in US/Eastern rather than US/Pacific due to database or driver constraints.",
+ ]);
+ });
- expect(
- formatValue(rows[0][0], {
- column: DateTimeColumn({ unit: "hour" }),
- }),
- ).toEqual("January 1, 2016, 1:00 AM");
+ it("should warn if there are multiple timezones", () => {
+ const seriesInTZ = tz => ({
+ data: {
+ cols: [DateTimeColumn(), NumberColumn()],
+ rows: [["2019-01-01", 1]],
+ requested_timezone: tz,
+ results_timezone: tz,
+ },
+ card: { display: "line", visualization_settings: {} },
+ });
+ const onRender = jest.fn();
- expect(getFormattedTooltips(onHoverChange.mock.calls[0][0])).toEqual([
- "January 1, 2016, 1:00 AM",
- "1",
- ]);
+ renderLineAreaBar(
+ element,
+ [seriesInTZ("US/Pacific"), seriesInTZ("US/Eastern")],
+ { onRender },
+ );
- expect(qsa(".axis.x .tick text").map(e => e.textContent)).toEqual([
- "January 1, 2016, 1:00 AM",
- "January 1, 2016, 2:00 AM",
- "January 1, 2016, 3:00 AM",
- "January 1, 2016, 4:00 AM",
+ const [[{ warnings }]] = onRender.mock.calls;
+ expect(warnings).toEqual([
+ "This chart contains queries run in multiple timezones: US/Pacific, US/Eastern",
]);
});
diff --git a/frontend/test/metabase/visualizations/lib/fill_data.unit.spec.js b/frontend/test/metabase/visualizations/lib/fill_data.unit.spec.js
new file mode 100644
index 00000000000..bd0164f9041
--- /dev/null
+++ b/frontend/test/metabase/visualizations/lib/fill_data.unit.spec.js
@@ -0,0 +1,174 @@
+import moment from "moment";
+
+import fillMissingValuesInDatas from "metabase/visualizations/lib/fill_data";
+
+describe("fillMissingValuesInDatas", () => {
+ it("should fill missing days", () => {
+ const time1 = moment("2018-01-01T00:00:00Z");
+ const time2 = moment("2018-01-31T00:00:00Z");
+ const rows = [[time1, 1], [time2, 2]];
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "timeseries",
+ series: () => ({ "line.missing": "none" }),
+ },
+ },
+ {
+ xValues: [time1, time2],
+ xDomain: [time1, time2],
+ xInterval: { interval: "day", count: 1, timezone: "Etc/UTC" },
+ },
+ [rows],
+ );
+
+ const yValues = filledData.map(d => d[1]);
+ expect(yValues).toEqual([1, ...new Array(29).fill(null), 2]);
+ });
+
+ it("should fill missing hours", () => {
+ const time1 = moment("2018-01-01T00:00:00Z");
+ const time2 = moment("2018-01-05T00:00:00Z");
+ const rows = [[time1, 1], [time2, 2]];
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "timeseries",
+ series: () => ({ "line.missing": "none" }),
+ },
+ },
+ {
+ xValues: [time1, time2],
+ xDomain: [time1, time2],
+ xInterval: { interval: "hour", count: 1, timezone: "Etc/UTC" },
+ },
+ [rows],
+ );
+
+ const yValues = filledData.map(d => d[1]);
+ expect(yValues).toEqual([1, ...new Array(95).fill(null), 2]);
+ });
+
+ it("should fill linear data", () => {
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "linear",
+ series: () => ({ "line.missing": "none" }),
+ },
+ },
+ { xValues: [1, 3], xDomain: [1, 3], xInterval: 1 },
+ [[[1, 1], [3, 1]]],
+ );
+
+ expect(filledData).toEqual([[1, 1], [2, null], [3, 1]]);
+ });
+
+ it("should fill with zeros", () => {
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "linear",
+ series: () => ({ "line.missing": "zero" }),
+ },
+ },
+ { xValues: [1, 3], xDomain: [1, 3], xInterval: 1 },
+ [[[1, 1], [3, 1]]],
+ );
+
+ expect(filledData).toEqual([[1, 1], [2, 0], [3, 1]]);
+ });
+
+ it("shouldn't fill data when line.missing = interpolate", () => {
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "linear",
+ series: () => ({ "line.missing": "interpolate" }),
+ },
+ },
+ { xValues: [1, 3], xDomain: [1, 3], xInterval: 1 },
+ [[[1, 1], [3, 1]]],
+ );
+
+ expect(filledData).toEqual([[1, 1], [3, 1]]);
+ });
+
+ it("shouldn't fill data when the range is >10k", () => {
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "linear",
+ series: () => ({ "line.missing": "zero" }),
+ },
+ },
+ { xValues: [1, 11000], xDomain: [1, 11000], xInterval: 1 },
+ [[[1, 1], [11000, 1]]],
+ );
+
+ expect(filledData).toEqual([[1, 1], [11000, 1]]);
+ });
+
+ it("shouldn't fill data when the range is >10k for timeseries", () => {
+ const t1 = moment("2018-01-01T00:00:00Z");
+ const t2 = moment("2020-01-01T00:00:00Z");
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "timeseries",
+ series: () => ({ "line.missing": "zero" }),
+ },
+ },
+ {
+ xValues: [t1, t2],
+ xDomain: [t1, t2],
+ xInterval: { interval: "hour", count: 1, timezone: "Etc/UTC" },
+ },
+ [[[t1, 1], [t2, 1]]],
+ );
+
+ expect(filledData).toEqual([[t1, 1], [t2, 1]]);
+ });
+
+ it("should use interval while filling numeric data", () => {
+ const [filledData] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "linear",
+ series: () => ({ "line.missing": "zero" }),
+ },
+ },
+ { xValues: [10, 30], xDomain: [10, 30], xInterval: 10 },
+ [[[10, 1], [30, 1]]],
+ );
+
+ expect(filledData).toEqual([[10, 1], [20, 0], [30, 1]]);
+ });
+
+ it("should maintain _origin on rows", () => {
+ // the _origin property is used in tooltips, so make sure it's carried over
+ const row = [1, 1];
+ row._origin = [1, 1, 2, 3];
+ const [[{ _origin }]] = fillMissingValuesInDatas(
+ {
+ series: [{}],
+ settings: {
+ "graph.x_axis.scale": "linear",
+ series: () => ({ "line.missing": "zero" }),
+ },
+ },
+ { xValues: [1], xDomain: [1, 1], xInterval: 1 },
+ [[row]],
+ );
+
+ expect(_origin).toEqual([1, 1, 2, 3]);
+ });
+});
diff --git a/frontend/test/metabase/visualizations/lib/timeseries.unit.spec.js b/frontend/test/metabase/visualizations/lib/timeseries.unit.spec.js
index 21fa26822f9..56091f21af2 100644
--- a/frontend/test/metabase/visualizations/lib/timeseries.unit.spec.js
+++ b/frontend/test/metabase/visualizations/lib/timeseries.unit.spec.js
@@ -1,7 +1,14 @@
+import moment from "moment";
+
import {
dimensionIsTimeseries,
computeTimeseriesDataInverval,
+ timeseriesScale,
+ getTimezone,
} from "metabase/visualizations/lib/timeseries";
+import { getVisualizationTransformed } from "metabase/visualizations";
+
+import { StringColumn, NumberColumn } from "../__support__/visualizations";
import { TYPE } from "metabase/lib/types";
@@ -145,4 +152,265 @@ describe("visualization.lib.timeseries", () => {
});
});
});
+
+ describe("timeseriesScale", () => {
+ it("should create day ranges", () => {
+ const scale = timeseriesScale({
+ interval: "day",
+ count: 1,
+ timezone: "Etc/UTC",
+ }).domain([
+ moment("2019-03-08T00:00:00.000Z"),
+ moment("2019-03-12T00:00:00.000Z"),
+ ]);
+
+ expect(scale.ticks().map(t => t.toISOString())).toEqual([
+ "2019-03-08T00:00:00.000Z",
+ "2019-03-09T00:00:00.000Z",
+ "2019-03-10T00:00:00.000Z",
+ "2019-03-11T00:00:00.000Z",
+ "2019-03-12T00:00:00.000Z",
+ ]);
+ });
+
+ it("should create day ranges in pacific time across dst boundary", () => {
+ const scale = timeseriesScale({
+ interval: "day",
+ count: 1,
+ timezone: "US/Pacific",
+ }).domain([
+ moment("2019-03-08T00:00:00.000-08"),
+ moment("2019-03-12T00:00:00.000-07"),
+ ]);
+
+ expect(scale.ticks().map(t => t.toISOString())).toEqual([
+ "2019-03-08T08:00:00.000Z",
+ "2019-03-09T08:00:00.000Z",
+ "2019-03-10T08:00:00.000Z",
+ "2019-03-11T07:00:00.000Z",
+ "2019-03-12T07:00:00.000Z",
+ ]);
+ });
+
+ it("should create hour ranges in pacific time across spring dst boundary", () => {
+ const scale = timeseriesScale({
+ interval: "hour",
+ count: 1,
+ timezone: "US/Pacific",
+ }).domain([
+ moment("2019-03-10T00:00:00.000-08"),
+ moment("2019-03-10T04:00:00.000-07"),
+ ]);
+
+ expect(scale.ticks().map(t => t.format())).toEqual([
+ "2019-03-10T00:00:00-08:00",
+ "2019-03-10T01:00:00-08:00",
+ "2019-03-10T03:00:00-07:00",
+ "2019-03-10T04:00:00-07:00",
+ ]);
+ });
+
+ it("should create hour ranges in pacific time across fall dst boundary", () => {
+ const scale = timeseriesScale({
+ interval: "hour",
+ count: 1,
+ timezone: "US/Pacific",
+ }).domain([
+ moment("2019-11-03T00:00:00.000-07"),
+ moment("2019-11-03T04:00:00.000-08"),
+ ]);
+
+ expect(scale.ticks().map(t => t.format())).toEqual([
+ "2019-11-03T00:00:00-07:00",
+ "2019-11-03T01:00:00-07:00",
+ "2019-11-03T01:00:00-08:00",
+ "2019-11-03T02:00:00-08:00",
+ "2019-11-03T03:00:00-08:00",
+ "2019-11-03T04:00:00-08:00",
+ ]);
+ });
+
+ it("should create day ranges that don't align with UTC hours", () => {
+ const scale = timeseriesScale({
+ interval: "day",
+ count: 1,
+ timezone: "Asia/Kathmandu",
+ }).domain([
+ moment("2019-01-01T18:15:00.000Z"),
+ moment("2019-01-03T18:15:00.000Z"),
+ ]);
+
+ expect(scale.ticks().map(t => t.toISOString())).toEqual([
+ "2019-01-01T18:15:00.000Z",
+ "2019-01-02T18:15:00.000Z",
+ "2019-01-03T18:15:00.000Z",
+ ]);
+ });
+
+ it("should create day ranges when the domain doesn't line up with unit boundaries", () => {
+ const scale = timeseriesScale({
+ interval: "day",
+ count: 1,
+ timezone: "Etc/UTC",
+ }).domain([
+ moment("2019-03-07T12:34:56.789Z"),
+ moment("2019-03-12T12:34:56.789Z"),
+ ]);
+
+ expect(scale.ticks().map(t => t.toISOString())).toEqual([
+ "2019-03-08T00:00:00.000Z",
+ "2019-03-09T00:00:00.000Z",
+ "2019-03-10T00:00:00.000Z",
+ "2019-03-11T00:00:00.000Z",
+ "2019-03-12T00:00:00.000Z",
+ ]);
+ });
+
+ it("should create empty ranges if there are no ticks in domain", () => {
+ const scale = timeseriesScale({
+ interval: "day",
+ count: 1,
+ timezone: "Etc/UTC",
+ }).domain([
+ moment("2019-03-09T01:00:00.000Z"),
+ moment("2019-03-09T22:00:00.000Z"),
+ ]);
+
+ expect(scale.ticks().length).toBe(0);
+ });
+
+ it("should create month ranges in timezone", () => {
+ const scale = timeseriesScale({
+ interval: "month",
+ count: 1,
+ timezone: "Asia/Hong_kong",
+ }).domain([
+ moment("2019-03-07T12:34:56.789Z"),
+ moment("2019-04-12T12:34:56.789Z"),
+ ]);
+
+ expect(scale.ticks().map(t => t.toISOString())).toEqual([
+ "2019-03-31T16:00:00.000Z",
+ ]);
+ });
+
+ it("should create month ranges spaced by count", () => {
+ const scale = timeseriesScale({
+ interval: "month",
+ count: 3,
+ timezone: "Etc/UTC",
+ }).domain([
+ moment("2018-11-01T00:00:00.000Z"),
+ moment("2020-02-01T00:00:00.000Z"),
+ ]);
+
+ expect(scale.ticks().map(t => t.toISOString())).toEqual([
+ "2019-01-01T00:00:00.000Z",
+ "2019-04-01T00:00:00.000Z",
+ "2019-07-01T00:00:00.000Z",
+ "2019-10-01T00:00:00.000Z",
+ "2020-01-01T00:00:00.000Z",
+ ]);
+ });
+
+ it("should create 50 year ranges", () => {
+ const scale = timeseriesScale({
+ interval: "year",
+ count: 50,
+ timezone: "Etc/UTC",
+ }).domain([
+ moment("1890-01-01T00:00:00.000Z"),
+ moment("2020-01-01T00:00:00.000Z"),
+ ]);
+
+ expect(scale.ticks().map(t => t.toISOString())).toEqual([
+ "1900-01-01T00:00:00.000Z",
+ "1950-01-01T00:00:00.000Z",
+ "2000-01-01T00:00:00.000Z",
+ ]);
+ });
+
+ for (const unit of ["month", "quarter", "year"]) {
+ it(`should produce results with ${unit}s`, () => {
+ const ticks = timeseriesScale({
+ interval: unit,
+ count: 1,
+ timezone: "Etc/UTC",
+ })
+ .domain([
+ moment("1999-12-31T23:59:59Z"),
+ moment("2001-01-01T00:00:01Z"),
+ ])
+ .ticks();
+
+ // we're just ensuring that it produces some results and that the first
+ // and last are correctly rounded regardless of unit
+ expect(ticks[0].toISOString()).toEqual("2000-01-01T00:00:00.000Z");
+ expect(ticks[ticks.length - 1].toISOString()).toEqual(
+ "2001-01-01T00:00:00.000Z",
+ );
+ });
+ }
+
+ // same as above but with a smaller range so the test runs faster
+ for (const unit of ["minute", "hour", "day"]) {
+ it(`should produce results with ${unit}s`, () => {
+ const ticks = timeseriesScale({
+ interval: unit,
+ count: 1,
+ timezone: "Etc/UTC",
+ })
+ .domain([
+ moment("1999-12-31T23:59:59Z"),
+ moment("2000-01-02T00:00:01Z"),
+ ])
+ .ticks();
+
+ expect(ticks[0].toISOString()).toEqual("2000-01-01T00:00:00.000Z");
+ expect(ticks[ticks.length - 1].toISOString()).toEqual(
+ "2000-01-02T00:00:00.000Z",
+ );
+ });
+ }
+
+ // weeks are split out because their boundaries don't align with other units
+ it(`should produce results with weeks`, () => {
+ const ticks = timeseriesScale({
+ interval: "week",
+ count: 1,
+ timezone: "Etc/UTC",
+ })
+ .domain([
+ moment("2000-01-02T12:34:56Z"),
+ moment("2000-02-02T12:34:56Z"),
+ ])
+ .ticks();
+
+ expect(ticks[0].toISOString()).toEqual("2000-01-09T00:00:00.000Z");
+ expect(ticks[ticks.length - 1].toISOString()).toEqual(
+ "2000-01-30T00:00:00.000Z",
+ );
+ });
+ });
+ describe("getTimezone", () => {
+ const series = [
+ {
+ card: { visualization_settings: {}, display: "bar" },
+ data: {
+ results_timezone: "US/Eastern",
+ cols: [StringColumn({ name: "a" }), NumberColumn({ name: "b" })],
+ rows: [],
+ },
+ },
+ ];
+ it("should extract results_timezone", () => {
+ const timezone = getTimezone(series);
+ expect(timezone).toBe("US/Eastern");
+ });
+ it("should extract results_timezone after series is transformed", () => {
+ const { series: transformed } = getVisualizationTransformed(series);
+ const timezone = getTimezone(transformed);
+ expect(timezone).toBe("US/Eastern");
+ });
+ });
});
diff --git a/modules/drivers/druid/test/metabase/driver/druid_test.clj b/modules/drivers/druid/test/metabase/driver/druid_test.clj
index cbb1d475f88..72ebd08a0c3 100644
--- a/modules/drivers/druid/test/metabase/driver/druid_test.clj
+++ b/modules/drivers/druid/test/metabase/driver/druid_test.clj
@@ -121,9 +121,10 @@
:display_name "count"
:base_type :type/Integer
:field_ref [:field-literal "count" :type/Integer]}])
- :native_form {:query native-query-1}}
- :expected_timezone "UTC"
- :actual_timezone "UTC"}
+ :native_form {:query native-query-1}
+ :requested_timezone "UTC"
+ :results_timezone "UTC"}}
+
(-> (process-native-query native-query-1)
(m/dissoc-in [:data :insights])))
diff --git a/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj b/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj
index 718618cfe9a..8249f0217ba 100644
--- a/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj
+++ b/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj
@@ -264,25 +264,25 @@
(expect
{:row_count 1
:status :completed
- :data {:rows [["Toucan Sighting" 1000]]
- :native_form expected-ga-query
- :cols [{:description "This is ga:eventLabel"
- :special_type nil
- :name "ga:eventLabel"
- :settings nil
- :source :breakout
- :parent_id nil
- :visibility_type :normal
- :display_name "ga:eventLabel"
- :fingerprint nil
- :base_type :type/Text}
- {:name "metric"
- :display_name "metric"
- :source :aggregation
- :description "This is metric"
- :base_type :type/Text}]}
- :expected_timezone "UTC"
- :actual_timezone "UTC"}
+ :data {:rows [["Toucan Sighting" 1000]]
+ :native_form expected-ga-query
+ :cols [{:description "This is ga:eventLabel"
+ :special_type nil
+ :name "ga:eventLabel"
+ :settings nil
+ :source :breakout
+ :parent_id nil
+ :visibility_type :normal
+ :display_name "ga:eventLabel"
+ :fingerprint nil
+ :base_type :type/Text}
+ {:name "metric"
+ :display_name "metric"
+ :source :aggregation
+ :description "This is metric"
+ :base_type :type/Text}]
+ :requested_timezone "UTC"
+ :results_timezone "UTC"}}
(with-redefs [ga/memoized-column-metadata (fn [_ column-name]
{:display_name column-name
:description (str "This is " column-name)
diff --git a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
index ce0b1139d25..afe1794dd40 100644
--- a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
+++ b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
@@ -74,16 +74,16 @@
(datasets/test-driver :mongo
(is (= {:status :completed
:row_count 1
- :data {:rows [[1]]
- :cols [{:name "count"
- :display_name "count"
- :base_type :type/Integer
- :source :native
- :field_ref [:field-literal "count" :type/Integer]}]
- :native_form {:collection "venues"
- :query native-query}}
- :expected_timezone "UTC"
- :actual_timezone "UTC"}
+ :data {:rows [[1]]
+ :cols [{:name "count"
+ :display_name "count"
+ :base_type :type/Integer
+ :source :native
+ :field_ref [:field-literal "count" :type/Integer]}]
+ :native_form {:collection "venues"
+ :query native-query}
+ :requested_timezone "UTC"
+ :results_timezone "UTC"}}
(-> (qp/process-query {:native {:query native-query
:collection "venues"}
:type :native
diff --git a/package.json b/package.json
index fc9786df496..226923fca1b 100644
--- a/package.json
+++ b/package.json
@@ -36,6 +36,7 @@
"leaflet.heat": "^0.2.0",
"lodash.memoize": "^4.1.2",
"moment": "2.19.3",
+ "moment-timezone": "^0.5.26",
"mustache": "^2.3.2",
"node-libs-browser": "^2.0.0",
"normalizr": "^3.0.2",
diff --git a/src/metabase/api/public.clj b/src/metabase/api/public.clj
index e5fa927fbb9..a8908795ce7 100644
--- a/src/metabase/api/public.clj
+++ b/src/metabase/api/public.clj
@@ -75,7 +75,7 @@
[results]
(u/select-nested-keys
results
- [[:data :cols :rows :rows_truncated :insights] [:json_query :parameters] :status :expected_timezone :actual_timezone]))
+ [[:data :cols :rows :rows_truncated :insights :requested_timezone :results_timezone] [:json_query :parameters] :status]))
(defmethod transform-results :failed
[{:keys [error], error-type :error_type, :as results}]
diff --git a/src/metabase/query_processor/middleware/add_settings.clj b/src/metabase/query_processor/middleware/add_settings.clj
index b7a91bbfd90..ed012987a33 100644
--- a/src/metabase/query_processor/middleware/add_settings.clj
+++ b/src/metabase/query_processor/middleware/add_settings.clj
@@ -43,6 +43,8 @@
query (cond-> query
settings (assoc :settings settings))
results (qp query)]
- (assoc results
- :expected_timezone (expected-timezone-id)
- :actual_timezone (actual-timezone-id)))))
+ (update results
+ :data
+ assoc
+ :requested_timezone (expected-timezone-id)
+ :results_timezone (actual-timezone-id)))))
diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj
index 5ca6b0989ba..0d4317cbb3b 100644
--- a/test/metabase/api/dataset_test.clj
+++ b/test/metabase/api/dataset_test.clj
@@ -59,9 +59,11 @@
(testing "Just a basic sanity check to make sure Query Processor endpoint is still working correctly."
(let [result ((test-users/user->client :rasta) :post 200 "dataset" (data/mbql-query checkins
{:aggregation [[:count]]}))]
- (is (= {:data {:rows [[1000]]
- :cols [(tu/obj->json->obj (qp.test/aggregate-col :count))]
- :native_form true}
+ (is (= {:data {:rows [[1000]]
+ :cols [(tu/obj->json->obj (qp.test/aggregate-col :count))]
+ :native_form true
+ :results_timezone "UTC"
+ :requested_timezone "UTC"}
:row_count 1
:status "completed"
:context "ad-hoc"
@@ -73,8 +75,6 @@
:started_at true
:running_time true
:average_execution_time nil
- :actual_timezone "UTC"
- :expected_timezone "UTC"
:database_id (data/id)}
(format-response result)))
(is (= {:hash true
@@ -306,10 +306,12 @@
(deftest report-timezone-test
(datasets/test-driver :postgres
- (is (= {:expected_timezone "US/Pacific"
- :actual_timezone "US/Pacific"}
+ (is (= {:requested_timezone "US/Pacific"
+ :results_timezone "US/Pacific"}
(tu/with-temporary-setting-values [report-timezone "US/Pacific"]
(let [results ((test-users/user->client :rasta) :post 200 "dataset" (data/mbql-query checkins
{:aggregation [[:count]]}))]
- (select-keys results [:expected_timezone :actual_timezone]))))
- "expected (desired) and actual timezone should be returned as part of query results")))
+ (-> results
+ :data
+ (select-keys [:requested_timezone :results_timezone]))))
+ "expected (desired) and actual timezone should be returned as part of query results"))))
diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj
index 7d5338a6820..3b1de6a2476 100644
--- a/test/metabase/api/embed_test.clj
+++ b/test/metabase/api/embed_test.clj
@@ -73,13 +73,14 @@
(defn successful-query-results
([]
- {:data {:cols [(tu/obj->json->obj (qp.test/aggregate-col :count))]
- :rows [[100]]
- :insights nil}
+ {:data {:cols [(tu/obj->json->obj (qp.test/aggregate-col :count))]
+ :rows [[100]]
+ :insights nil
+ :results_timezone "UTC"
+ :requested_timezone "UTC"
+ }
:json_query {:parameters nil}
- :status "completed"
- :actual_timezone "UTC"
- :expected_timezone "UTC"})
+ :status "completed"})
([results-format]
(case results-format
diff --git a/test/metabase/api/public_test.clj b/test/metabase/api/public_test.clj
index 365aa64061b..7990843ebdf 100644
--- a/test/metabase/api/public_test.clj
+++ b/test/metabase/api/public_test.clj
@@ -271,7 +271,7 @@
:aggregation [[:count]]}}))
(expect
- #{:cols :rows :insights}
+ #{:cols :rows :insights :results_timezone :requested_timezone}
(tu/with-temporary-setting-values [enable-public-sharing true]
(tt/with-temp Card [{uuid :public_uuid} (card-with-trendline)]
(-> (http/client :get 200 (str "public/card/" uuid "/query"))
diff --git a/test/metabase/driver/sql_jdbc/native_test.clj b/test/metabase/driver/sql_jdbc/native_test.clj
index 8cbabd02dec..73b0647831c 100644
--- a/test/metabase/driver/sql_jdbc/native_test.clj
+++ b/test/metabase/driver/sql_jdbc/native_test.clj
@@ -14,16 +14,16 @@
(expect
{:status :completed
:row_count 2
- :data {:rows [[100]
- [99]]
- :cols [{:name "ID"
- :display_name "ID"
- :base_type :type/Integer
- :source :native
- :field_ref [:field-literal "ID" :type/Integer]}]
- :native_form {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2"}}
- :expected_timezone "UTC"
- :actual_timezone "UTC"}
+ :data {:rows [[100]
+ [99]]
+ :cols [{:name "ID"
+ :display_name "ID"
+ :base_type :type/Integer
+ :source :native
+ :field_ref [:field-literal "ID" :type/Integer]}]
+ :native_form {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2"}
+ :requested_timezone "UTC"
+ :results_timezone "UTC"}}
(-> (qp/process-query {:native {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2"}
:type :native
:database (data/id)})
@@ -34,26 +34,26 @@
(expect
{:status :completed
:row_count 2
- :data {:rows [[100 "Mohawk Bend" 46]
- [99 "Golden Road Brewing" 10]]
- :cols [{:name "ID"
- :display_name "ID"
- :source :native
- :base_type :type/Integer
- :field_ref [:field-literal "ID" :type/Integer]}
- {:name "NAME"
- :display_name "NAME"
- :source :native
- :base_type :type/Text
- :field_ref [:field-literal "NAME" :type/Text]}
- {:name "CATEGORY_ID"
- :display_name "CATEGORY_ID"
- :source :native
- :base_type :type/Integer
- :field_ref [:field-literal "CATEGORY_ID" :type/Integer]}]
- :native_form {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2"}}
- :expected_timezone "UTC"
- :actual_timezone "UTC"}
+ :data {:rows [[100 "Mohawk Bend" 46]
+ [99 "Golden Road Brewing" 10]]
+ :cols [{:name "ID"
+ :display_name "ID"
+ :source :native
+ :base_type :type/Integer
+ :field_ref [:field-literal "ID" :type/Integer]}
+ {:name "NAME"
+ :display_name "NAME"
+ :source :native
+ :base_type :type/Text
+ :field_ref [:field-literal "NAME" :type/Text]}
+ {:name "CATEGORY_ID"
+ :display_name "CATEGORY_ID"
+ :source :native
+ :base_type :type/Integer
+ :field_ref [:field-literal "CATEGORY_ID" :type/Integer]}]
+ :native_form {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2"}
+ :requested_timezone "UTC"
+ :results_timezone "UTC"}}
(-> (qp/process-query {:native {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2"}
:type :native
:database (data/id)})
diff --git a/test/metabase/query_processor/middleware/add_settings_test.clj b/test/metabase/query_processor/middleware/add_settings_test.clj
index cfd0c157fb9..3151f2f8f2b 100644
--- a/test/metabase/query_processor/middleware/add_settings_test.clj
+++ b/test/metabase/query_processor/middleware/add_settings_test.clj
@@ -36,23 +36,23 @@
"if the driver doesn't support `:set-timezone`, query should be unchanged, even if `report-timezone` is valid")))
(deftest post-processing-test
- (doseq [[driver timezone->expected] {::timezone-driver {"US/Pacific" {:actual_timezone "US/Pacific"
- :expected_timezone "US/Pacific"}
- nil {:actual_timezone "UTC"
- :expected_timezone "UTC"}}
- ::no-timezone-driver {"US/Pacific" {:actual_timezone "UTC"
- :expected_timezone "US/Pacific"}
- nil {:actual_timezone "UTC"
- :expected_timezone "UTC"}}}
+ (doseq [[driver timezone->expected] {::timezone-driver {"US/Pacific" {:results_timezone "US/Pacific"
+ :requested_timezone "US/Pacific"}
+ nil {:results_timezone "UTC"
+ :requested_timezone "UTC"}}
+ ::no-timezone-driver {"US/Pacific" {:results_timezone "UTC"
+ :requested_timezone "US/Pacific"}
+ nil {:results_timezone "UTC"
+ :requested_timezone "UTC"}}}
[timezone expected] timezone->expected]
(testing driver
(tu/with-temporary-setting-values [report-timezone timezone]
(driver/with-driver driver
- (is (= (assoc expected :results? true)
+ (is (= expected
(let [query {:query? true}
results {:results? true}
add-settings (add-settings/add-settings (constantly results))]
- (add-settings query)))))))))
+ (:data (add-settings query))))))))))
(defn- env [_]
"SOME_VALUE")
diff --git a/yarn.lock b/yarn.lock
index 88c0a5f235b..1ae8720a999 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -8877,6 +8877,13 @@ module-deps-sortable@4.0.6:
through2 "^2.0.0"
xtend "^4.0.0"
+moment-timezone@^0.5.26:
+ version "0.5.26"
+ resolved "https://registry.yarnpkg.com/moment-timezone/-/moment-timezone-0.5.26.tgz#c0267ca09ae84631aa3dc33f65bedbe6e8e0d772"
+ integrity sha512-sFP4cgEKTCymBBKgoxZjYzlSovC20Y6J7y3nanDc5RoBIXKlZhoYwBoZGe3flwU6A372AcRwScH8KiwV6zjy1g==
+ dependencies:
+ moment ">= 2.9.0"
+
moment@2.19.3:
version "2.19.3"
resolved "https://registry.yarnpkg.com/moment/-/moment-2.19.3.tgz#bdb99d270d6d7fda78cc0fbace855e27fe7da69f"
@@ -8887,6 +8894,11 @@ moment@2.x.x:
resolved "https://registry.yarnpkg.com/moment/-/moment-2.20.1.tgz#d6eb1a46cbcc14a2b2f9434112c1ff8907f313fd"
integrity sha512-Yh9y73JRljxW5QxN08Fner68eFLxM5ynNOAw2LbIB1YAGeQzZT8QFSUvkAz609Zf+IHhhaUxqZK8dG3W/+HEvg==
+"moment@>= 2.9.0":
+ version "2.24.0"
+ resolved "https://registry.yarnpkg.com/moment/-/moment-2.24.0.tgz#0d055d53f5052aa653c9f6eb68bb5d12bf5c2b5b"
+ integrity sha512-bV7f+6l2QigeBBZSM/6yTNq4P2fNpSWj/0e7jQcy87A8e7o2nAfP/34/2ky5Vw4B9S446EtIhodAzkFCcR4dQg==
+
moo@^0.4.3:
version "0.4.3"
resolved "https://registry.yarnpkg.com/moo/-/moo-0.4.3.tgz#3f847a26f31cf625a956a87f2b10fbc013bfd10e"
--
GitLab