From 9d8201f83d124ce043f03d7b19899aaa9be4b187 Mon Sep 17 00:00:00 2001
From: "metabase-bot[bot]"
<109303359+metabase-bot[bot]@users.noreply.github.com>
Date: Wed, 3 Apr 2024 13:11:48 +0000
Subject: [PATCH] Fix row id mapping in object detail modal displaying (#40874)
(#40951)
* Map row id on object detail id
* Add a fallback
* Add e2e test
* Add a limit
* Provide a fix for #34070
* Simplify test
Co-authored-by: Uladzimir Havenchyk <125459446+uladzimirdev@users.noreply.github.com>
---
.../object_detail.cy.spec.js | 107 +++++++++++++++++-
.../src/metabase/query_builder/selectors.js | 23 ++++
.../TableInteractive/TableInteractive.jsx | 21 +++-
3 files changed, 141 insertions(+), 10 deletions(-)
diff --git a/e2e/test/scenarios/visualizations-tabular/object_detail.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/object_detail.cy.spec.js
index 5329b04889f..007d0119001 100644
--- a/e2e/test/scenarios/visualizations-tabular/object_detail.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-tabular/object_detail.cy.spec.js
@@ -12,10 +12,19 @@ import {
getTableId,
visitPublicQuestion,
visitPublicDashboard,
+ createQuestion,
} from "e2e/support/helpers";
-const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID, PEOPLE, PEOPLE_ID } =
- SAMPLE_DATABASE;
+const {
+ ORDERS,
+ ORDERS_ID,
+ PRODUCTS,
+ PRODUCTS_ID,
+ PEOPLE,
+ PEOPLE_ID,
+ REVIEWS,
+ REVIEWS_ID,
+} = SAMPLE_DATABASE;
const FIRST_ORDER_ID = 9676;
const SECOND_ORDER_ID = 10874;
@@ -45,7 +54,7 @@ describe("scenarios > question > object details", { tags: "@slow" }, () => {
cy.signInAsAdmin();
});
- it("shows correct object detail card for questions with joins (metabase #27094)", () => {
+ it("shows correct object detail card for questions with joins (metabase#27094)", () => {
const questionDetails = {
name: "14775",
query: {
@@ -74,6 +83,96 @@ describe("scenarios > question > object details", { tags: "@slow" }, () => {
});
});
+ it("shows correct object detail card for questions with joins after clicking on view details (metabase#39477)", () => {
+ const questionDetails = {
+ name: "39477",
+ query: {
+ "source-table": ORDERS_ID,
+ joins: [
+ {
+ fields: "all",
+ "source-table": PRODUCTS_ID,
+ condition: [
+ "=",
+ ["field-id", ORDERS.PRODUCT_ID],
+ ["joined-field", "Products", ["field-id", PRODUCTS.ID]],
+ ],
+ alias: "Products",
+ },
+ ],
+ limit: 5,
+ },
+ };
+
+ createQuestion(questionDetails, { visitQuestion: true });
+
+ cy.log("check click on 1st row");
+
+ cy.get(".cellData").contains("37.65").realHover();
+ cy.findByTestId("detail-shortcut").findByRole("button").click();
+
+ cy.findByTestId("object-detail").within(() => {
+ cy.get("h2").should("contain", "Order").should("contain", 1);
+ cy.findByTestId("object-detail-close-button").click();
+ });
+
+ cy.log("check click on 3rd row");
+
+ cy.get(".cellData").contains("52.72").realHover();
+ cy.findByTestId("detail-shortcut").findByRole("button").click();
+
+ cy.findByTestId("object-detail").within(() => {
+ cy.get("h2").should("contain", "Order").should("contain", 3);
+ cy.findByText("52.72");
+ });
+ });
+
+ it("applies correct filter (metabase#34070)", () => {
+ const questionDetails = {
+ name: "34070",
+ query: {
+ "source-table": PRODUCTS_ID,
+ joins: [
+ {
+ fields: "all",
+ alias: "Products",
+ condition: [
+ "=",
+ [
+ "field",
+ PRODUCTS.ID,
+ {
+ "base-type": "type/BigInteger",
+ },
+ ],
+ [
+ "field",
+ REVIEWS.PRODUCT_ID,
+ {
+ "base-type": "type/BigInteger",
+ "join-alias": "Products",
+ },
+ ],
+ ],
+ "source-table": REVIEWS_ID,
+ },
+ ],
+ limit: 10,
+ },
+ };
+
+ createQuestion(questionDetails, { visitQuestion: true });
+
+ cy.get(".cellData").contains("4966277046676").realHover();
+ cy.findByTestId("detail-shortcut").findByRole("button").click();
+
+ cy.findByRole("dialog").findByTestId("fk-relation-orders").click();
+
+ cy.findByTestId("qb-filters-panel")
+ .findByText("Product ID is 3")
+ .should("be.visible");
+ });
+
it("handles browsing records by PKs", () => {
cy.createQuestion(TEST_QUESTION, { visitQuestion: true });
drillPK({ id: FIRST_ORDER_ID });
@@ -212,7 +311,7 @@ describe("scenarios > question > object details", { tags: "@slow" }, () => {
cy.findByTestId("object-detail").findByText("Searsboro").click();
});
- it("should work with non-numeric IDs (metabse#22768)", () => {
+ it("should work with non-numeric IDs (metabase#22768)", () => {
cy.request("PUT", `/api/field/${PRODUCTS.ID}`, {
semantic_type: null,
});
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index a2c1ef2130c..d9fb7bf4fc4 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -182,6 +182,29 @@ export const getPKRowIndexMap = createSelector(
},
);
+// it's very similar to `getPKRowIndexMap` but it is required for covering "view details" click
+// we don't have objectId there, only rowId, mapping from `getPKRowIndexMap` is opposite
+// if rows are showing the same PK, only last one will have the entry in the map
+// and we'll not know which object to show
+export const getRowIndexToPKMap = createSelector(
+ [getFirstQueryResult, getPKColumnIndex],
+ (result, PKColumnIndex) => {
+ if (!result || !Number.isSafeInteger(PKColumnIndex)) {
+ return {};
+ }
+ const { rows } = result.data;
+ if (PKColumnIndex < 0) {
+ return rows.map((_, index) => index);
+ }
+ const map = {};
+ rows.forEach((row, index) => {
+ const PKValue = row[PKColumnIndex];
+ map[index] = PKValue;
+ });
+ return map;
+ },
+);
+
export const getQueryStartTime = state => state.qb.queryStartTime;
export const getDatabaseId = createSelector(
diff --git a/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx b/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx
index 6108edc4a0e..bc18e35ce3b 100644
--- a/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx
+++ b/frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx
@@ -19,7 +19,10 @@ import Tooltip from "metabase/core/components/Tooltip";
import { getScrollBarSize } from "metabase/lib/dom";
import { formatValue } from "metabase/lib/formatting";
import { zoomInRow } from "metabase/query_builder/actions";
-import { getQueryBuilderMode } from "metabase/query_builder/selectors";
+import {
+ getRowIndexToPKMap,
+ getQueryBuilderMode,
+} from "metabase/query_builder/selectors";
import { EmotionCacheProvider } from "metabase/styled-components/components/EmotionCacheProvider";
import { Icon, DelayGroup } from "metabase/ui";
import {
@@ -75,6 +78,7 @@ function pickRowsToMeasure(rows, columnIndex, count = 10) {
const mapStateToProps = state => ({
queryBuilderMode: getQueryBuilderMode(state),
+ rowIndexToPkMap: getRowIndexToPKMap(state),
});
const mapDispatchToProps = dispatch => ({
@@ -477,11 +481,16 @@ class TableInteractive extends Component {
}
}
- pkClick = rowIndex => {
- const objectId = this.state.IDColumn
- ? this.props.data.rows[rowIndex][this.state.IDColumnIndex]
- : rowIndex;
- return e => this.props.onZoomRow(objectId);
+ pkClick = rowIndex => () => {
+ let objectId;
+
+ if (this.state.IDColumn) {
+ objectId = this.props.data.rows[rowIndex][this.state.IDColumnIndex];
+ } else {
+ objectId = this.props.rowIndexToPkMap[rowIndex] ?? rowIndex;
+ }
+
+ this.props.onZoomRow(objectId);
};
onKeyDown = event => {
--
GitLab