From a716bd803c79b82cab70b4dd25110cb192097e9f Mon Sep 17 00:00:00 2001
From: Benoit Vinay <ben@benoitvinay.com>
Date: Fri, 6 May 2022 10:34:19 +0200
Subject: [PATCH] Pin map with null locations removed from Map props (#22425)
* Pin map with null locations removed from Map props
* Unskip cypress test
* mapProps used for Map component
* Rounding lat/long numbers in tests
* data-testid added to no results vizualisation
* Fixed tests for map with only null values
* Simplified rows filtering
---
.../metabase/visualizations/components/PinMap.jsx | 13 +++++++++----
.../visualizations/components/Visualization.jsx | 2 +-
.../18061-maps-only-nulls-crash-frontend.cy.spec.js | 9 ++++-----
...8063-maps-null-location-wrong-tooltip.cy.spec.js | 7 +++----
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/frontend/src/metabase/visualizations/components/PinMap.jsx b/frontend/src/metabase/visualizations/components/PinMap.jsx
index b0a1791e57e..fe377f4b7f7 100644
--- a/frontend/src/metabase/visualizations/components/PinMap.jsx
+++ b/frontend/src/metabase/visualizations/components/PinMap.jsx
@@ -130,9 +130,11 @@ export default class PinMap extends Component {
]);
// only use points with numeric coordinates & metric
- const points = allPoints.filter(
+ const validPoints = allPoints.map(
([lat, lng, metric]) => lat != null && lng != null && metric != null,
);
+ const points = allPoints.filter((_, i) => validPoints[i]);
+ const updatedRows = rows.filter((_, i) => validPoints[i]);
const warnings = [];
const filteredRows = allPoints.length - points.length;
@@ -166,7 +168,7 @@ export default class PinMap extends Component {
bounds._northEast.lat += binHeight;
}
- return { points, bounds, min, max, binWidth, binHeight };
+ return { rows: updatedRows, points, bounds, min, max, binWidth, binHeight };
}
render() {
@@ -176,7 +178,10 @@ export default class PinMap extends Component {
const Map = MAP_COMPONENTS_BY_TYPE[settings["map.pin_type"]];
- const { points, bounds, min, max, binHeight, binWidth } = this.state;
+ const { rows, points, bounds, min, max, binHeight, binWidth } = this.state;
+
+ const mapProps = { ...this.props };
+ mapProps.series[0].data.rows = rows;
return (
<div
@@ -188,7 +193,7 @@ export default class PinMap extends Component {
>
{Map ? (
<Map
- {...this.props}
+ {...mapProps}
ref={map => (this._map = map)}
className="absolute top left bottom right z1"
onMapCenterChange={this.onMapCenterChange}
diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx
index 696145be2cf..a2681af3624 100644
--- a/frontend/src/metabase/visualizations/components/Visualization.jsx
+++ b/frontend/src/metabase/visualizations/components/Visualization.jsx
@@ -476,7 +476,7 @@ export default class Visualization extends React.PureComponent {
}
>
<Tooltip tooltip={t`No results!`} isEnabled={small}>
- <img src={NoResults} />
+ <img data-testid="no-results-image" src={NoResults} />
</Tooltip>
{!small && <span className="h4 text-bold">No results!</span>}
</div>
diff --git a/frontend/test/metabase/scenarios/visualizations/reproductions/18061-maps-only-nulls-crash-frontend.cy.spec.js b/frontend/test/metabase/scenarios/visualizations/reproductions/18061-maps-only-nulls-crash-frontend.cy.spec.js
index 1de64da699a..ab49a90c4e5 100644
--- a/frontend/test/metabase/scenarios/visualizations/reproductions/18061-maps-only-nulls-crash-frontend.cy.spec.js
+++ b/frontend/test/metabase/scenarios/visualizations/reproductions/18061-maps-only-nulls-crash-frontend.cy.spec.js
@@ -141,15 +141,14 @@ describe("issue 18061", () => {
it("should handle data sets that contain only null values for longitude/latitude (metabase#18061-3)", () => {
visitAlias("@publicLink");
+ cy.findByText("18061D");
+ cy.findByText("18061");
cy.get(".PinMap");
addFilter("Twitter");
-
cy.location("search").should("eq", "?category=Twitter");
-
- cy.findByText("18061D");
- cy.findByText("18061");
- cy.get(".PinMap");
+ cy.findAllByTestId("no-results-image");
+ cy.get(".PinMap").should("not.exist");
});
});
});
diff --git a/frontend/test/metabase/scenarios/visualizations/reproductions/18063-maps-null-location-wrong-tooltip.cy.spec.js b/frontend/test/metabase/scenarios/visualizations/reproductions/18063-maps-null-location-wrong-tooltip.cy.spec.js
index fb047ffad0e..3871f0318da 100644
--- a/frontend/test/metabase/scenarios/visualizations/reproductions/18063-maps-null-location-wrong-tooltip.cy.spec.js
+++ b/frontend/test/metabase/scenarios/visualizations/reproductions/18063-maps-null-location-wrong-tooltip.cy.spec.js
@@ -10,7 +10,7 @@ const questionDetails = {
display: "map",
};
-describe.skip("issue 18063", () => {
+describe("issue 18063", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
@@ -30,7 +30,6 @@ describe.skip("issue 18063", () => {
// Click anywhere to close both popovers that open automatically.
// Please see: https://github.com/metabase/metabase/issues/18063#issuecomment-927836691
cy.findByText("Map type").click();
- cy.findByText("Map type").click();
});
it("should show the correct tooltip details for pin map even when some locations are null (metabase#18063)", () => {
@@ -40,8 +39,8 @@ describe.skip("issue 18063", () => {
cy.get(".leaflet-marker-icon").trigger("mousemove");
popover().within(() => {
- testPairedTooltipValues("LATITUDE", "55.6761");
- testPairedTooltipValues("LONGITUDE", "12.5683");
+ testPairedTooltipValues("LATITUDE", "55.68");
+ testPairedTooltipValues("LONGITUDE", "12.57");
testPairedTooltipValues("COUNT", "1");
testPairedTooltipValues("NAME", "Copenhagen");
});
--
GitLab