diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts
index 329da4e5aa91c451ca0660e215cf3f012861588d..a4c8a414a2c7c04ca176c17d294b47d9cec6037f 100644
--- a/frontend/src/metabase-lib/lib/Question.ts
+++ b/frontend/src/metabase-lib/lib/Question.ts
@@ -285,6 +285,10 @@ class QuestionInner {
return this.setCard(assoc(this.card(), "display", display));
}
+ /**
+ * returns whether this question is a model
+ * @returns boolean
+ */
isDataset() {
return this._card && this._card.dataset;
}
diff --git a/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx b/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx
index 9f1a194e18db6a6ad820780b4809069dcb49a7bb..9308f2fc7edc55b3362aaa88f09376b72ed8316f 100644
--- a/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx
+++ b/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx
@@ -3,8 +3,10 @@ import { isFK, isPK } from "metabase/lib/schema_metadata";
import { zoomInRow } from "metabase/query_builder/actions";
function hasManyPKColumns(question) {
- const table = question.query().table();
- const fields = table?.fields ?? question.getResultMetadata();
+ const fields = question.isDataset()
+ ? question.getResultMetadata() ?? question.query().table?.()?.fields
+ : question.query().table?.()?.fields ?? question.getResultMetadata();
+
return fields.filter(field => isPK(field)).length > 1;
}
diff --git a/frontend/src/metabase/query_builder/actions/object-detail.js b/frontend/src/metabase/query_builder/actions/object-detail.js
index e4a1f44e0f93ac4bd8128217c813ea18b592cce7..d0ede23ffddb1033041624551b5d64e258cce532 100644
--- a/frontend/src/metabase/query_builder/actions/object-detail.js
+++ b/frontend/src/metabase/query_builder/actions/object-detail.js
@@ -11,6 +11,7 @@ import { FieldDimension } from "metabase-lib/lib/Dimension";
import {
getCard,
getFirstQueryResult,
+ getPKColumnIndex,
getNextRowPKValue,
getPreviousRowPKValue,
getTableForeignKeys,
@@ -19,9 +20,12 @@ import { setCardAndRun } from "./core";
import { updateUrl } from "./navigation";
export const ZOOM_IN_ROW = "metabase/qb/ZOOM_IN_ROW";
-export const zoomInRow = ({ objectId }) => dispatch => {
+export const zoomInRow = ({ objectId }) => (dispatch, getState) => {
dispatch({ type: ZOOM_IN_ROW, payload: { objectId } });
- dispatch(updateUrl(null, { objectId, replaceState: false }));
+
+ // don't show object id in url if it is a row index
+ const hasPK = getPKColumnIndex(getState()) !== -1;
+ hasPK && dispatch(updateUrl(null, { objectId, replaceState: false }));
};
export const RESET_ROW_ZOOM = "metabase/qb/RESET_ROW_ZOOM";
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index f2610c131563aa1a8e86c23ab846f5f53c14e225..bbf0e8372d1a0452ba83cb3290d3f8b4f85890c2 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -140,6 +140,10 @@ export const getPKColumnIndex = createSelector(
return;
}
const { cols } = result.data;
+ const hasMultiplePks = cols.filter(isPK).length > 1;
+ if (hasMultiplePks) {
+ return -1;
+ }
return cols.findIndex(isPK);
},
);
@@ -151,6 +155,9 @@ export const getPKRowIndexMap = createSelector(
return {};
}
const { rows } = result.data;
+ if (PKColumnIndex < 0) {
+ return rows.map((_, index) => index);
+ }
const map = {};
rows.forEach((row, index) => {
const PKValue = row[PKColumnIndex];
@@ -426,6 +433,9 @@ export const getPreviousRowPKValue = createSelector(
if (!result) {
return;
}
+ if (PKColumnIndex === -1) {
+ return rowIndex - 1;
+ }
const { rows } = result.data;
return rows[rowIndex - 1][PKColumnIndex];
},
@@ -437,6 +447,9 @@ export const getNextRowPKValue = createSelector(
if (!result) {
return;
}
+ if (PKColumnIndex === -1) {
+ return rowIndex + 1;
+ }
const { rows } = result.data;
return rows[rowIndex + 1][PKColumnIndex];
},
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.styled.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.styled.tsx
index 1dbbb003add4a3c0e777d3fd91faa4cd5c409402..bf2f7ec51be8a65a2cc8d5c7d03e077a483d52f9 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.styled.tsx
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.styled.tsx
@@ -1,13 +1,13 @@
import styled from "@emotion/styled";
import { breakpointMinMedium } from "metabase/styled-components/theme/media-queries";
-import colors from "metabase/lib/colors";
+import { color } from "metabase/lib/colors";
interface ObjectDetailModalProps {
wide: boolean;
}
export const ObjectDetailModal = styled.div<ObjectDetailModalProps>`
- border: 1px solid ${colors.border};
+ border: 1px solid ${color("border")};
border-radius: 0.5rem;
overflow: hidden;
${breakpointMinMedium} {
@@ -28,6 +28,11 @@ export const ObjectDetailBodyWrapper = styled.div`
height: calc(100vh - 8rem);
`;
+export const ObjectIdLabel = styled.span`
+ color: ${color("text-medium")};
+ margin-left: 0.5rem;
+`;
+
export const ObjectDetailsTable = styled.div`
overflow-y: auto;
flex: 1;
@@ -41,7 +46,7 @@ export const ObjectRelationships = styled.div`
overflow-y: auto;
flex: 0 0 100%;
padding: 2rem;
- background-color: ${colors["bg-light"]};
+ background-color: ${color("bg-light")};
${breakpointMinMedium} {
flex: 0 0 33.3333%;
max-height: calc(80vh - 4rem);
@@ -52,7 +57,7 @@ export const CloseButton = styled.div`
display: flex;
margin-left: 1rem;
padding-left: 1rem;
- border-left: 1px solid ${colors.border};
+ border-left: 1px solid ${color("border")};
${breakpointMinMedium} {
display: none;
}
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx
index d946119d381602666f8e62d380a6f7bb27aa71e2..42b3ec17ef1cb2d7037827348419ae4f17d28b0a 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx
@@ -3,6 +3,7 @@ import { connect } from "react-redux";
import { t } from "ttag";
import Question from "metabase-lib/lib/Question";
+import { isPK } from "metabase/lib/schema_metadata";
import { Table } from "metabase-types/types/Table";
import { ForeignKey } from "metabase-types/api/foreignKey";
import { DatasetData } from "metabase-types/types/Dataset";
@@ -34,12 +35,18 @@ import {
} from "metabase/query_builder/selectors";
import { columnSettings } from "metabase/visualizations/lib/settings/column";
-import { getObjectName, getIdValue, getSingleResultsRow } from "./utils";
+import {
+ getObjectName,
+ getDisplayId,
+ getIdValue,
+ getSingleResultsRow,
+} from "./utils";
import { DetailsTable } from "./ObjectDetailsTable";
import { Relationships } from "./ObjectRelationships";
import {
ObjectDetailModal,
ObjectDetailBodyWrapper,
+ ObjectIdLabel,
CloseButton,
ErrorWrapper,
} from "./ObjectDetail.styled";
@@ -214,9 +221,20 @@ export function ObjectDetailFn({
return null;
}
- const objectName = getObjectName({ table, question });
+ const objectName = getObjectName({
+ table,
+ question,
+ cols: data.cols,
+ zoomedRow,
+ });
- const hasRelationships = tableForeignKeys && !!tableForeignKeys.length;
+ const displayId = getDisplayId({ cols: data.cols, zoomedRow });
+ const hasPk = !!data.cols.find(isPK);
+ const hasRelationships = !!(
+ tableForeignKeys &&
+ !!tableForeignKeys.length &&
+ hasPk
+ );
return (
<Modal
@@ -235,7 +253,7 @@ export function ObjectDetailFn({
<ObjectDetailHeader
canZoom={canZoom && (canZoomNextRow || canZoomPreviousRow)}
objectName={objectName}
- objectId={zoomedRowID}
+ objectId={displayId}
canZoomPreviousRow={canZoomPreviousRow}
canZoomNextRow={canZoomNextRow}
viewPreviousObjectDetail={viewPreviousObjectDetail}
@@ -245,8 +263,9 @@ export function ObjectDetailFn({
<ObjectDetailBody
data={data}
objectName={objectName}
- zoomedRow={zoomedRow}
+ zoomedRow={zoomedRow ?? []}
settings={settings}
+ hasRelationships={hasRelationships}
onVisualizationClick={onVisualizationClick}
visualizationIsClickable={visualizationIsClickable}
tableForeignKeys={tableForeignKeys}
@@ -262,7 +281,7 @@ export function ObjectDetailFn({
export interface ObjectDetailHeaderProps {
canZoom: boolean;
objectName: string;
- objectId: ObjectId;
+ objectId: ObjectId | null | unknown;
canZoomPreviousRow: boolean;
canZoomNextRow: boolean;
viewPreviousObjectDetail: () => void;
@@ -284,7 +303,8 @@ export function ObjectDetailHeader({
<div className="Grid border-bottom relative">
<div className="Grid-cell">
<h2 className="p3">
- {objectName} {objectId}
+ {objectName}
+ {objectId !== null && <ObjectIdLabel> {objectId}</ObjectIdLabel>}
</h2>
</div>
<div className="flex align-center">
@@ -331,8 +351,9 @@ export function ObjectDetailHeader({
export interface ObjectDetailBodyProps {
data: DatasetData;
objectName: string;
- zoomedRow: unknown[] | undefined;
+ zoomedRow: unknown[];
settings: unknown;
+ hasRelationships: boolean;
onVisualizationClick: OnVisualizationClickType;
visualizationIsClickable: (clicked: unknown) => boolean;
tableForeignKeys: ForeignKey[];
@@ -347,6 +368,7 @@ export function ObjectDetailBody({
objectName,
zoomedRow,
settings,
+ hasRelationships = false,
onVisualizationClick,
visualizationIsClickable,
tableForeignKeys,
@@ -362,12 +384,14 @@ export function ObjectDetailBody({
onVisualizationClick={onVisualizationClick}
visualizationIsClickable={visualizationIsClickable}
/>
- <Relationships
- objectName={objectName}
- tableForeignKeys={tableForeignKeys}
- tableForeignKeyReferences={tableForeignKeyReferences}
- foreignKeyClicked={followForeignKey}
- />
+ {hasRelationships && (
+ <Relationships
+ objectName={objectName}
+ tableForeignKeys={tableForeignKeys}
+ tableForeignKeyReferences={tableForeignKeyReferences}
+ foreignKeyClicked={followForeignKey}
+ />
+ )}
</ObjectDetailBodyWrapper>
);
}
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.unit.spec.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.unit.spec.tsx
index 6c34637f408b4b0105dd631bf4303c4657ce8116..c4c603b8d187a5035122db22789fe11f8b0bd640 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.unit.spec.tsx
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.unit.spec.tsx
@@ -60,6 +60,7 @@ describe("Object Detail", () => {
settings={{
column: () => null,
}}
+ hasRelationships={false}
onVisualizationClick={() => null}
visualizationIsClickable={() => false}
tableForeignKeys={[]}
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.tsx
index 6ea9b545465efaf1fca9ebe8c24ae30407ae2337..904ecfdc91a7fc9b63e54d4567303fc1cb072de4 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.tsx
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.tsx
@@ -5,6 +5,8 @@ import { t } from "ttag";
import { DatasetData } from "metabase-types/types/Dataset";
import ExpandableString from "metabase/query_builder/components/ExpandableString";
+import EmptyState from "metabase/components/EmptyState";
+
import { isID } from "metabase/lib/schema_metadata";
import { TYPE, isa } from "metabase/lib/types";
import { formatValue, formatColumn } from "metabase/lib/formatting";
@@ -100,7 +102,7 @@ export function DetailsTableCell({
export interface DetailsTableProps {
data: DatasetData;
- zoomedRow: unknown[] | undefined;
+ zoomedRow: unknown[];
settings: unknown;
onVisualizationClick: OnVisualizationClickType;
visualizationIsClickable: (clicked: any) => boolean;
@@ -113,8 +115,12 @@ export function DetailsTable({
onVisualizationClick,
visualizationIsClickable,
}: DetailsTableProps): JSX.Element {
- const { rows, cols } = data;
- const row = zoomedRow || rows[0];
+ const { cols } = data;
+ const row = zoomedRow;
+
+ if (!row?.length) {
+ return <EmptyState message={t`No details found`} />;
+ }
return (
<ObjectDetailsTable>
@@ -124,7 +130,7 @@ export function DetailsTable({
<GridCell>
<DetailsTableCell
column={column}
- value={row[columnIndex]}
+ value={row[columnIndex] ?? t`Empty`}
isColumnName
settings={settings}
className="text-bold text-medium"
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.unit.spec.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.unit.spec.tsx
index 425336aa03ec890e7aba777dd24300036b0dd8ba..5108b2452c05703b80bb1884d0dfdcc79591130e 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.unit.spec.tsx
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetailsTable.unit.spec.tsx
@@ -63,7 +63,7 @@ describe("ObjectDetailsTable", () => {
render(
<DetailsTable
data={objectDetailCard.data}
- zoomedRow={undefined}
+ zoomedRow={objectDetailCard.data.rows[0]}
onVisualizationClick={() => null}
visualizationIsClickable={() => false}
settings={{}}
@@ -78,7 +78,7 @@ describe("ObjectDetailsTable", () => {
render(
<DetailsTable
data={invalidObjectDetailCard.data}
- zoomedRow={undefined}
+ zoomedRow={invalidObjectDetailCard.data.rows[0]}
onVisualizationClick={() => null}
visualizationIsClickable={() => false}
settings={{}}
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx
index 8bb579963645b28bb6cd5237c3e896ed62fba78c..1a6c20c4db71e9f554884a2842d8fd0a3216498f 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectRelationships.tsx
@@ -41,7 +41,7 @@ export function Relationships({
return (
<ObjectRelationships>
<div className="text-bold text-medium">
- {jt`This ${(
+ {jt`${(
<span className="text-dark" key={objectName}>
{objectName}
</span>
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/utils.ts b/frontend/src/metabase/visualizations/components/ObjectDetail/utils.ts
index 4a4d883f8718686f22b2ffb7bed2383f5f53795a..2e49368725cfd80685a889c9049addf3baf72420 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/utils.ts
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/utils.ts
@@ -2,22 +2,32 @@ import { t } from "ttag";
import _ from "underscore";
import { singularize } from "metabase/lib/formatting";
-import { isPK } from "metabase/lib/schema_metadata";
+import { isPK, isEntityName } from "metabase/lib/schema_metadata";
import { Table } from "metabase-types/types/Table";
import Question from "metabase-lib/lib/Question";
-import { DatasetData } from "metabase-types/types/Dataset";
+import { DatasetData, Column } from "metabase-types/types/Dataset";
import { ObjectId } from "./types";
export interface GetObjectNameArgs {
table: Table | null;
question: Question;
+ cols: Column[];
+ zoomedRow: unknown[] | undefined;
}
export const getObjectName = ({
table,
question,
+ cols,
+ zoomedRow,
}: GetObjectNameArgs): string => {
+ const entityNameColumn = cols && cols?.findIndex(isEntityName);
+
+ if (zoomedRow?.length && zoomedRow[entityNameColumn]) {
+ return zoomedRow[entityNameColumn] as string;
+ }
+
const tableObjectName = table && table.objectName();
if (tableObjectName) {
return tableObjectName;
@@ -26,7 +36,41 @@ export const getObjectName = ({
if (questionName) {
return singularize(questionName);
}
- return t`Unknown`;
+ return t`Item Detail`;
+};
+
+export interface GetDisplayIdArgs {
+ cols: Column[];
+ zoomedRow: unknown[] | undefined;
+}
+
+export const getDisplayId = ({
+ cols,
+ zoomedRow,
+}: GetDisplayIdArgs): ObjectId | null => {
+ const hasSinglePk =
+ cols.reduce(
+ (pks: number, col: Column) => (isPK(col) ? pks + 1 : pks),
+ 0,
+ ) === 1;
+
+ if (!zoomedRow) {
+ return null;
+ }
+
+ if (hasSinglePk) {
+ const pkColumn = cols.findIndex(isPK);
+ return zoomedRow[pkColumn] as ObjectId;
+ }
+
+ const hasEntityName = cols && !!cols?.find(isEntityName);
+
+ if (hasEntityName) {
+ return null;
+ }
+
+ // TODO: respect user column reordering
+ return zoomedRow[0] as ObjectId;
};
export interface GetIdValueArgs {
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/utils.unit.spec.ts b/frontend/src/metabase/visualizations/components/ObjectDetail/utils.unit.spec.ts
new file mode 100644
index 0000000000000000000000000000000000000000..fe4626f6db7dc9ea5ff12a6af998a7298f47cfad
--- /dev/null
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/utils.unit.spec.ts
@@ -0,0 +1,165 @@
+import {
+ metadata,
+ SAMPLE_DATABASE,
+ ORDERS,
+} from "__support__/sample_database_fixture";
+
+import Question from "metabase-lib/lib/Question";
+import { Card } from "metabase-types/types/Card";
+
+import { getObjectName, getDisplayId, getIdValue } from "./utils";
+
+const card = {
+ id: 1,
+ name: "Special Orders",
+ display: "table",
+ visualization_settings: {},
+ can_write: true,
+ dataset_query: {
+ type: "query",
+ database: SAMPLE_DATABASE?.id,
+ query: {
+ "source-table": ORDERS.id,
+ },
+ },
+} as Card;
+
+describe("ObjectDetail utils", () => {
+ const idCol = {
+ name: "id",
+ display_name: "ID",
+ base_type: "int",
+ effective_type: "int",
+ semantic_type: "type/PK",
+ };
+ const qtyCol = {
+ name: "qty",
+ display_name: "qty",
+ base_type: "int",
+ effective_type: "int",
+ semantic_type: "type/int",
+ };
+ const nameCol = {
+ name: "id",
+ display_name: "ID",
+ base_type: "string",
+ effective_type: "string",
+ semantic_type: "type/Name",
+ };
+
+ describe("getObjectName", () => {
+ const question = new Question(card, metadata);
+ const table = question.table();
+
+ it("should get an entity name when there is an entity name column", () => {
+ const name = getObjectName({
+ table: null,
+ question: question,
+ cols: [idCol, qtyCol, nameCol],
+ zoomedRow: [22, 33, "Giant Sprocket"],
+ });
+
+ expect(name).toBe("Giant Sprocket");
+ });
+
+ it("should get a singularized table name if no entity name is present", () => {
+ const name = getObjectName({
+ table: table as any,
+ question: question,
+ cols: [idCol, qtyCol],
+ zoomedRow: [22, 33],
+ });
+
+ expect(name).toBe("Order");
+ });
+
+ it("should get a singularized question name if neither table nor entity names are present", () => {
+ const name = getObjectName({
+ table: null,
+ question: question,
+ cols: [idCol, qtyCol],
+ zoomedRow: [22, 33],
+ });
+
+ expect(name).toBe("Special Order");
+ });
+
+ it("should fall back to default text", () => {
+ const name = getObjectName({
+ table: null,
+ question: new Question(
+ {
+ ...card,
+ name: "",
+ },
+ metadata,
+ ),
+ cols: [idCol, qtyCol],
+ zoomedRow: [22, 33],
+ });
+
+ expect(name).toBe("Item Detail");
+ });
+ });
+
+ describe("getDisplayId", () => {
+ it("should get a display id when there is a single primary key column", () => {
+ const id = getDisplayId({
+ cols: [idCol, qtyCol, nameCol],
+ zoomedRow: [22, 33, "Giant Sprocket"],
+ });
+
+ expect(id).toBe(22);
+ });
+
+ it("should return null if there is no primary key and an entity name", () => {
+ const id = getDisplayId({
+ cols: [qtyCol, nameCol],
+ zoomedRow: [33, "Giant Sprocket"],
+ });
+
+ expect(id).toBe(null);
+ });
+
+ it("should return the first row's value if there is neither an entity name nor a primary key", () => {
+ const id = getDisplayId({
+ cols: [qtyCol, qtyCol],
+ zoomedRow: [33, 44],
+ });
+
+ expect(id).toBe(33);
+ });
+ });
+
+ describe("getIdValue", () => {
+ it("should return the zoomed Row id if present", () => {
+ const id = getIdValue({
+ data: {
+ cols: [idCol, qtyCol, nameCol],
+ rows: [
+ [22, 33, "Giant Sprocket"],
+ [44, 55, "Tiny Sprocket"],
+ ],
+ },
+ zoomedRowID: 1,
+ });
+
+ expect(id).toBe(1);
+ });
+
+ // this code should no longer be reachable now that we always reach object detail by zooming
+ it("should return the primary key of the first row if now zoomed row id exists", () => {
+ const id = getIdValue({
+ data: {
+ cols: [idCol, qtyCol, nameCol],
+ rows: [
+ [22, 33, "Giant Sprocket"],
+ [44, 55, "Tiny Sprocket"],
+ ],
+ },
+ });
+
+ expect(id).toBe(22);
+ });
+ });
+});
diff --git a/frontend/src/metabase/visualizations/components/TableInteractive.jsx b/frontend/src/metabase/visualizations/components/TableInteractive.jsx
index 91790ec1dfaceaec557dc95e46a170f188e43624..2260a9d1bb04402138b1024e81eba2fdf47be0bf 100644
--- a/frontend/src/metabase/visualizations/components/TableInteractive.jsx
+++ b/frontend/src/metabase/visualizations/components/TableInteractive.jsx
@@ -3,10 +3,15 @@ import React, { Component } from "react";
import PropTypes from "prop-types";
import ReactDOM from "react-dom";
import { t } from "ttag";
+import { connect } from "react-redux";
+import _ from "underscore";
+import cx from "classnames";
+import Draggable from "react-draggable";
+import { Grid, ScrollSync } from "react-virtualized";
+
import "./TableInteractive.css";
import Icon from "metabase/components/Icon";
-
import ExternalLink from "metabase/core/components/ExternalLink";
import Button from "metabase/core/components/Button";
import Tooltip from "metabase/components/Tooltip";
@@ -25,15 +30,11 @@ import { fieldRefForColumn } from "metabase/lib/dataset";
import { isAdHocModelQuestionCard } from "metabase/lib/data-modeling/utils";
import Dimension from "metabase-lib/lib/Dimension";
import { getScrollBarSize } from "metabase/lib/dom";
-
-import _ from "underscore";
-import cx from "classnames";
+import { zoomInRow } from "metabase/query_builder/actions";
import ExplicitSize from "metabase/components/ExplicitSize";
import MiniBar from "./MiniBar";
-import { Grid, ScrollSync } from "react-virtualized";
-import Draggable from "react-draggable";
import Ellipsified from "metabase/components/Ellipsified";
import DimensionInfoPopover from "metabase/components/MetadataInfo/DimensionInfoPopover";
@@ -64,6 +65,10 @@ function pickRowsToMeasure(rows, columnIndex, count = 10) {
return rowIndexes;
}
+const mapDispatchToProps = dispatch => ({
+ onZoomRow: objectId => dispatch(zoomInRow({ objectId })),
+});
+
class TableInteractive extends Component {
constructor(props) {
super(props);
@@ -71,6 +76,7 @@ class TableInteractive extends Component {
this.state = {
columnWidths: [],
contentWidths: null,
+ showDetailShortcut: true,
};
this.columnHasResized = {};
this.headerRefs = [];
@@ -110,6 +116,7 @@ class TableInteractive extends Component {
this._measure();
this._findIDColumn(this.props.data, this.props.isPivoted);
+ this._showDetailShortcut(this.props.query, this.props.isPivoted);
}
componentWillUnmount() {
@@ -142,6 +149,7 @@ class TableInteractive extends Component {
if (isDataChange) {
this._findIDColumn(nextData, newProps.isPivoted);
+ this._showDetailShortcut(this.props.query, this.props.isPivoted);
}
}
@@ -155,9 +163,14 @@ class TableInteractive extends Component {
IDColumnIndex: pkIndex === -1 ? null : pkIndex,
IDColumn: pkIndex === -1 ? null : data.cols[pkIndex],
});
- if (pkIndex !== -1) {
- document.addEventListener("keydown", this.onKeyDown);
- }
+ document.addEventListener("keydown", this.onKeyDown);
+ };
+
+ _showDetailShortcut = (query, isPivoted) => {
+ const hasAggregation = !!query?.aggregations?.()?.length;
+ this.setState({
+ showDetailShortcut: !(isPivoted || hasAggregation),
+ });
};
_getColumnSettings(props) {
@@ -432,18 +445,18 @@ class TableInteractive extends Component {
}
}
- pkClick(rowIndex) {
- const columnIndex = this.state.IDColumnIndex;
- const clicked = this.getCellClickedObject(rowIndex, columnIndex);
-
- return e => this.onVisualizationClick(clicked, e.currentTarget);
- }
+ pkClick = rowIndex => {
+ const objectId = this.state.IDColumn
+ ? this.props.data.rows[rowIndex][this.state.IDColumnIndex]
+ : rowIndex;
+ return e => this.props.onZoomRow(objectId);
+ };
onKeyDown = event => {
const detailEl = this.detailShortcutRef.current;
const visibleDetailButton =
!!detailEl && Array.from(detailEl.classList).includes("show") && detailEl;
- const canViewRowDetail = !!this.state.IDColumn && !!visibleDetailButton;
+ const canViewRowDetail = !this.props.isPivoted && !!visibleDetailButton;
if (event.key === "Enter" && canViewRowDetail) {
const hoveredRowIndex = Number(detailEl.dataset.showDetailRowindex);
@@ -453,7 +466,7 @@ class TableInteractive extends Component {
cellRenderer = ({ key, style, rowIndex, columnIndex }) => {
const { data, settings } = this.props;
- const { dragColIndex } = this.state;
+ const { dragColIndex, showDetailShortcut } = this.state;
const { rows, cols } = data;
const column = cols[columnIndex];
@@ -497,7 +510,7 @@ class TableInteractive extends Component {
}}
className={cx("TableInteractive-cellWrapper text-dark", {
"TableInteractive-cellWrapper--firstColumn": columnIndex === 0,
- padLeft: columnIndex === 0 && !this.state.IDColumn,
+ padLeft: columnIndex === 0 && !showDetailShortcut,
"TableInteractive-cellWrapper--lastColumn":
columnIndex === cols.length - 1,
"TableInteractive-emptyCell": value == null,
@@ -523,12 +536,10 @@ class TableInteractive extends Component {
: undefined
}
onMouseEnter={
- this.state.IDColumn
- ? e => this.handleHoverRow(e, rowIndex)
- : undefined
+ showDetailShortcut ? e => this.handleHoverRow(e, rowIndex) : undefined
}
onMouseLeave={
- this.state.IDColumn ? e => this.handleLeaveRow() : undefined
+ showDetailShortcut ? e => this.handleLeaveRow() : undefined
}
tabIndex="0"
>
@@ -558,7 +569,7 @@ class TableInteractive extends Component {
}
getColumnPositions = () => {
- let left = this.state.IDColumn ? SIDEBAR_WIDTH : 0;
+ let left = this.state.showDetailShortcut ? SIDEBAR_WIDTH : 0;
return this.props.data.cols.map((col, index) => {
const width = this.getColumnWidth({ index });
const pos = {
@@ -577,7 +588,7 @@ class TableInteractive extends Component {
const { cols } = this.props.data;
const indexes = cols.map((col, index) => index);
indexes.splice(dragColNewIndex, 0, indexes.splice(dragColIndex, 1)[0]);
- let left = this.state.IDColumn ? SIDEBAR_WIDTH : 0;
+ let left = this.state.showDetailShortcut ? SIDEBAR_WIDTH : 0;
const lefts = indexes.map(index => {
const thisLeft = left;
left += columnPositions[index].width;
@@ -626,7 +637,7 @@ class TableInteractive extends Component {
getColumnTitle,
renderTableHeaderWrapper,
} = this.props;
- const { dragColIndex } = this.state;
+ const { dragColIndex, showDetailShortcut } = this.state;
const { cols } = data;
const column = cols[columnIndex];
@@ -711,7 +722,7 @@ class TableInteractive extends Component {
"TableInteractive-cellWrapper TableInteractive-headerCellData text-medium text-brand-hover",
{
"TableInteractive-cellWrapper--firstColumn": columnIndex === 0,
- padLeft: columnIndex === 0 && !this.state.IDColumn,
+ padLeft: columnIndex === 0 && !showDetailShortcut,
"TableInteractive-cellWrapper--lastColumn":
columnIndex === cols.length - 1,
"TableInteractive-cellWrapper--active": isDragging,
@@ -798,13 +809,15 @@ class TableInteractive extends Component {
};
getDisplayColumnWidth = ({ index: displayIndex }) => {
- if (this.state.IDColumn && displayIndex === 0) {
+ if (this.state.showDetailShortcut && displayIndex === 0) {
return SIDEBAR_WIDTH;
}
- // if we have an ID column, we've added a column of empty cells and need to shift
+ // if the detail shortcut is visible, we've added a column of empty cells and need to shift
// the display index to get the data index
- const dataIndex = this.state.IDColumn ? displayIndex - 1 : displayIndex;
+ const dataIndex = this.state.showDetailShortcut
+ ? displayIndex - 1
+ : displayIndex;
return this.getColumnWidth({ index: dataIndex });
};
@@ -880,7 +893,7 @@ class TableInteractive extends Component {
}
const headerHeight = this.props.tableHeaderHeight || HEADER_HEIGHT;
- const gutterColumn = this.state.IDColumn ? 1 : 0;
+ const gutterColumn = this.state.showDetailShortcut ? 1 : 0;
return (
<ScrollSync>
@@ -1043,6 +1056,7 @@ export default _.compose(
ExplicitSize({
refreshMode: props => (props.isDashboard ? "debounce" : "throttle"),
}),
+ connect(null, mapDispatchToProps),
memoizeClass(
"_getCellClickedObjectCached",
"_getHeaderClickedObjectCached",
diff --git a/frontend/test/metabase/modes/components/drill/ObjectDetailDrill.unit.spec.js b/frontend/test/metabase/modes/components/drill/ObjectDetailDrill.unit.spec.js
index adcd2867c8aed69e05157b31814fc7d5193a66c5..034ebeab0267d51b8a4851cf82246ec88a4d4bb5 100644
--- a/frontend/test/metabase/modes/components/drill/ObjectDetailDrill.unit.spec.js
+++ b/frontend/test/metabase/modes/components/drill/ObjectDetailDrill.unit.spec.js
@@ -90,6 +90,7 @@ describe("ObjectDetailDrill", () => {
describe("PK cells", () => {
describe("general", () => {
const mockDispatch = jest.fn();
+ const mockGetState = () => ({ qb: { queryResults: {}, card: {} } });
const { actions, cellValue } = setup({
column: ORDERS.ID.column(),
});
@@ -102,7 +103,7 @@ describe("ObjectDetailDrill", () => {
it("should return correct redux action", () => {
const [action] = actions;
- action.action()(mockDispatch);
+ action.action()(mockDispatch, mockGetState);
expect(mockDispatch).toHaveBeenCalledWith({
type: ZOOM_IN_ROW,
payload: {
diff --git a/frontend/test/metabase/scenarios/dashboard/dashboard-drill.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/dashboard-drill.cy.spec.js
index 0bb494001d2b11073ecf1f4e4518ac83be777f96..389d512e8a48d3564a4347fce8521c19e2ec4779 100644
--- a/frontend/test/metabase/scenarios/dashboard/dashboard-drill.cy.spec.js
+++ b/frontend/test/metabase/scenarios/dashboard/dashboard-drill.cy.spec.js
@@ -403,7 +403,7 @@ describe("scenarios > dashboard > dashboard drill", () => {
cy.wait("@dataset");
cy.findByTestId("object-detail").within(() => {
- cy.findByText(PK_VALUE);
+ cy.findAllByText(PK_VALUE);
});
const pattern = new RegExp(`/question\\?objectId=${PK_VALUE}#*`);
diff --git a/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js b/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js
index c4113d40b4ff12468b5670bbc7fd8cd67130c937..4f1579b98118ec74a44e7b8c813fd370f121cfe9 100644
--- a/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js
+++ b/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js
@@ -218,8 +218,8 @@ describe("scenarios > models metadata", () => {
drillFK({ id: 1 });
cy.wait("@dataset");
cy.findByTestId("object-detail").within(() => {
- cy.findByText("1");
- cy.findByText("Hudson Borer");
+ cy.findByText("68883"); // zip
+ cy.findAllByText("Hudson Borer");
});
cy.go("back"); // close modal
@@ -232,8 +232,8 @@ describe("scenarios > models metadata", () => {
drillFK({ id: 7 });
cy.wait("@dataset");
cy.findByTestId("object-detail").within(() => {
- cy.findByText("7");
- cy.findByText("perry.ruecker");
+ cy.findAllByText("7");
+ cy.findAllByText("perry.ruecker");
});
});
});
@@ -255,8 +255,8 @@ describe("scenarios > models metadata", () => {
drillDashboardFK({ id: 1 });
cy.wait("@dataset");
cy.findByTestId("object-detail").within(() => {
- cy.findByText("1");
- cy.findByText("Hudson Borer");
+ cy.findAllByText("1");
+ cy.findAllByText("Hudson Borer");
});
cy.go("back");
@@ -267,8 +267,8 @@ describe("scenarios > models metadata", () => {
drillDashboardFK({ id: 7 });
cy.wait("@dataset");
cy.findByTestId("object-detail").within(() => {
- cy.findByText("7");
- cy.findByText("perry.ruecker");
+ cy.findAllByText("7");
+ cy.findAllByText("perry.ruecker");
});
});
});
diff --git a/frontend/test/metabase/scenarios/question/reproductions/16938-nested-native-query-pk-drill.cy.spec.js b/frontend/test/metabase/scenarios/question/reproductions/16938-nested-native-query-pk-drill.cy.spec.js
index 0d7e0debd8eff0f0a0bf0e9609452ad8fa5d5edf..db97d880d454742d844fd29ee916c2b95a915594 100644
--- a/frontend/test/metabase/scenarios/question/reproductions/16938-nested-native-query-pk-drill.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/reproductions/16938-nested-native-query-pk-drill.cy.spec.js
@@ -29,7 +29,7 @@ describe("issue 16938", () => {
.click();
cy.findByTestId("object-detail").within(() => {
- cy.findByText(`Order ${ORDER_ID}`);
+ cy.findByText("37.65");
});
});
});
diff --git a/frontend/test/metabase/scenarios/visualizations/object_detail.cy.spec.js b/frontend/test/metabase/scenarios/visualizations/object_detail.cy.spec.js
index fea720d7c51bcf9803e1eef3233b981bbd6e5f19..51af1fff44d0143cef4db8401dcf3901a0d2360f 100644
--- a/frontend/test/metabase/scenarios/visualizations/object_detail.cy.spec.js
+++ b/frontend/test/metabase/scenarios/visualizations/object_detail.cy.spec.js
@@ -58,7 +58,7 @@ describe("scenarios > question > object details", () => {
drillFK({ id: 1 });
- assertUserDetailView({ id: 1 });
+ assertUserDetailView({ id: 1, name: "Hudson Borer" });
getPreviousObjectDetailButton().should("not.exist");
getNextObjectDetailButton().should("not.exist");
@@ -69,7 +69,7 @@ describe("scenarios > question > object details", () => {
changeSorting("User ID", "desc");
drillFK({ id: 2500 });
- assertDetailView({ id: 2500, entityName: "Person", byFK: true });
+ assertUserDetailView({ id: 2500, name: "Kenny Schmidt" });
getPreviousObjectDetailButton().should("not.exist");
getNextObjectDetailButton().should("not.exist");
});
@@ -125,7 +125,8 @@ describe("scenarios > question > object details", () => {
cy.url().should("contain", "objectId=2");
cy.findByTestId("object-detail")
- .findByText("Domenica Williamson")
+ .findAllByText("Domenica Williamson")
+ .last()
.click();
// Popover is blocking the city. If it renders, Cypress will not be able to click on "Searsboro" and the test will fail.
// Unfortunately, asserting that the popover does not exist will give us a false positive result.
@@ -168,8 +169,8 @@ function assertOrderDetailView({ id }) {
assertDetailView({ id, entityName: "Order" });
}
-function assertUserDetailView({ id }) {
- assertDetailView({ id, entityName: "Person", byFK: true });
+function assertUserDetailView({ id, name }) {
+ assertDetailView({ id, entityName: name, byFK: true });
}
function getPreviousObjectDetailButton() {