From 88aa0fab6f0542c4c7503c00c53b2cc04272a4f8 Mon Sep 17 00:00:00 2001
From: Ryan Laurie <30528226+iethree@users.noreply.github.com>
Date: Fri, 24 Feb 2023 03:22:41 -0700
Subject: [PATCH] Fallback to generating action form fields from parameters
(#28512)
* fallback to generating form metadata from parameters
* fix actions test
* update unit tests
* update e2e test strings
---
.../components/ActionViz/Action.unit.spec.tsx | 14 +--
.../ActionParametersInputForm.unit.spec.tsx | 20 ++--
frontend/src/metabase/actions/utils.ts | 12 +-
.../src/metabase/actions/utils.unit.spec.ts | 113 +++++++++++++++++-
.../actions-on-dashboards.cy.spec.js | 10 +-
.../scenarios/models/model-actions.cy.spec.js | 2 +-
6 files changed, 140 insertions(+), 31 deletions(-)
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 536365f1d68..95d783d12d9 100644
--- a/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx
+++ b/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx
@@ -25,14 +25,12 @@ const defaultProps = {
database_id: 2,
parameters: [
createMockActionParameter({
- id: "1",
- name: "Parameter 1",
+ id: "parameter_1",
type: "type/Text",
target: ["variable", ["template-tag", "1"]],
}),
createMockActionParameter({
- id: "2",
- name: "Parameter 2",
+ id: "parameter_2",
type: "type/Text",
target: ["variable", ["template-tag", "2"]],
}),
@@ -219,8 +217,8 @@ describe("Actions > ActionViz > ActionComponent", () => {
const expectedBody = {
modelId: 777,
parameters: {
- "1": "foo",
- "2": "bar",
+ parameter_1: "foo",
+ parameter_2: "bar",
},
};
@@ -252,8 +250,8 @@ describe("Actions > ActionViz > ActionComponent", () => {
const expectedBody = {
modelId: 777,
parameters: {
- "1": "foo",
- "2": "baz",
+ parameter_1: "foo",
+ parameter_2: "baz",
},
};
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 193fae26394..2474d3165d4 100644
--- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx
+++ b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx
@@ -19,13 +19,11 @@ import ActionParametersInputModal from "./ActionParametersInputModal";
const defaultProps = {
missingParameters: [
createMockActionParameter({
- id: "1",
- name: "Parameter 1",
+ id: "parameter_1",
type: "type/Text",
}),
createMockActionParameter({
- id: "2",
- name: "Parameter 2",
+ id: "parameter_2",
type: "type/Text",
}),
],
@@ -55,8 +53,8 @@ async function setupModal(options?: any) {
function setupPrefetch() {
fetchMock.get("path:/api/dashboard/123/dashcard/456/execute", {
- "1": "uno",
- "2": "dos",
+ parameter_1: "uno",
+ parameter_2: "dos",
});
}
@@ -95,8 +93,8 @@ describe("Actions > ActionParametersInputForm", () => {
await waitFor(() => {
expect(submitSpy).toHaveBeenCalledWith({
- "1": "uno",
- "2": "dos",
+ parameter_1: "uno",
+ parameter_2: "dos",
});
});
});
@@ -104,13 +102,11 @@ describe("Actions > ActionParametersInputForm", () => {
it("should generate field types from parameter types", async () => {
const missingParameters = [
createMockActionParameter({
- id: "1",
- name: "Parameter 1",
+ id: "parameter_1",
type: "type/Text",
}),
createMockActionParameter({
- id: "2",
- name: "Parameter 2",
+ id: "parameter_2",
type: "type/Integer",
}),
];
diff --git a/frontend/src/metabase/actions/utils.ts b/frontend/src/metabase/actions/utils.ts
index 3cb49b8da81..b2b1e4ffffd 100644
--- a/frontend/src/metabase/actions/utils.ts
+++ b/frontend/src/metabase/actions/utils.ts
@@ -17,7 +17,7 @@ import type {
} from "metabase-types/api";
import { getResponseErrorMessage } from "metabase/core/utils/errors";
-import { slugify } from "metabase/lib/formatting";
+import { slugify, humanize } from "metabase/lib/formatting";
import { isEmpty } from "metabase/lib/validate";
import { TYPE } from "metabase-lib/types/constants";
@@ -141,7 +141,14 @@ export const generateFieldSettingsFromParameters = (
params.forEach((param, index) => {
const field = fieldMetadataMap[param.id]
? new Field(fieldMetadataMap[param.id])
- : undefined;
+ : 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;
@@ -156,7 +163,6 @@ export const generateFieldSettingsFromParameters = (
description: field?.description ?? "",
fieldType: getFieldType(param),
inputType: getInputType(param, field),
- field: field ?? undefined,
});
});
return fieldSettings;
diff --git a/frontend/src/metabase/actions/utils.unit.spec.ts b/frontend/src/metabase/actions/utils.unit.spec.ts
index 9317e2d1334..cb25969e16e 100644
--- a/frontend/src/metabase/actions/utils.unit.spec.ts
+++ b/frontend/src/metabase/actions/utils.unit.spec.ts
@@ -124,6 +124,42 @@ describe("sortActionParams", () => {
expect(settings.inputType).toBe("category");
});
+ 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),
+ );
+
+ expect(settings.fieldType).toBe("string");
+ expect(settings.inputType).toBe("datetime");
+ });
+
+ 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),
+ );
+
+ 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" })];
@@ -165,8 +201,8 @@ describe("sortActionParams", () => {
generateFieldSettingsFromParameters(params, fields),
);
- expect(settings.placeholder).toBe("test-field");
- expect(settings.title).toBe("test-field");
+ expect(settings.placeholder).toBe("Test-field");
+ expect(settings.title).toBe("Test-field");
expect(settings.name).toBe("test-field");
});
@@ -201,6 +237,79 @@ describe("sortActionParams", () => {
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");
+ });
+ });
});
describe("getInputType", () => {
diff --git a/frontend/test/metabase/scenarios/dashboard/actions-on-dashboards.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/actions-on-dashboards.cy.spec.js
index ec90a209d5b..5896dbc38fb 100644
--- a/frontend/test/metabase/scenarios/dashboard/actions-on-dashboards.cy.spec.js
+++ b/frontend/test/metabase/scenarios/dashboard/actions-on-dashboards.cy.spec.js
@@ -109,8 +109,8 @@ const MODEL_NAME = "Test Action Model";
cy.button("Create").click();
modal().within(() => {
- cy.findByPlaceholderText("team_name").type("Zany Zebras");
- cy.findByPlaceholderText("score").type("44");
+ cy.findByPlaceholderText("Team name").type("Zany Zebras");
+ cy.findByPlaceholderText("Score").type("44");
cy.button("Save").click();
});
@@ -152,12 +152,12 @@ 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");
- cy.findByPlaceholderText("score")
+ cy.findByPlaceholderText("Score")
.should("have.value", "30")
.clear()
.type("88");
@@ -202,7 +202,7 @@ const MODEL_NAME = "Test Action Model";
cy.button("Delete").click();
modal().within(() => {
- cy.findByPlaceholderText("id").type("3");
+ cy.findByPlaceholderText("Id").type("3");
cy.button("Delete").click();
});
diff --git a/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js b/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js
index 6a47754f650..62ff5459f3b 100644
--- a/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js
+++ b/frontend/test/metabase/scenarios/models/model-actions.cy.spec.js
@@ -239,7 +239,7 @@ describe(
cy.visit(url);
// Order 1 has quantity 2 by default, so we're not actually mutating data
- cy.findByLabelText("id").type("1");
+ cy.findByLabelText("Id").type("1");
cy.findByLabelText(/quantity/i).type("2");
cy.findByRole("button", { name: "Submit" }).click();
--
GitLab