diff --git a/frontend/src/metabase-lib/lib/Mode/Mode.ts b/frontend/src/metabase-lib/lib/Mode/Mode.ts
index b684dc17bc3f1d9ce632b36fbe7b541afe68815f..a523bbcb1f2261d1944f4538496748e3574e09f3 100644
--- a/frontend/src/metabase-lib/lib/Mode/Mode.ts
+++ b/frontend/src/metabase-lib/lib/Mode/Mode.ts
@@ -1,4 +1,3 @@
-import { getMode } from "metabase/modes/lib/modes";
import {
ClickAction,
ClickObject,
@@ -15,16 +14,6 @@ export default class Mode {
this._queryMode = queryMode;
}
- static forQuestion(question: Question): Mode | null {
- // TODO Atte Keinänen 6/22/17: Move getMode here and refactor it after writing tests
- const queryMode = getMode(question);
- if (queryMode) {
- return new Mode(question, queryMode);
- }
-
- return null;
- }
-
queryMode() {
return this._queryMode;
}
diff --git a/frontend/src/metabase-lib/lib/Mode/index.ts b/frontend/src/metabase-lib/lib/Mode/index.ts
index e42afab4d3da45a9432e8595cfbdbb5e4004495c..5584a7e25ba6536567fa8278bbfdac670874bdc5 100644
--- a/frontend/src/metabase-lib/lib/Mode/index.ts
+++ b/frontend/src/metabase-lib/lib/Mode/index.ts
@@ -1,2 +1,2 @@
export { default } from "./Mode";
-export { getMode } from "./utils";
+export { getModeType } from "./utils";
diff --git a/frontend/src/metabase-lib/lib/Mode/utils.ts b/frontend/src/metabase-lib/lib/Mode/utils.ts
index 974e2f7e47203010f57d5d72cc5d3595a64c8691..30a7336e2888d9cf6404511e2f48733eedf47ddd 100644
--- a/frontend/src/metabase-lib/lib/Mode/utils.ts
+++ b/frontend/src/metabase-lib/lib/Mode/utils.ts
@@ -13,7 +13,7 @@ import {
} from "metabase-lib/lib/Mode/constants";
import { ModeType } from "./types";
-export function getMode(question: Question): ModeType | null {
+export function getModeType(question: Question): ModeType | null {
if (!question) {
return null;
}
diff --git a/frontend/src/metabase-lib/lib/Question.ts b/frontend/src/metabase-lib/lib/Question.ts
index 21df1eb0bad1700d8b90f74a9c562f06f945641e..66b14b0dc953b2bdd50764f26c3c4f34bc00433c 100644
--- a/frontend/src/metabase-lib/lib/Question.ts
+++ b/frontend/src/metabase-lib/lib/Question.ts
@@ -820,32 +820,6 @@ class QuestionInner {
}
}
- mode(): Mode | null | undefined {
- return Mode.forQuestion(this);
- }
-
- /**
- * Returns true if, based on filters and table columns, the expected result is a single row.
- * However, it might not be true when a PK column is not unique, leading to multiple rows.
- * Because of that, always check query results in addition to this property.
- */
- isObjectDetail(): boolean {
- const mode = this.mode();
- return mode ? mode.name() === "object" : false;
- }
-
- objectDetailPK(): any {
- const query = this.query();
-
- if (this.isObjectDetail() && query instanceof StructuredQuery) {
- const filters = query.filters();
-
- if (filters[0] && isStandard(filters[0])) {
- return filters[0][2];
- }
- }
- }
-
/**
* A user-defined name for the question
*/
@@ -1329,10 +1303,9 @@ class QuestionInner {
}
}
-export default class Question extends memoizeClass<QuestionInner>(
- "query",
- "mode",
-)(QuestionInner) {
+export default class Question extends memoizeClass<QuestionInner>("query")(
+ QuestionInner,
+) {
/**
* TODO Atte Keinänen 6/13/17: Discussed with Tom that we could use the default Question constructor instead,
* but it would require changing the constructor signature so that `card` is an optional parameter and has a default value
diff --git a/frontend/src/metabase/modes/lib/modes.ts b/frontend/src/metabase/modes/lib/modes.ts
index 8da65f966bc1e22fef77b06e9d9c3890c549e4fb..7e51b535682a5041d02bf71e3d0291fb2ee60bac 100644
--- a/frontend/src/metabase/modes/lib/modes.ts
+++ b/frontend/src/metabase/modes/lib/modes.ts
@@ -1,6 +1,6 @@
import { QueryMode } from "metabase-types/types/Visualization";
import Question from "metabase-lib/lib/Question";
-import { getMode as getModeFromLib } from "metabase-lib/lib/Mode";
+import Mode, { getModeType } from "metabase-lib/lib/Mode";
import {
MODE_TYPE_ACTION,
MODE_TYPE_NATIVE,
@@ -19,8 +19,13 @@ import PivotMode from "../components/modes/PivotMode";
import NativeMode from "../components/modes/NativeMode";
import DefaultMode from "../components/modes/DefaultMode";
-export function getMode(question: Question): QueryMode | any | null {
- const mode = getModeFromLib(question);
+export function getMode(question: Question): Mode | null {
+ const queryMode = getQueryMode(question);
+ return queryMode ? new Mode(question, queryMode) : null;
+}
+
+export function getQueryMode(question: Question): QueryMode | any | null {
+ const mode = getModeType(question);
if (!mode) {
return null;
}
diff --git a/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx b/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx
index 6ae9508af4de09bc4cefd962ef5eaa01c0ea6f86..123e40ec13ebb1a962b1c94f2e914e21eeaa223a 100644
--- a/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx
+++ b/frontend/src/metabase/query_builder/components/view/QuestionDataSource.jsx
@@ -202,12 +202,6 @@ function getDataSourceParts({ question, subHead, isObjectDetail }) {
);
}
- if (isObjectDetail) {
- parts.push({
- name: question.objectDetailPK(),
- });
- }
-
return parts.filter(
part => React.isValidElement(part) || part.name || part.icon,
);
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index 576046db6016b62a64288f3329484d5559bb2bde..fd7191ad598b3f6cedaa14b06495254b2bc731b6 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -24,6 +24,7 @@ import { getMetadata } from "metabase/selectors/metadata";
import { getAlerts } from "metabase/alert/selectors";
import { getEmbedOptions, getIsEmbedded } from "metabase/selectors/embed";
import { parseTimestamp } from "metabase/lib/time";
+import { getMode as getQuestionMode } from "metabase/modes/lib/modes";
import { getSortedTimelines } from "metabase/lib/timelines";
import { getSetting } from "metabase/selectors/settings";
import {
@@ -559,7 +560,9 @@ const isZoomingRow = createSelector(
export const getMode = createSelector(
[getLastRunQuestion, isZoomingRow],
(question, isZoomingRow) =>
- isZoomingRow ? new Mode(question, ObjectMode) : question && question.mode(),
+ isZoomingRow
+ ? new Mode(question, ObjectMode)
+ : question && getQuestionMode(question),
);
export const getIsObjectDetail = createSelector(
diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx
index c7fee3f237de1b21f7a3000ac56033916f33c6a5..5e82673bb2e0123960fa211f86078f4189829001 100644
--- a/frontend/src/metabase/visualizations/components/Visualization.jsx
+++ b/frontend/src/metabase/visualizations/components/Visualization.jsx
@@ -25,6 +25,7 @@ import { isSameSeries } from "metabase/visualizations/lib/utils";
import { performDefaultAction } from "metabase/visualizations/lib/action";
import { getFont } from "metabase/styled-components/selectors";
+import { getMode } from "metabase/modes/lib/modes";
import Utils from "metabase/lib/utils";
import {
@@ -222,7 +223,9 @@ class Visualization extends React.PureComponent {
return new Mode(question, maybeModeOrQueryMode);
}
- return question?.mode();
+ if (question) {
+ return getMode(question);
+ }
}
getClickActions(clicked) {
diff --git a/frontend/test/metabase-lib/lib/Mode.unit.spec.js b/frontend/test/metabase-lib/lib/Mode.unit.spec.js
index 6401f5ec6951979495635d4034f1857f7f00b403..47f4a40412f7d27c319a34aa281f0118f24aaad6 100644
--- a/frontend/test/metabase-lib/lib/Mode.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Mode.unit.spec.js
@@ -6,6 +6,7 @@ import {
PEOPLE,
} from "__support__/sample_database_fixture";
+import { getMode } from "metabase/modes/lib/modes";
import Question from "metabase-lib/lib/Question";
describe("Mode", () => {
@@ -17,25 +18,27 @@ describe("Mode", () => {
// testbed for generative testing? see http://leebyron.com/testcheck-js
it("returns `segment` mode with raw data", () => {
- const mode = rawDataQuery.question().mode();
+ const question = rawDataQuery.question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("segment");
});
it("returns `metric` mode with >= 1 aggregations", () => {
- const mode = rawDataQuery.aggregate(["count"]).question().mode();
+ const question = rawDataQuery.aggregate(["count"]).question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("metric");
});
it("returns `timeseries` mode with >=1 aggregations and date breakout", () => {
- const mode = rawDataQuery
+ const question = rawDataQuery
.aggregate(["count"])
.breakout(["field", ORDERS.CREATED_AT.id, { "temporal-unit": "day" }])
- .question()
- .mode();
+ .question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("timeseries");
});
it("returns `timeseries` mode with >=1 aggregations and date + category breakout", () => {
- const mode = rawDataQuery
+ const question = rawDataQuery
.aggregate(["count"])
.breakout(["field", ORDERS.CREATED_AT.id, { "temporal-unit": "day" }])
.breakout([
@@ -43,26 +46,26 @@ describe("Mode", () => {
PRODUCTS.CATEGORY.id,
{ "source-field": ORDERS.PRODUCT_ID.id },
])
- .question()
- .mode();
+ .question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("timeseries");
});
it("returns `geo` mode with >=1 aggregations and an address breakout", () => {
- const mode = rawDataQuery
+ const question = rawDataQuery
.aggregate(["count"])
.breakout([
"field",
PEOPLE.STATE.id,
{ "source-field": ORDERS.USER_ID.id },
])
- .question()
- .mode();
+ .question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("geo");
});
it("returns `pivot` mode with >=1 aggregations and 1-2 category breakouts", () => {
- const mode = rawDataQuery
+ const question = rawDataQuery
.aggregate(["count"])
.breakout([
"field",
@@ -74,21 +77,21 @@ describe("Mode", () => {
PEOPLE.STATE.id,
{ "source-field": ORDERS.USER_ID.id },
])
- .question()
- .mode();
+ .question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("pivot");
});
it("returns `segment` mode with pk filter", () => {
- const mode = rawDataQuery
+ const question = rawDataQuery
.filter(["=", ["field", ORDERS.ID.id, null], 42])
- .question()
- .mode();
+ .question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("segment");
});
it("returns `default` mode with >=0 aggregations and >=3 breakouts", () => {
- const mode = rawDataQuery
+ const question = rawDataQuery
.aggregate(["count"])
.breakout(["field", ORDERS.CREATED_AT.id, { "temporal-unit": "day" }])
.breakout([
@@ -101,8 +104,8 @@ describe("Mode", () => {
PEOPLE.STATE.id,
{ "source-field": ORDERS.USER_ID.id },
])
- .question()
- .mode();
+ .question();
+ const mode = getMode(question);
expect(mode && mode.name()).toEqual("default");
});
it("returns `default` mode with >=1 aggregations and >=1 breakouts when first neither date or category", () => {});
@@ -126,7 +129,7 @@ describe("Mode", () => {
// this is action-specific so just rudimentary tests here showing that the actionsForClick logic works
// Action-specific tests would optimally be in their respective test files
describe("for a question with an aggregation and a time breakout", () => {
- const timeBreakoutQuestionMode = Question.create({
+ const question = Question.create({
databaseId: SAMPLE_DATABASE.id,
tableId: ORDERS.id,
metadata,
@@ -135,16 +138,12 @@ describe("Mode", () => {
.aggregate(["count"])
.breakout(["field", 1, { "temporal-unit": "day" }])
.question()
- .setDisplay("table")
- .mode();
+ .setDisplay("table");
+ const mode = getMode(question);
it("has pivot as mode actions 1 and 2", () => {
- expect(timeBreakoutQuestionMode.actionsForClick()[0].name).toBe(
- "pivot-by-category",
- );
- expect(timeBreakoutQuestionMode.actionsForClick()[1].name).toBe(
- "pivot-by-location",
- );
+ expect(mode.actionsForClick()[0].name).toBe("pivot-by-category");
+ expect(mode.actionsForClick()[1].name).toBe("pivot-by-location");
});
});
});
diff --git a/frontend/test/metabase-lib/lib/Question.unit.spec.js b/frontend/test/metabase-lib/lib/Question.unit.spec.js
index 9e2647602129fbbc183f675eb8c7a22d982004f7..6f044564b6ed6b60ca10e7446f33d03583bf0437 100644
--- a/frontend/test/metabase-lib/lib/Question.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/Question.unit.spec.js
@@ -171,9 +171,6 @@ describe("Question", () => {
it("has correct display settings", () => {
expect(question.display()).toBe("table");
});
- it("has correct mode", () => {
- expect(question.mode().name()).toBe("segment");
- });
});
describe("Question.create(...)", () => {
@@ -381,31 +378,6 @@ describe("Question", () => {
// At the same time, the choice that which actions are visible depend on the question's properties
// as actions are filtered using those
describe("METHODS FOR DRILL-THROUGH / ACTION WIDGET", () => {
- const rawDataQuestion = new Question(orders_raw_card, metadata);
- const timeBreakoutQuestion = Question.create({
- databaseId: SAMPLE_DATABASE.id,
- tableId: ORDERS.id,
- metadata,
- })
- .query()
- .aggregate(["count"])
- .breakout(["field", 1, { "temporal-unit": "day" }])
- .question()
- .setDisplay("table");
-
- describe("mode()", () => {
- describe("for a new question with Orders table and Raw data aggregation", () => {
- it("returns the correct mode", () => {
- expect(rawDataQuestion.mode().name()).toBe("segment");
- });
- });
- describe("for a question with an aggregation and a time breakout", () => {
- it("returns the correct mode", () => {
- expect(timeBreakoutQuestion.mode().name()).toBe("timeseries");
- });
- });
- });
-
describe("aggregate(...)", () => {
const question = new Question(orders_raw_card, metadata);
it("returns the correct query for a summarization of a raw data table", () => {
diff --git a/frontend/test/metabase/modes/lib/modes.unit.spec.js b/frontend/test/metabase/modes/lib/modes.unit.spec.js
index 681dc2ce81306f14dc46634dd5d2672a83344885..e91ad1d52735fa32edc84a8861ffbd9b61e0a8cb 100644
--- a/frontend/test/metabase/modes/lib/modes.unit.spec.js
+++ b/frontend/test/metabase/modes/lib/modes.unit.spec.js
@@ -1,21 +1,21 @@
-import { getMode } from "metabase/modes/lib/modes";
+import { getQueryMode } from "metabase/modes/lib/modes";
import SegmentMode from "metabase/modes/components/modes/SegmentMode";
import { ORDERS } from "__support__/sample_database_fixture";
describe("modes", () => {
- describe("getMode", () => {
+ describe("getQueryMode", () => {
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(SegmentMode);
+ expect(getQueryMode(question)).toBe(SegmentMode);
});
it("should be in segment mode when selecting multiple PK IDs", () => {
const filter = ["=", ["field", ORDERS.ID.id, null], 42, 24];
const query = ORDERS.query().filter(filter);
const question = ORDERS.question().setQuery(query);
- expect(getMode(question)).toBe(SegmentMode);
+ expect(getQueryMode(question)).toBe(SegmentMode);
});
});
});