From 8aa2cc2f601c6614532c5e0cc3f3b220719226a6 Mon Sep 17 00:00:00 2001
From: Cal Herries <39073188+calherries@users.noreply.github.com>
Date: Wed, 3 May 2023 11:04:14 +0530
Subject: [PATCH] Use display name from model for implicit actions (#30356)
* Use display name from card for implicit actions
* Remove fields arg from generateFieldSettingsFromParameters
* Update implicit action test to include display-name
* Tidy
* Fix unit tests
* Fix e2e test
* Only include display-name in the parameters for implicit action types
* Use `field.displayName()`
* Fix some tests
* Attempt to fix test
* Fix ActionParametersInputForm unit tests
* Only generate field settings from parameters if action.type is implicit
* Fix e2e tests
* Fix unit tests
* Fix tests
---
.../actions-on-dashboards.cy.spec.js | 32 +--
.../scenarios/models/model-actions.cy.spec.js | 4 +-
.../src/metabase-types/api/mocks/actions.ts | 1 +
.../components/ActionViz/Action.unit.spec.tsx | 18 +-
.../ActionParametersInputForm.unit.spec.tsx | 38 +++-
.../actions/hooks/use-action-form/utils.ts | 41 ++--
.../hooks/use-action-form/utils.unit.spec.ts | 193 +++---------------
.../PublicAction/PublicAction.unit.spec.tsx | 2 +
src/metabase/models/action.clj | 10 +-
test/metabase/models/action_test.clj | 24 ++-
10 files changed, 138 insertions(+), 225 deletions(-)
diff --git a/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js b/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
index e1095a3b79b..f3a60bf95a9 100644
--- a/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
+++ b/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
@@ -140,7 +140,7 @@ const MODEL_NAME = "Test Action Model";
clickHelper("Create");
modal().within(() => {
- cy.findByPlaceholderText("Team name").type("Zany Zebras");
+ cy.findByPlaceholderText("Team Name").type("Zany Zebras");
cy.findByPlaceholderText("Score").type("44");
cy.button("Save").click();
@@ -181,7 +181,7 @@ const MODEL_NAME = "Test Action Model";
cy.wait("@executePrefetch");
// let's check that the existing values are pre-filled correctly
modal().within(() => {
- cy.findByPlaceholderText("Team name")
+ cy.findByPlaceholderText("Team Name")
.should("have.value", "Energetic Elephants")
.clear()
.type("Emotional Elephants");
@@ -229,7 +229,7 @@ const MODEL_NAME = "Test Action Model";
clickHelper("Delete");
modal().within(() => {
- cy.findByPlaceholderText("Id").type("3");
+ cy.findByPlaceholderText("ID").type("3");
cy.button("Delete").click();
});
@@ -283,7 +283,7 @@ const MODEL_NAME = "Test Action Model";
modal().within(() => {
changeValue({
- fieldName: "Uuid",
+ fieldName: "UUID",
fieldType: "text",
oldValue: oldRow.uuid,
newValue: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a77",
@@ -321,7 +321,7 @@ const MODEL_NAME = "Test Action Model";
// we can't assert on this value because mysql and postgres seem to
// handle timezones differently 🥴
- cy.findByPlaceholderText("Timestamptz")
+ cy.findByPlaceholderText("TimestampTZ")
.should("have.attr", "type", "datetime-local")
.clear()
.type("2020-05-01T16:45:00");
@@ -370,12 +370,12 @@ const MODEL_NAME = "Test Action Model";
clickHelper("Create");
modal().within(() => {
- cy.findByPlaceholderText("Uuid").type(
+ cy.findByPlaceholderText("UUID").type(
"a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a15",
);
cy.findByPlaceholderText("Integer").type("-20");
- cy.findByPlaceholderText("Integerunsigned").type("20");
+ cy.findByPlaceholderText("IntegerUnsigned").type("20");
cy.findByPlaceholderText("Tinyint").type("101");
cy.findByPlaceholderText("Tinyint1").type("1");
cy.findByPlaceholderText("Smallint").type("32767");
@@ -392,10 +392,10 @@ const MODEL_NAME = "Test Action Model";
cy.findByPlaceholderText("Date").type("2020-02-01");
cy.findByPlaceholderText("Datetime").type("2020-03-01T12:00:00");
- cy.findByPlaceholderText("Datetimetz").type("2020-03-01T12:00:00");
+ cy.findByPlaceholderText("DatetimeTZ").type("2020-03-01T12:00:00");
cy.findByPlaceholderText("Time").type("12:57:57");
cy.findByPlaceholderText("Timestamp").type("2020-03-01T12:00:00");
- cy.findByPlaceholderText("Timestamptz").type("2020-03-01T12:00:00");
+ cy.findByPlaceholderText("TimestampTZ").type("2020-03-01T12:00:00");
cy.button("Save").click();
});
@@ -456,9 +456,9 @@ const MODEL_NAME = "Test Action Model";
clickHelper("Create");
modal().within(() => {
- cy.findByPlaceholderText("Uuid").should("be.visible");
- cy.findByPlaceholderText("Json").should("not.exist");
- cy.findByPlaceholderText("Jsonb").should("not.exist");
+ cy.findByPlaceholderText("UUID").should("be.visible");
+ cy.findByPlaceholderText("JSON").should("not.exist");
+ cy.findByPlaceholderText("JSONB").should("not.exist");
cy.findByPlaceholderText("Binary").should("not.exist");
if (dialect === "mysql") {
@@ -524,14 +524,14 @@ const MODEL_NAME = "Test Action Model";
// the instance is in US/Pacific so it's -8 hours
if (dialect === "postgres") {
changeValue({
- fieldName: "Datetimetz",
+ fieldName: "DatetimeTZ",
fieldType: "datetime-local",
oldValue: "2020-01-01T00:35:55",
newValue: newTime,
});
changeValue({
- fieldName: "Timestamptz",
+ fieldName: "TimestampTZ",
fieldType: "datetime-local",
oldValue: "2020-01-01T00:35:55",
newValue: newTime,
@@ -540,14 +540,14 @@ const MODEL_NAME = "Test Action Model";
if (dialect === "mysql") {
changeValue({
- fieldName: "Datetimetz",
+ fieldName: "DatetimeTZ",
fieldType: "datetime-local",
oldValue: oldRow.datetimeTZ.replace(" ", "T"),
newValue: newTime,
});
changeValue({
- fieldName: "Timestamptz",
+ fieldName: "TimestampTZ",
fieldType: "datetime-local",
oldValue: oldRow.timestampTZ.replace(" ", "T"),
newValue: newTime,
diff --git a/e2e/test/scenarios/models/model-actions.cy.spec.js b/e2e/test/scenarios/models/model-actions.cy.spec.js
index 1bbebf6e312..b0fc4892c3e 100644
--- a/e2e/test/scenarios/models/model-actions.cy.spec.js
+++ b/e2e/test/scenarios/models/model-actions.cy.spec.js
@@ -48,7 +48,7 @@ const TEST_TEMPLATE_TAG = {
id: TEST_PARAMETER.id,
type: "number",
name: TEST_PARAMETER.slug,
- "display-name": "Id",
+ "display-name": TEST_PARAMETER.name,
slug: TEST_PARAMETER.slug,
};
@@ -405,7 +405,7 @@ describe(
cy.visit(url);
// team 2 has 10 points, let's give them more
- cy.findByLabelText("Id").type("2");
+ cy.findByLabelText("ID").type("2");
cy.findByLabelText(/score/i).type("16");
cy.findByLabelText(/team name/i).type("Bouncy Bears");
diff --git a/frontend/src/metabase-types/api/mocks/actions.ts b/frontend/src/metabase-types/api/mocks/actions.ts
index f5fc4e65132..6cadc46e285 100644
--- a/frontend/src/metabase-types/api/mocks/actions.ts
+++ b/frontend/src/metabase-types/api/mocks/actions.ts
@@ -44,6 +44,7 @@ export const createMockQueryAction = ({
archived: false,
created_at: new Date().toISOString(),
updated_at: new Date().toISOString(),
+ visualization_settings: { fields: {} },
public_uuid: null,
...opts,
type: "query",
diff --git a/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx b/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx
index be0613a20c3..c90db49a9fe 100644
--- a/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx
+++ b/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx
@@ -39,15 +39,29 @@ const ACTION = createMockQueryAction({
parameters: [
createMockActionParameter({
id: "parameter_1",
- type: "type/Text",
+ name: "Parameter 1",
+ type: "string/=",
target: ["variable", ["template-tag", "1"]],
}),
createMockActionParameter({
id: "parameter_2",
- type: "type/Integer",
+ name: "Parameter 2",
+ type: "number/=",
target: ["variable", ["template-tag", "2"]],
}),
],
+ visualization_settings: {
+ fields: {
+ parameter_1: createMockFieldSettings({
+ fieldType: "string",
+ inputType: "string",
+ }),
+ parameter_2: createMockFieldSettings({
+ fieldType: "number",
+ inputType: "number",
+ }),
+ },
+ },
});
function createMockActionDashboardCard(
diff --git a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx
index 1678acbea4d..62bc5f29fa6 100644
--- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx
+++ b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx
@@ -9,6 +9,7 @@ import { render, screen } from "__support__/ui";
import {
createMockActionDashboardCard,
createMockActionParameter,
+ createMockFieldSettings,
createMockQueryAction,
createMockImplicitQueryAction,
createMockDashboard,
@@ -23,16 +24,30 @@ import ActionParametersInputModal, {
const parameter1 = createMockActionParameter({
id: "parameter_1",
+ name: "Parameter 1",
type: "type/Text",
});
const parameter2 = createMockActionParameter({
id: "parameter_2",
+ name: "Parameter 2",
type: "type/Text",
});
const mockAction = createMockQueryAction({
parameters: [parameter1, parameter2],
+ visualization_settings: {
+ fields: {
+ parameter_1: createMockFieldSettings({
+ id: "parameter_1",
+ placeholder: "Parameter 1 placeholder",
+ }),
+ parameter_2: createMockFieldSettings({
+ id: "parameter_1",
+ placeholder: "Parameter 2 placeholder",
+ }),
+ },
+ },
});
const defaultProps: ActionParametersInputFormProps = {
@@ -110,14 +125,16 @@ describe("Actions > ActionParametersInputForm", () => {
});
it("should generate field types from parameter types", async () => {
- const action = createMockQueryAction({
+ const action = createMockImplicitQueryAction({
parameters: [
createMockActionParameter({
id: "parameter_1",
+ "display-name": "Parameter 1",
type: "type/Text",
}),
createMockActionParameter({
id: "parameter_2",
+ "display-name": "Parameter 2",
type: "type/Integer",
}),
],
@@ -139,6 +156,18 @@ describe("Actions > ActionParametersInputForm", () => {
const idParameter = createMockActionParameter({ id: "id" });
+ const parameter1 = createMockActionParameter({
+ id: "parameter_1",
+ type: "type/Text",
+ "display-name": "Parameter 1",
+ });
+
+ const parameter2 = createMockActionParameter({
+ id: "parameter_2",
+ type: "type/Text",
+ "display-name": "Parameter 2",
+ });
+
await setup({
action: createMockImplicitQueryAction({
type: "implicit",
@@ -208,10 +237,9 @@ describe("Actions > ActionParametersInputForm", () => {
expect(screen.getByText("My Test Modal")).toBeInTheDocument();
expect(screen.getByTestId("action-form")).toBeInTheDocument();
- expect(screen.getByPlaceholderText("Parameter 1")).toHaveAttribute(
- "type",
- "text",
- );
+ expect(
+ screen.getByPlaceholderText("Parameter 1 placeholder"),
+ ).toHaveAttribute("type", "text");
});
it("should show a delete confirm message with the showConfirmMessage prop", async () => {
diff --git a/frontend/src/metabase/actions/hooks/use-action-form/utils.ts b/frontend/src/metabase/actions/hooks/use-action-form/utils.ts
index 96b8df14d47..36975a5c923 100644
--- a/frontend/src/metabase/actions/hooks/use-action-form/utils.ts
+++ b/frontend/src/metabase/actions/hooks/use-action-form/utils.ts
@@ -1,6 +1,5 @@
import moment from "moment-timezone";
-import { slugify, humanize } from "metabase/lib/formatting";
import { isEmpty } from "metabase/lib/validate";
import { getDefaultFieldSettings } from "metabase/actions/utils";
@@ -116,39 +115,27 @@ export const getInputType = (param: Parameter, field?: Field) => {
return "string";
};
-export const generateFieldSettingsFromParameters = (
- params: Parameter[],
- fields?: Field[],
-) => {
+export const generateFieldSettingsFromParameters = (params: Parameter[]) => {
const fieldSettings: Record<ParameterId, LocalFieldSettings> = {};
- const fieldMetadataMap = Object.fromEntries(
- fields?.map(f => [slugify(f.name), f]) ?? [],
- );
-
params.forEach((param, index) => {
- const field = fieldMetadataMap[param.id]
- ? new Field(fieldMetadataMap[param.id])
- : new Field({
- id: param.id,
- name: param.id,
- slug: param.id,
- display_name: humanize(param.id),
- base_type: param.type,
- semantic_type: param.type,
- });
-
- const name = param["display-name"] ?? param.name ?? param.id;
- const displayName = field?.displayName?.() ?? name;
+ const field = new Field({
+ id: param.id,
+ name: param.id,
+ slug: param.id,
+ display_name: param["display-name"],
+ base_type: param.type,
+ semantic_type: param.type,
+ });
fieldSettings[param.id] = getDefaultFieldSettings({
id: param.id,
- name,
- title: displayName,
- placeholder: displayName,
- required: !!param?.required,
+ name: param.name,
+ title: field.displayName(),
+ placeholder: field.displayName(),
+ required: !!param.required,
order: index,
- description: field?.description ?? "",
+ description: "",
fieldType: getFieldType(param),
inputType: getInputType(param, field),
});
diff --git a/frontend/src/metabase/actions/hooks/use-action-form/utils.unit.spec.ts b/frontend/src/metabase/actions/hooks/use-action-form/utils.unit.spec.ts
index e1561455864..0196adb1b39 100644
--- a/frontend/src/metabase/actions/hooks/use-action-form/utils.unit.spec.ts
+++ b/frontend/src/metabase/actions/hooks/use-action-form/utils.unit.spec.ts
@@ -10,6 +10,7 @@ const createParameter = (options?: any) => {
return {
id: "test_parameter",
name: "Test Parameter",
+ "display-name": "Test Parameter",
type: "type/Text",
...options,
};
@@ -190,10 +191,9 @@ describe("actions > containers > ActionParametersInputForm > utils", () => {
describe("generateFieldSettingsFromParameters", () => {
it("should generate settings for a string field", () => {
- const fields = [createField({ name: "test-field" })];
- const params = [createParameter({ id: "test-field" })];
+ const params = [createParameter({ id: "test-field", type: "type/Text" })];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
expect(settings.fieldType).toBe("string");
@@ -201,18 +201,11 @@ describe("actions > containers > ActionParametersInputForm > utils", () => {
});
it("should generate settings for an Integer field", () => {
- const fields = [
- createField({
- name: "test-field",
- base_type: "type/Integer",
- semantic_type: "type/Integer",
- }),
- ];
const params = [
createParameter({ id: "test-field", type: "type/Integer" }),
];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
expect(settings.fieldType).toBe("number");
@@ -220,50 +213,35 @@ describe("actions > containers > ActionParametersInputForm > utils", () => {
});
it("should generate settings for a float field", () => {
- const fields = [
- createField({
- name: "test-field",
- base_type: "type/Float",
- semantic_type: "type/Float",
- }),
- ];
const params = [
createParameter({ id: "test-field", type: "type/Float" }),
];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
expect(settings.fieldType).toBe("number");
expect(settings.inputType).toBe("number");
});
- it("should generate settings for a category field", () => {
- const fields = [
- createField({ name: "test-field", semantic_type: "type/Category" }),
+ it("generates field settings for an integer field", () => {
+ const params = [
+ createParameter({ id: "test-field", type: "type/Integer" }),
];
- const params = [createParameter({ id: "test-field", type: "type/Text" })];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
- expect(settings.fieldType).toBe("string");
- expect(settings.inputType).toBe("string");
+ expect(settings.fieldType).toBe("number");
+ expect(settings.inputType).toBe("number");
});
it("should generate settings for a dateTime field", () => {
- const fields = [
- createField({
- name: "test-field",
- base_type: "type/DateTime",
- semantic_type: "type/DateTime",
- }),
- ];
const params = [
createParameter({ id: "test-field", type: "type/DateTime" }),
];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
expect(settings.fieldType).toBe("string");
@@ -271,171 +249,64 @@ describe("actions > containers > ActionParametersInputForm > utils", () => {
});
it("should generate settings for a date field", () => {
- const fields = [
- createField({
- name: "test-field",
- base_type: "type/Date",
- semantic_type: "type/Date",
- }),
- ];
const params = [createParameter({ id: "test-field", type: "type/Date" })];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
expect(settings.fieldType).toBe("string");
expect(settings.inputType).toBe("date");
});
- it("should set the parameter id as the object key", () => {
- const fields = [createField({ name: "test-field" })];
- const params = [createParameter({ id: "test-field" })];
- const [id, _settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
- );
-
- expect(id).toEqual("test-field");
- });
-
- it("should get display name from field metadata", () => {
- const fields = [createField({ name: "test-field" })];
- const params = [createParameter({ id: "test-field" })];
+ it("generates field settings for a json field", () => {
+ const params = [
+ createParameter({ id: "test-field", type: "type/Structured" }),
+ ];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
- expect(settings.placeholder).toBe("Test Field");
- expect(settings.title).toBe("Test Field");
+ expect(settings.fieldType).toBe("string");
+ expect(settings.inputType).toBe("text");
});
- it("matches field names to parameter ids case-insensitively", () => {
- const fields = [createField({ name: "TEST-field" })];
+ it("should set the parameter id as the object key", () => {
const params = [createParameter({ id: "test-field" })];
- const [id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ const [id, _settings] = getFirstEntry(
+ generateFieldSettingsFromParameters(params),
);
expect(id).toEqual("test-field");
- expect(settings.placeholder).toBe("Test Field");
- expect(settings.title).toBe("Test Field");
- expect(settings.name).toBe("Test Parameter");
});
- it("sets settings from parameter if there is no corresponding field", () => {
- const fields = [createField({ name: "xyz", description: "foo bar baz" })];
- const params = [createParameter({ id: "test-field", name: null })];
+ it("should get title and placeholder from the parameter", () => {
+ const params = [
+ createParameter({ id: "test-field", "display-name": "Test Field" }),
+ ];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
- expect(settings.placeholder).toBe("Test-field");
- expect(settings.title).toBe("Test-field");
- expect(settings.name).toBe("test-field");
+ expect(settings.placeholder).toBe("Test Field");
+ expect(settings.title).toBe("Test Field");
});
it("sets required prop to true", () => {
- const fields = [createField({ name: "test-field" })];
const params = [createParameter({ id: "test-field", required: true })];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
expect(settings.required).toBe(true);
});
it("sets required prop to false", () => {
- const fields = [createField({ name: "test-field" })];
const params = [createParameter({ id: "test-field", required: false })];
const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
+ generateFieldSettingsFromParameters(params),
);
expect(settings.required).toBe(false);
});
-
- it("sets description text", () => {
- const fields = [
- createField({ name: "test-field", description: "foo bar baz" }),
- ];
- const params = [createParameter({ id: "test-field" })];
- const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params, fields),
- );
-
- expect(settings.description).toBe("foo bar baz");
- });
-
- describe("without field metadata", () => {
- it("humanizes parameter id in the field title", () => {
- const params = [
- createParameter({ id: "test_field", type: "type/Integer" }),
- ];
- const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params),
- );
-
- expect(settings.title).toBe("Test field");
- });
-
- it("generates field settings for a numeric field", () => {
- const params = [
- createParameter({ id: "test-field", type: "type/Integer" }),
- ];
- const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params),
- );
-
- expect(settings.fieldType).toBe("number");
- expect(settings.inputType).toBe("number");
- });
-
- it("generates field settings for a string field", () => {
- const params = [
- createParameter({ id: "test-field", type: "type/String" }),
- ];
- const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params),
- );
-
- expect(settings.fieldType).toBe("string");
- expect(settings.inputType).toBe("string");
- });
-
- it("generates field settings for a date field", () => {
- const params = [
- createParameter({ id: "test-field", type: "type/Date" }),
- ];
- const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params),
- );
-
- expect(settings.fieldType).toBe("string");
- expect(settings.inputType).toBe("date");
- });
-
- it("generates field settings for a datetime field", () => {
- const params = [
- createParameter({ id: "test-field", type: "type/DateTime" }),
- ];
- const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params),
- );
-
- expect(settings.fieldType).toBe("string");
- expect(settings.inputType).toBe("datetime");
- });
-
- it("generates field settings for a json field", () => {
- const params = [
- createParameter({ id: "test-field", type: "type/Structured" }),
- ];
- const [_id, settings] = getFirstEntry(
- generateFieldSettingsFromParameters(params),
- );
-
- expect(settings.fieldType).toBe("string");
- expect(settings.inputType).toBe("text");
- });
- });
});
});
diff --git a/frontend/src/metabase/public/containers/PublicAction/PublicAction.unit.spec.tsx b/frontend/src/metabase/public/containers/PublicAction/PublicAction.unit.spec.tsx
index 11806cd875a..f327e7316f3 100644
--- a/frontend/src/metabase/public/containers/PublicAction/PublicAction.unit.spec.tsx
+++ b/frontend/src/metabase/public/containers/PublicAction/PublicAction.unit.spec.tsx
@@ -27,6 +27,7 @@ const TEST_PUBLIC_ID = "test-public-id";
const SIZE_PARAMETER = createMockActionParameter({
id: "size",
name: "Size",
+ "display-name": "Size",
type: "number/=",
slug: "size",
target: ["variable", ["template-tag", "size"]],
@@ -35,6 +36,7 @@ const SIZE_PARAMETER = createMockActionParameter({
const COLOR_PARAMETER = createMockActionParameter({
id: "color",
name: "Color",
+ "display-name": "Color",
type: "string/=",
slug: "color",
target: ["variable", ["template-tag", "color"]],
diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj
index 9de56873748..327ebda54ce 100644
--- a/src/metabase/models/action.clj
+++ b/src/metabase/models/action.clj
@@ -206,10 +206,13 @@
:let [fields (:fields table)]
;; Skip tables for have conflicting slugified columns i.e. table has "name" and "NAME" columns.
:when (unique-field-slugs? fields)
- :let [card (get card-by-table-id (:id table))
- exposed-fields (into #{} (keep :id) (:result_metadata card))
+ :let [card (get card-by-table-id (:id table))
+ id->metadata (m/index-by :id (:result_metadata card))
parameters (->> fields
- (filter #(contains? exposed-fields (:id %)))
+ ;; get display_name from metadata
+ (keep (fn [field]
+ (when-let [metadata (id->metadata (:id field))]
+ (assoc field :display_name (:display_name metadata)))))
;; remove exploded json fields and any structured field
(remove (some-fn
;; exploded json fields can't be recombined in sql yet
@@ -220,6 +223,7 @@
(comp #{:type/*} :effective_type)))
(map (fn [field]
{:id (u/slugify (:name field))
+ :display-name (:display_name field)
:target [:variable [:template-tag (u/slugify (:name field))]]
:type (:base_type field)
:required (:database_required field)
diff --git a/test/metabase/models/action_test.clj b/test/metabase/models/action_test.clj
index f379bd632ad..3efdc3da633 100644
--- a/test/metabase/models/action_test.clj
+++ b/test/metabase/models/action_test.clj
@@ -6,6 +6,7 @@
[metabase.driver :as driver]
[metabase.models :refer [Action Card Dashboard DashboardCard]]
[metabase.models.action :as action]
+ [metabase.query-processor :as qp]
[metabase.sync :as sync]
[metabase.test :as mt]
[metabase.test.data.one-off-dbs :as one-off-dbs]
@@ -26,15 +27,20 @@
(deftest hydrate-implicit-action-test
(mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
(mt/with-actions-test-data-and-actions-enabled
- (mt/with-actions [{:keys [action-id] :as _context} {:type :implicit}]
- (is (partial= {:id action-id
- :name "Update Example"
- :database_id (mt/id)
- :parameters [(if (= driver/*driver* :h2)
- {:type :type/BigInteger}
- {:type :type/Integer})
- {:type :type/Text, :id "name"}]}
- (action/select-action :id action-id))))))
+ (let [query (mt/mbql-query categories)]
+ (mt/with-actions [_model {:dataset true
+ :dataset_query query
+ :result_metadata (assoc-in (get-in (qp/process-query query) [:data :results_metadata :columns])
+ [1 :display_name] "Display Name")}
+ {:keys [action-id] :as _context} {:type :implicit}]
+ (is (partial= {:id action-id
+ :name "Update Example"
+ :database_id (mt/id)
+ :parameters [(if (= driver/*driver* :h2)
+ {:type :type/BigInteger}
+ {:type :type/Integer})
+ {:type :type/Text, :id "name" :display-name "Display Name"}]}
+ (action/select-action :id action-id)))))))
(testing "Implicit actions do not map parameters to json fields (parents or nested)"
(mt/test-drivers (mt/normal-drivers-with-feature :actions/custom :nested-field-columns)
(mt/dataset json
--
GitLab