From 8293efd2f06836344186085f355de4d82d2b38b6 Mon Sep 17 00:00:00 2001
From: Kamil Mielnik <kamil@kamilmielnik.com>
Date: Fri, 30 Jun 2023 16:38:47 +0700
Subject: [PATCH] Fix - Adding filter of summarized field grouped by latitude
and longitude on a map causes table visualization error (#31952)
* Fix #30058
* Add missing setting key
* Add a unit test attempt at #30058
* Mock store instead of using deprecated MetabaseSettings.set
* Remove LeafletGridHeatMap unit tests
* Add e2e test for #30058
* Fix typing
* Don't allow undefined "map-tile-server-url"
* Tag the test with repo and issue id
---
.../reproductions/30058-map-crash.cy.spec.js | 61 +++++++++++++++++++
.../src/metabase-types/api/mocks/settings.ts | 1 +
frontend/src/metabase-types/api/settings.ts | 1 +
.../components/LeafletGridHeatMap.jsx | 4 +-
4 files changed, 65 insertions(+), 2 deletions(-)
create mode 100644 e2e/test/scenarios/visualizations/reproductions/30058-map-crash.cy.spec.js
diff --git a/e2e/test/scenarios/visualizations/reproductions/30058-map-crash.cy.spec.js b/e2e/test/scenarios/visualizations/reproductions/30058-map-crash.cy.spec.js
new file mode 100644
index 00000000000..362ed9cf3be
--- /dev/null
+++ b/e2e/test/scenarios/visualizations/reproductions/30058-map-crash.cy.spec.js
@@ -0,0 +1,61 @@
+import { popover, restore, visitQuestionAdhoc } from "e2e/support/helpers";
+
+import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
+import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
+
+const { PEOPLE, PEOPLE_ID } = SAMPLE_DATABASE;
+
+const testQuery = {
+ type: "query",
+ query: {
+ "source-query": {
+ "source-table": PEOPLE_ID,
+ aggregation: [["count"]],
+ breakout: [
+ [
+ "field",
+ PEOPLE.LATITUDE,
+ { "base-type": "type/Float", binning: { strategy: "default" } },
+ ],
+ [
+ "field",
+ PEOPLE.LONGITUDE,
+ { "base-type": "type/Float", binning: { strategy: "default" } },
+ ],
+ ],
+ },
+ },
+ database: SAMPLE_DB_ID,
+};
+
+describe("issue 30058", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsNormalUser();
+ });
+
+ it("visualization does not crash after adding a filter (metabase#30058)", () => {
+ visitQuestionAdhoc({
+ dataset_query: testQuery,
+ display: "map",
+ displayIsLocked: true,
+ });
+
+ cy.icon("notebook").click();
+ cy.button("Filter").click();
+ popover().within(() => {
+ cy.findByText("Count").click();
+ cy.icon("chevrondown").click();
+ });
+ cy.findByTestId("operator-select-list").findByText("Greater than").click();
+ popover().within(() => {
+ cy.findByPlaceholderText("Enter a number").type("2");
+ cy.findByText("Add filter").click();
+ });
+
+ cy.button("Visualize").click();
+ cy.wait("@dataset");
+
+ cy.get(".Icon-warning").should("not.exist");
+ });
+});
diff --git a/frontend/src/metabase-types/api/mocks/settings.ts b/frontend/src/metabase-types/api/mocks/settings.ts
index f4964cb4783..06a249433cd 100644
--- a/frontend/src/metabase-types/api/mocks/settings.ts
+++ b/frontend/src/metabase-types/api/mocks/settings.ts
@@ -163,6 +163,7 @@ export const createMockSettings = (opts?: Partial<Settings>): Settings => ({
"ldap-configured?": false,
"ldap-enabled": false,
"loading-message": "doing-science",
+ "map-tile-server-url": "https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png",
"openai-api-key": null,
"openai-organization": null,
"openai-model": null,
diff --git a/frontend/src/metabase-types/api/settings.ts b/frontend/src/metabase-types/api/settings.ts
index 6915955c22a..4fc600c0884 100644
--- a/frontend/src/metabase-types/api/settings.ts
+++ b/frontend/src/metabase-types/api/settings.ts
@@ -205,6 +205,7 @@ export interface Settings {
"ldap-configured?": boolean;
"ldap-enabled": boolean;
"loading-message": LoadingMessage;
+ "map-tile-server-url": string;
"openai-api-key": string | null;
"openai-organization": string | null;
"openai-model": string | null;
diff --git a/frontend/src/metabase/visualizations/components/LeafletGridHeatMap.jsx b/frontend/src/metabase/visualizations/components/LeafletGridHeatMap.jsx
index e2d57be15b1..ccbc0eb679a 100644
--- a/frontend/src/metabase/visualizations/components/LeafletGridHeatMap.jsx
+++ b/frontend/src/metabase/visualizations/components/LeafletGridHeatMap.jsx
@@ -66,8 +66,6 @@ export default class LeafletGridHeatMap extends LeafletMap {
const longitureValues = points.map(row => row[longitudeIndex]);
for (let i = 0; i < totalSquares; i++) {
- const [latitude, longiture, metric] = points[i];
-
if (i >= points.length) {
gridLayer.removeLayer(gridSquares[i]);
}
@@ -78,6 +76,8 @@ export default class LeafletGridHeatMap extends LeafletMap {
}
if (i < points.length) {
+ const [latitude, longiture, metric] = points[i];
+
gridSquares[i].setStyle({ color: colorScale(metric) });
const [latMin, latMax] = getValueRange(
--
GitLab