diff --git a/frontend/src/metabase/dashboard/actions.js b/frontend/src/metabase/dashboard/actions.js
index d440abf492877a3dcb1141c04e599078ba892927..c2ab2b00892911b575ffdaac66d1795b89c3468d 100644
--- a/frontend/src/metabase/dashboard/actions.js
+++ b/frontend/src/metabase/dashboard/actions.js
@@ -29,7 +29,6 @@ import {
getParameterValuesBySlug,
getParameterValuesByIdFromQueryParams,
} from "metabase/parameters/utils/parameter-values";
-import * as Urls from "metabase/lib/urls";
import { SIDEBAR_NAME } from "metabase/dashboard/constants";
import Utils from "metabase/lib/utils";
@@ -1070,15 +1069,14 @@ export const navigateToNewCardFromDashboard = createThunkAction(
const isDrillingFromNativeModel =
previousQuestion.isDataset() && previousQuestion.isNative();
- // when the query is for a specific object (ie `=` filter on PK column)
- // it does not make sense to apply parameter filters
- // because we'll be navigating to the details view of a specific row on a table
- const url = question.isObjectDetail()
- ? Urls.serializedQuestion(question.card())
- : question.getUrlWithParameters(parametersMappedToCard, parameterValues, {
- clean: !isDrillingFromNativeModel,
- objectId,
- });
+ const url = question.getUrlWithParameters(
+ parametersMappedToCard,
+ parameterValues,
+ {
+ clean: !isDrillingFromNativeModel,
+ objectId,
+ },
+ );
dispatch(openUrl(url));
},
diff --git a/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx b/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx
index a0a348e644354a7e1dedb726ed2a61fbf811b4f3..9f1a194e18db6a6ad820780b4809069dcb49a7bb 100644
--- a/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx
+++ b/frontend/src/metabase/modes/components/drill/ObjectDetailDrill.jsx
@@ -1,6 +1,5 @@
import { t } from "ttag";
import { isFK, isPK } from "metabase/lib/schema_metadata";
-import * as Urls from "metabase/lib/urls";
import { zoomInRow } from "metabase/query_builder/actions";
function hasManyPKColumns(question) {
@@ -11,26 +10,14 @@ function hasManyPKColumns(question) {
function getActionForPKColumn({ question, column, objectId, extraData }) {
if (hasManyPKColumns(question)) {
- // Filter by a clicked value, then a user can click on the 2nd, 3d, ..., Nth PK cells
- // to narrow down filtering and eventually enter the object detail view once all PKs are filtered
return ["question", () => question.filter("=", column, objectId)];
}
const isDashboard = !!extraData?.dashboard;
- if (isDashboard) {
- const { parameterValuesBySlug = {} } = extraData;
- const hasParameters = Object.keys(parameterValuesBySlug).length > 0;
-
- // This should result in a metabase/dashboard/actions navigateToNewCardFromDashboard call
- // That will convert dashboard parameters into question filters
- // and make sure the clicked row will be present in the query results
- if (hasParameters) {
- const getNextQuestion = () => question;
- const getExtraData = () => ({ objectId });
- return ["question", getNextQuestion, getExtraData];
- }
- return ["url", () => Urls.question(question.card(), { objectId })];
+ // the question from the dashboard may have filters applied already
+ if (isDashboard) {
+ return ["question", () => question];
}
return ["action", () => zoomInRow({ objectId })];
@@ -49,16 +36,13 @@ function getBaseActionObject() {
function getPKAction({ question, column, objectId, extraData }) {
const actionObject = getBaseActionObject();
- const [actionKey, action, extra] = getActionForPKColumn({
+ const [actionKey, action] = getActionForPKColumn({
question,
column,
objectId,
extraData,
});
actionObject[actionKey] = action;
- if (extra) {
- actionObject.extra = extra;
- }
return actionObject;
}
@@ -96,5 +80,8 @@ export default ({ question, clicked }) => {
const { column, value: objectId, extraData } = clicked;
const params = { question, column, objectId, extraData };
const actionObject = isPK(column) ? getPKAction(params) : getFKAction(params);
+ if (!hasManyPKColumns(question)) {
+ actionObject.extra = () => ({ objectId });
+ }
return actionObject ? [actionObject] : [];
};
diff --git a/frontend/src/metabase/modes/lib/modes.js b/frontend/src/metabase/modes/lib/modes.js
index 1974fa35cd65b7b28ab7bfc21a17a3185f182346..73cb7f321cbc8e1e91d7536397890372fdd3d78c 100644
--- a/frontend/src/metabase/modes/lib/modes.js
+++ b/frontend/src/metabase/modes/lib/modes.js
@@ -1,4 +1,3 @@
-import ObjectMode from "../components/modes/ObjectMode";
import SegmentMode from "../components/modes/SegmentMode";
import MetricMode from "../components/modes/MetricMode";
import TimeseriesMode from "../components/modes/TimeseriesMode";
@@ -10,23 +9,6 @@ import DefaultMode from "../components/modes/DefaultMode";
import StructuredQuery from "metabase-lib/lib/queries/StructuredQuery";
import NativeQuery from "metabase-lib/lib/queries/NativeQuery";
-const isPKFilter = (filters, query) => {
- const sourceTablePKFields =
- query?.table()?.fields.filter(field => field.isPK()) || [];
-
- if (sourceTablePKFields.length === 0) {
- return false;
- }
-
- const hasEqualityFilterForEveryPK = sourceTablePKFields.every(pkField => {
- const filter = filters.find(filter => filter.field()?.id === pkField.id);
-
- return filter?.operatorName() === "=" && filter?.arguments().length === 1;
- });
-
- return hasEqualityFilterForEveryPK;
-};
-
export function getMode(question) {
if (!question) {
return null;
@@ -41,14 +23,9 @@ export function getMode(question) {
if (query instanceof StructuredQuery) {
const aggregations = query.aggregations();
const breakouts = query.breakouts();
- const filters = query.filters();
if (aggregations.length === 0 && breakouts.length === 0) {
- if (isPKFilter(filters, query)) {
- return ObjectMode;
- } else {
- return SegmentMode;
- }
+ return SegmentMode;
}
if (aggregations.length > 0 && breakouts.length === 0) {
return MetricMode;
diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js
index 3b054fd0c0422d76ba051812e2c7675899cb75ee..952b309186b53dfd2643c890598f76811c4c2e9a 100644
--- a/frontend/src/metabase/query_builder/actions.js
+++ b/frontend/src/metabase/query_builder/actions.js
@@ -1019,7 +1019,7 @@ export const setCardAndRun = (nextCard, shouldUpdateUrl = true) => {
export const NAVIGATE_TO_NEW_CARD = "metabase/qb/NAVIGATE_TO_NEW_CARD";
export const navigateToNewCardInsideQB = createThunkAction(
NAVIGATE_TO_NEW_CARD,
- ({ nextCard, previousCard }) => {
+ ({ nextCard, previousCard, objectId }) => {
return async (dispatch, getState) => {
if (previousCard === nextCard) {
// Do not reload questions with breakouts when clicked on a legend item
@@ -1041,6 +1041,9 @@ export const navigateToNewCardInsideQB = createThunkAction(
// to start building a new ad-hoc question based on a dataset
dispatch(setCardAndRun({ ...card, dataset: false }));
}
+ if (objectId !== undefined) {
+ dispatch(zoomInRow({ objectId }));
+ }
}
};
},
diff --git a/frontend/src/metabase/query_builder/components/view/QuestionDataSource.unit.spec.js b/frontend/src/metabase/query_builder/components/view/QuestionDataSource.unit.spec.js
index d70020c4263e140dd02b64e5f4adbf1dd95e580d..c77d961ef46ffdfa7de73f3fdbdc88bf37d8c099 100644
--- a/frontend/src/metabase/query_builder/components/view/QuestionDataSource.unit.spec.js
+++ b/frontend/src/metabase/query_builder/components/view/QuestionDataSource.unit.spec.js
@@ -503,7 +503,10 @@ describe("QuestionDataSource", () => {
});
});
- describe("Object Detail", () => {
+ describe.skip("Object Detail", () => {
+ // these tests do not apply to the new modal object detail view
+ // but will be useful when we implement the new version of full page
+ // object detail
[
GUI_TEST_CASE.SAVED_OBJECT_DETAIL,
GUI_TEST_CASE.AD_HOC_OBJECT_DETAIL,
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index 924d9365f12041d906f58e1b9d59afe4479a517b..444b9ef68d565a358d2574c618d196b7c943310a 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -479,15 +479,8 @@ export const getMode = createSelector(
);
export const getIsObjectDetail = createSelector(
- [getMode, getQueryResults, isZoomingRow],
- (mode, results, isZoomingSingleRow) => {
- if (isZoomingSingleRow) {
- return true;
- }
- // It handles filtering by a manually set PK column that is not unique
- const hasMultipleRows = results?.some(({ data }) => data?.rows.length > 1);
- return mode?.name() === "object" && !hasMultipleRows;
- },
+ [getMode, isZoomingRow],
+ (mode, isZoomingSingleRow) => isZoomingSingleRow || mode?.name() === "object",
);
export const getIsDirty = createSelector(
diff --git a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx
index 903e4d8dde209e441459967a21b58f63048d78ed..d946119d381602666f8e62d380a6f7bb27aa71e2 100644
--- a/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx
+++ b/frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx
@@ -233,7 +233,7 @@ export function ObjectDetailFn({
) : (
<div className="ObjectDetail" data-testid="object-detail">
<ObjectDetailHeader
- canZoom={canZoom}
+ canZoom={canZoom && (canZoomNextRow || canZoomPreviousRow)}
objectName={objectName}
objectId={zoomedRowID}
canZoomPreviousRow={canZoomPreviousRow}
@@ -317,7 +317,6 @@ export function ObjectDetailHeader({
data-testId="object-detail-close-button"
onlyIcon
borderless
- disabled={!canZoomNextRow}
onClick={closeObjectDetail}
icon="close"
iconSize={20}
diff --git a/frontend/src/metabase/visualizations/components/TableInteractive.jsx b/frontend/src/metabase/visualizations/components/TableInteractive.jsx
index f4dc5a11bb6e62b1402483cf6ac1fc50a4eb11ae..898607c3bbf1b9be979ded2cda846c46ecd45e84 100644
--- a/frontend/src/metabase/visualizations/components/TableInteractive.jsx
+++ b/frontend/src/metabase/visualizations/components/TableInteractive.jsx
@@ -149,9 +149,10 @@ export default class TableInteractive extends Component {
}
_findIDColumn = (data, isPivoted = false) => {
- const pkIndex = isPivoted
- ? -1
- : data.cols.findIndex(col => isPK(col) && col.id !== undefined);
+ const hasManyPKColumns = data.cols.filter(isPK).length > 1;
+
+ const pkIndex =
+ isPivoted || hasManyPKColumns ? -1 : data.cols.findIndex(isPK);
this.setState({
IDColumnIndex: pkIndex === -1 ? null : pkIndex,
diff --git a/frontend/src/metabase/visualizations/lib/action.js b/frontend/src/metabase/visualizations/lib/action.js
index 6c2b71f115b3eae6e8e573fa1d01526fe48ef9a7..5cc834e24ac58c9ea9464b45dc5a758f211daf06 100644
--- a/frontend/src/metabase/visualizations/lib/action.js
+++ b/frontend/src/metabase/visualizations/lib/action.js
@@ -23,7 +23,11 @@ export function performAction(action, { dispatch, onChangeCardAndRun }) {
const question = action.question();
const extra = action?.extra?.() ?? {};
if (question) {
- onChangeCardAndRun({ nextCard: question.card(), ...extra });
+ onChangeCardAndRun({
+ nextCard: question.card(),
+ ...extra,
+ objectId: extra.objectId,
+ });
didPerform = true;
}
}
diff --git a/frontend/test/metabase-lib/lib/Mode.unit.spec.js b/frontend/test/metabase-lib/lib/Mode.unit.spec.js
index 5691f8200c15e283f1fcc5a599ac501cfda3e1e9..e25dcd9bd73db4b217d2b6532f1e028521be27a0 100644
--- a/frontend/test/metabase-lib/lib/Mode.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Mode.unit.spec.js
@@ -82,12 +82,12 @@ describe("Mode", () => {
expect(mode && mode.name()).toEqual("pivot");
});
- it("returns `object` mode with pk filter", () => {
+ it("returns `segment` mode with pk filter", () => {
const mode = rawDataQuery
.filter(["=", ["field", ORDERS.ID.id, null], 42])
.question()
.mode();
- expect(mode && mode.name()).toEqual("object");
+ expect(mode && mode.name()).toEqual("segment");
});
it("returns `default` mode with >=0 aggregations and >=3 breakouts", () => {
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 bdcc96d8305a3c3fabbbc968ac420165fbfd0fef..adcd2867c8aed69e05157b31814fc7d5193a66c5 100644
--- a/frontend/test/metabase/modes/components/drill/ObjectDetailDrill.unit.spec.js
+++ b/frontend/test/metabase/modes/components/drill/ObjectDetailDrill.unit.spec.js
@@ -179,7 +179,7 @@ describe("ObjectDetailDrill", () => {
describe("from dashboard", () => {
describe("without parameters", () => {
- const { actions, cellValue } = setup({
+ const { actions } = setup({
question: SAVED_QUESTION,
column: ORDERS.ID.column(),
extraData: {
@@ -191,15 +191,15 @@ describe("ObjectDetailDrill", () => {
expect(actions).toMatchObject([
{
name: "object-detail",
- url: expect.any(Function),
+ question: expect.any(Function),
},
]);
});
it("should return correct URL to object detail", () => {
const [action] = actions;
- expect(action.url()).toBe(
- `/question/${SAVED_QUESTION.id()}-${SAVED_QUESTION.displayName()}/${cellValue}`,
+ expect(action.question().getUrl()).toBe(
+ `/question/${SAVED_QUESTION.id()}-${SAVED_QUESTION.displayName()}`,
);
});
});
@@ -230,7 +230,7 @@ describe("ObjectDetailDrill", () => {
it("should return correct action", () => {
const [action] = actions;
expect(action.question()).toBe(SAVED_QUESTION);
- expect(action.extra()).toEqual({ objectId: cellValue });
+ expect(action.extra().objectId).toEqual(cellValue);
});
});
});
@@ -255,6 +255,11 @@ describe("ObjectDetailDrill", () => {
filter: ["=", PRODUCTS.ID.reference(), cellValue],
});
});
+
+ it("should supply the foreign key as a return value from the extra() function", () => {
+ const [action] = actions;
+ expect(action.extra().objectId).toEqual(cellValue);
+ });
});
describe("with fk_target_field_id (model with customized metadata)", () => {
@@ -279,6 +284,10 @@ describe("ObjectDetailDrill", () => {
filter: ["=", PRODUCTS.ID.reference(), cellValue],
});
});
+ it("should supply the foreign key as a return value from the extra() function", () => {
+ const [action] = actions;
+ expect(action.extra().objectId).toEqual(cellValue);
+ });
});
});
});
diff --git a/frontend/test/metabase/modes/lib/modes.unit.spec.js b/frontend/test/metabase/modes/lib/modes.unit.spec.js
index 1baa9dc7e9f3ee39773df1aea87271d7a31faa5c..681dc2ce81306f14dc46634dd5d2672a83344885 100644
--- a/frontend/test/metabase/modes/lib/modes.unit.spec.js
+++ b/frontend/test/metabase/modes/lib/modes.unit.spec.js
@@ -1,16 +1,15 @@
import { getMode } from "metabase/modes/lib/modes";
-import ObjectMode from "metabase/modes/components/modes/ObjectMode";
import SegmentMode from "metabase/modes/components/modes/SegmentMode";
import { ORDERS } from "__support__/sample_database_fixture";
describe("modes", () => {
describe("getMode", () => {
- it("should be in object mode when selecting one PK ID", () => {
+ it("should be in segment mode when selecting one PK ID", () => {
const filter = ["=", ["field", ORDERS.ID.id, null], 42];
const query = ORDERS.query().filter(filter);
const question = ORDERS.question().setQuery(query);
- expect(getMode(question)).toBe(ObjectMode);
+ expect(getMode(question)).toBe(SegmentMode);
});
it("should be in segment mode when selecting multiple PK IDs", () => {
const filter = ["=", ["field", ORDERS.ID.id, null], 42, 24];
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 fc39ad08e83649fc9431003e824b844b04d85c99..c4113d40b4ff12468b5670bbc7fd8cd67130c937 100644
--- a/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js
+++ b/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js
@@ -222,7 +222,9 @@ describe("scenarios > models metadata", () => {
cy.findByText("Hudson Borer");
});
- cy.go("back");
+ cy.go("back"); // close modal
+ cy.wait("@dataset");
+ cy.go("back"); // navigate away from drilled table
cy.wait("@dataset");
// Drill to Reviews table
diff --git a/frontend/test/metabase/scenarios/question/new.cy.spec.js b/frontend/test/metabase/scenarios/question/new.cy.spec.js
index d1f31391add0605a058fba8b6fed4a62a2614934..9a08d9e684c93109d1264e6b68ea7a4ce302bf61 100644
--- a/frontend/test/metabase/scenarios/question/new.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/new.cy.spec.js
@@ -230,7 +230,7 @@ describe("scenarios > question > new", () => {
cy.url().should("include", "question#");
});
- it("should correctly choose between 'Object Detail' and 'Table (metabase#13717)", () => {
+ it("composite keys should act as filters on click (metabase#13717)", () => {
cy.request("PUT", `/api/field/${ORDERS.QUANTITY}`, {
semantic_type: "type/PK",
});
@@ -241,6 +241,11 @@ describe("scenarios > question > new", () => {
.eq(1) // first table body cell
.should("contain", "2") // quantity for order ID#1
.click();
+ cy.wait("@dataset");
+
+ cy.get(
+ "#main-data-grid .TableInteractive-cellWrapper--firstColumn",
+ ).should("have.length.gt", 1);
cy.log(
"**Reported at v0.34.3 - v0.37.0.2 / probably was always like this**",
@@ -254,8 +259,12 @@ describe("scenarios > question > new", () => {
.eq(1) // first table body cell
.should("contain", 1)
.click();
+ cy.wait("@dataset");
- cy.get(".ObjectDetail");
+ cy.log("only one row should appear after filtering by ID");
+ cy.get(
+ "#main-data-grid .TableInteractive-cellWrapper--firstColumn",
+ ).should("have.length", 1);
});
// flaky test (#19454)
diff --git a/frontend/test/metabase/scenarios/question/nulls.cy.spec.js b/frontend/test/metabase/scenarios/question/nulls.cy.spec.js
index a321d93da7222035055e4c2f7e3defafb64a72f8..9bac164114631f2fc81ccb22a3753e8e7c6d1078 100644
--- a/frontend/test/metabase/scenarios/question/nulls.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/nulls.cy.spec.js
@@ -22,7 +22,10 @@ describe("scenarios > question > null", () => {
name: "13571",
query: {
"source-table": ORDERS_ID,
- fields: [["field", ORDERS.DISCOUNT, null]],
+ fields: [
+ ["field", ORDERS.ID, null],
+ ["field", ORDERS.DISCOUNT, null],
+ ],
filter: ["=", ["field", ORDERS.ID, null], 1],
},
});
@@ -32,6 +35,7 @@ describe("scenarios > question > null", () => {
cy.findByText("13571").click();
cy.log("'No Results since at least v0.34.3");
+ cy.get("#detail-shortcut").click();
cy.findByText("Discount");
cy.findByText("Empty");
});
diff --git a/frontend/test/metabase/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js b/frontend/test/metabase/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js
index f1c788719322da79af1b4bbbd2f4a8b6931be603..3356d9f746299b8d6f133bad5a3de2269b7efcb7 100644
--- a/frontend/test/metabase/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/reproductions/17514-ui-overlay.cy.spec.js
@@ -140,11 +140,10 @@ describe("issue 17514", () => {
cy.findByText("Products").click();
visualize();
- cy.findByTextEnsureVisible("Subtotal");
// Cypress cannot click elements that are blocked by an overlay so this will immediately fail if the issue is not fixed
- cy.findByText("110.93").click();
- cy.findByText("Filter by this value");
+ cy.findByTextEnsureVisible("Subtotal").click();
+ cy.findByText("Filter by this column");
});
});
});
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 94ed50cb48b10ddcfd9e73b31f242a6b66df71d8..fea720d7c51bcf9803e1eef3233b981bbd6e5f19 100644
--- a/frontend/test/metabase/scenarios/visualizations/object_detail.cy.spec.js
+++ b/frontend/test/metabase/scenarios/visualizations/object_detail.cy.spec.js
@@ -62,6 +62,7 @@ describe("scenarios > question > object details", () => {
getPreviousObjectDetailButton().should("not.exist");
getNextObjectDetailButton().should("not.exist");
+ cy.go("back");
cy.go("back");
cy.wait("@dataset");