Filter nulls out of non-ordinal x axes (#10846)

frontend/src/metabase/visualizations/lib/renderer_utils.js

@@ -129,9 +129,19 @@ function getParseOptions({ settings, data }) {
 }
 
 export function getDatas({ settings, series }, warn) {
+  const isNotOrdinal = !isOrdinal(settings);
   return series.map(({ data }) => {
+    // non-ordinal dimensions can't display null values,
+    // so we filter them out and display a warning
+    const rows = isNotOrdinal
+      ? data.rows.filter(([x]) => x !== null)
+      : data.rows;
+    if (rows.length < data.rows.length) {
+      warn(nullDimensionWarning());
+    }
+
     const parseOptions = getParseOptions({ settings, data });
-    return data.rows.map(row => {
+    return rows.map(row => {
       const [x, ...rest] = row;
       const newRow = [parseXValue(x, parseOptions, warn), ...rest];
       newRow._origin = row._origin;
@@ -143,6 +153,7 @@ export function getDatas({ settings, series }, warn) {
 export function getXValues({ settings, series }) {
   // if _raw isn't set then we already have the raw series
   const { _raw: rawSeries = series } = series;
+  const isNotOrdinal = !isOrdinal(settings);
   const warn = () => {}; // no op since warning in handled by getDatas
   const uniqueValues = new Set();
   let isAscending = true;
@@ -155,6 +166,10 @@ export function getXValues({ settings, series }) {
     const parseOptions = getParseOptions({ settings, data });
     let lastValue;
     for (const row of data.rows) {
+      // non ordinal dimensions can't display null values, so we exclude them from xValues
+      if (isNotOrdinal && row[columnIndex] === null) {
+        continue;
+      }
       const value = parseXValue(row[columnIndex], parseOptions, warn);
       if (lastValue !== undefined) {
         isAscending = isAscending && lastValue <= value;

frontend/test/metabase/visualizations/lib/renderer_utils.unit.spec.js

 import moment from "moment";
 
 import {
+  getDatas,
   getXValues,
   parseXValue,
 } from "metabase/visualizations/lib/renderer_utils";
 
 describe("getXValues", () => {
-  function getXValuesForRows(listOfRows) {
+  function getXValuesForRows(listOfRows, settings = {}) {
     const series = listOfRows.map(rows => ({ data: { rows, cols: [{}] } }));
     series._raw = series;
-    const settings = {};
     return getXValues({ settings, series });
   }
 
@@ -119,6 +119,26 @@ describe("getXValues", () => {
       "2019-08-13T00:00:00Z",
     ]);
   });
+  it("should include nulls by default", () => {
+    const xValues = getXValuesForRows([[["foo"], [null], ["bar"]]]);
+    expect(xValues).toEqual(["foo", "(empty)", "bar"]);
+  });
+  it("should exclude nulls for histograms", () => {
+    const xValues = getXValuesForRows([[["foo"], [null], ["bar"]]], {
+      "graph.x_axis.scale": "histogram",
+    });
+    expect(xValues).toEqual(["foo", "bar"]);
+  });
+  it("should exclude nulls for timeseries", () => {
+    const xValues = getXValuesForRows(
+      [[["2019-01-02"], [null], ["2019-01-03"]]],
+      {
+        "graph.x_axis.scale": "timeseries",
+      },
+    );
+    const formattedXValues = xValues.map(v => v.format("YYYY-MM-DD"));
+    expect(formattedXValues).toEqual(["2019-01-02", "2019-01-03"]);
+  });
 });
 
 describe("parseXValue", () => {
@@ -136,3 +156,23 @@ describe("parseXValue", () => {
     expect(warn.mock.calls.length).toBe(2);
   });
 });
+
+describe("getDatas", () => {
+  it("should include rows with a null dimension by default", () => {
+    const settings = {};
+    const series = [{ data: { rows: [["foo"], [null], ["bar"]], cols: [{}] } }];
+    const warn = jest.fn();
+    const xValues = getDatas({ settings, series }, warn);
+    expect(xValues).toEqual([[["foo"], ["(empty)"], ["bar"]]]);
+  });
+  it("should exclude rows with null dimension for histograms", () => {
+    const settings = { "graph.x_axis.scale": "histogram" };
+    const series = [{ data: { rows: [["foo"], [null], ["bar"]], cols: [{}] } }];
+    const warn = jest.fn();
+    const xValues = getDatas({ settings, series }, warn);
+    expect(xValues).toEqual([[["foo"], ["bar"]]]);
+    expect(warn.mock.calls.length).toBe(1);
+    const [{ key: warningKey }] = warn.mock.calls[0];
+    expect(warningKey).toBe("NULL_DIMENSION_WARNING");
+  });
+});