From f074e561de57533ccd78f8505a8c11b6fae6801a Mon Sep 17 00:00:00 2001
From: Paul Rosenzweig <paulrosenzweig@users.noreply.github.com>
Date: Fri, 8 May 2020 14:05:29 -0400
Subject: [PATCH] Filter nulls in pin maps (#12159)

Co-authored-by: Morgan Hankins <morganhankins@gmail.com>
Co-authored-by: Tom Robinson <tlrobinson@gmail.com>
---
 .../src/metabase/meta/types/Visualization.js  |  2 ++
 .../visualizations/components/PinMap.jsx      | 18 ++++++++++++-
 .../components/PinMap.unit.spec.js            | 26 +++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 frontend/test/metabase/visualizations/components/PinMap.unit.spec.js

diff --git a/frontend/src/metabase/meta/types/Visualization.js b/frontend/src/metabase/meta/types/Visualization.js
index dbbb7b7bd34..fcef2b11f25 100644
--- a/frontend/src/metabase/meta/types/Visualization.js
+++ b/frontend/src/metabase/meta/types/Visualization.js
@@ -113,4 +113,6 @@ export type VisualizationProps = {
   onAddSeries?: Function,
   onEditSeries?: Function,
   onRemoveSeries?: Function,
+
+  onUpdateWarnings?: Function,
 };
diff --git a/frontend/src/metabase/visualizations/components/PinMap.jsx b/frontend/src/metabase/visualizations/components/PinMap.jsx
index df5c77e945e..72a22c85965 100644
--- a/frontend/src/metabase/visualizations/components/PinMap.jsx
+++ b/frontend/src/metabase/visualizations/components/PinMap.jsx
@@ -126,6 +126,7 @@ export default class PinMap extends Component {
           data: { cols, rows },
         },
       ],
+      onUpdateWarnings,
     } = props;
     const latitudeIndex = _.findIndex(
       cols,
@@ -140,12 +141,27 @@ export default class PinMap extends Component {
       col => col.name === settings["map.metric_column"],
     );
 
-    const points = rows.map(row => [
+    const allPoints = rows.map(row => [
       row[latitudeIndex],
       row[longitudeIndex],
       metricIndex >= 0 ? row[metricIndex] : 1,
     ]);
 
+    // only use points with numeric coordinates & metric
+    const points = allPoints.filter(
+      ([lat, lng, metric]) => lat != null && lng != null && metric != null,
+    );
+
+    const warnings = [];
+    const filteredRows = allPoints.length - points.length;
+    if (filteredRows > 0) {
+      warnings.push(
+        t`We filtered out ${filteredRows} row(s) containing null values.`,
+      );
+    }
+    // $FlowFixMe flow thinks warnings can be undefined
+    onUpdateWarnings(warnings);
+
     const bounds = L.latLngBounds(points);
 
     const min = d3.min(points, point => point[2]);
diff --git a/frontend/test/metabase/visualizations/components/PinMap.unit.spec.js b/frontend/test/metabase/visualizations/components/PinMap.unit.spec.js
new file mode 100644
index 00000000000..3fa8c95702e
--- /dev/null
+++ b/frontend/test/metabase/visualizations/components/PinMap.unit.spec.js
@@ -0,0 +1,26 @@
+import PinMap from "metabase/visualizations/components/PinMap";
+
+describe("PinMap", () => {
+  it("should filter out rows with null values", () => {
+    const onUpdateWarnings = jest.fn();
+    const data = {
+      cols: ["lat", "lng", "metric"].map(name => ({ name })),
+      rows: [[null, 0, 0], [0, null, 0], [0, 0, null], [0, 0, 0]],
+    };
+    const props = {
+      settings: {
+        "map.latitude_column": "lat",
+        "map.longitude_column": "lng",
+        "map.metric_column": "metric",
+      },
+      series: [{ data }],
+      onUpdateWarnings,
+    };
+    const { points } = new PinMap(props)._getPoints(props);
+
+    expect(points).toEqual([[0, 0, 0]]);
+    expect(onUpdateWarnings.mock.calls[0][0]).toEqual([
+      "We filtered out 3 row(s) containing null values.",
+    ]);
+  });
+});
-- 
GitLab