From cb51e574de31c7d4485a9dfbef3261f67c0b7495 Mon Sep 17 00:00:00 2001
From: Ryan Laurie <30528226+iethree@users.noreply.github.com>
Date: Mon, 12 Sep 2022 08:11:47 -0600
Subject: [PATCH] Action Creator 4: Derive parameter types from form settings
on save (#25297)
* derive parameter types from form settings on save
* update template tag types with parameter types
* improve tests
---
frontend/src/metabase-types/api/index.ts | 1 +
frontend/src/metabase-types/api/parameters.ts | 27 ++
.../api/writeback-form-settings.ts | 11 +-
.../src/metabase/entities/actions/actions.ts | 18 +-
.../metabase/entities/actions/constants.ts | 30 +++
.../src/metabase/entities/actions/utils.ts | 67 ++++-
.../entities/actions/utils.unit.spec.ts | 237 +++++++++++++++---
.../FormCreator/FieldSettingsPopover.tsx | 2 +-
.../ActionCreator/FormCreator/constants.ts | 6 +-
.../ActionCreator/FormCreator/utils.ts | 12 +-
10 files changed, 358 insertions(+), 53 deletions(-)
create mode 100644 frontend/src/metabase-types/api/parameters.ts
create mode 100644 frontend/src/metabase/entities/actions/constants.ts
diff --git a/frontend/src/metabase-types/api/index.ts b/frontend/src/metabase-types/api/index.ts
index 0d9de43dfab..83c3028cca1 100644
--- a/frontend/src/metabase-types/api/index.ts
+++ b/frontend/src/metabase-types/api/index.ts
@@ -25,3 +25,4 @@ export * from "./timeline";
export * from "./user";
export * from "./writeback";
export * from "./writeback-form-settings";
+export * from "./parameters";
diff --git a/frontend/src/metabase-types/api/parameters.ts b/frontend/src/metabase-types/api/parameters.ts
new file mode 100644
index 00000000000..312c18382c2
--- /dev/null
+++ b/frontend/src/metabase-types/api/parameters.ts
@@ -0,0 +1,27 @@
+export type StringParameterType =
+ | "string/="
+ | "string/!="
+ | "string/contains"
+ | "string/does-not-contain"
+ | "string/starts-with"
+ | "string/ends-with";
+
+export type NumberParameterType =
+ | "number/="
+ | "number/!="
+ | "number/between"
+ | "number/>="
+ | "number/<=";
+
+export type DateParameterType =
+ | "date/single"
+ | "date/range"
+ | "date/relative"
+ | "date/month-year"
+ | "date/quarter-year";
+("date/all-options");
+
+export type ParameterType =
+ | StringParameterType
+ | NumberParameterType
+ | DateParameterType;
diff --git a/frontend/src/metabase-types/api/writeback-form-settings.ts b/frontend/src/metabase-types/api/writeback-form-settings.ts
index 311575b6a3f..6b0df92174a 100644
--- a/frontend/src/metabase-types/api/writeback-form-settings.ts
+++ b/frontend/src/metabase-types/api/writeback-form-settings.ts
@@ -1,15 +1,16 @@
export type FormType = "inline" | "modal";
-export type FieldType = "text" | "number" | "date" | "category";
+export type FieldType = "string" | "number" | "date" | "category";
+
+export type DateInputType = "date" | "datetime" | "monthyear" | "quarteryear";
+
export type InputType =
+ | DateInputType
| "string"
| "text"
| "number"
- | "date"
- | "datetime"
- | "monthyear"
- | "quarteryear"
| "dropdown"
| "inline-select";
+
export type Size = "small" | "medium" | "large";
export type DateRange = [string, string];
diff --git a/frontend/src/metabase/entities/actions/actions.ts b/frontend/src/metabase/entities/actions/actions.ts
index fb905ed5e45..08b581e0579 100644
--- a/frontend/src/metabase/entities/actions/actions.ts
+++ b/frontend/src/metabase/entities/actions/actions.ts
@@ -6,7 +6,11 @@ import type { ActionFormSettings } from "metabase-types/api";
import { CardApi } from "metabase/services";
import { saveForm } from "./forms";
-import { removeOrphanSettings } from "metabase/entities/actions/utils";
+import {
+ removeOrphanSettings,
+ setParameterTypesFromFieldSettings,
+ setTemplateTagTypesFromFieldSettings,
+} from "metabase/entities/actions/utils";
type ActionParams = {
name: string;
@@ -24,12 +28,17 @@ const getAPIFn =
question,
collection_id,
formSettings,
- }: ActionParams) =>
- apifn({
+ }: ActionParams) => {
+ question = setTemplateTagTypesFromFieldSettings(formSettings, question);
+
+ return apifn({
...question.card(),
name,
description,
- parameters: question.parameters(),
+ parameters: setParameterTypesFromFieldSettings(
+ formSettings,
+ question.parameters(),
+ ),
is_write: true,
display: "table",
visualization_settings: removeOrphanSettings(
@@ -38,6 +47,7 @@ const getAPIFn =
),
collection_id,
});
+ };
const createAction = getAPIFn(CardApi.create);
const updateAction = getAPIFn(CardApi.update);
diff --git a/frontend/src/metabase/entities/actions/constants.ts b/frontend/src/metabase/entities/actions/constants.ts
new file mode 100644
index 00000000000..da65b5b94b7
--- /dev/null
+++ b/frontend/src/metabase/entities/actions/constants.ts
@@ -0,0 +1,30 @@
+import type { ParameterType } from "metabase-types/api";
+import type { TemplateTagType } from "metabase-types/types/Query";
+
+interface FieldTypeMap {
+ [key: string]: ParameterType;
+}
+
+interface TagTypeMap {
+ [key: string]: TemplateTagType;
+}
+
+export const fieldTypeToParameterTypeMap: FieldTypeMap = {
+ string: "string/=",
+ category: "string/=",
+ number: "number/=",
+};
+
+export const dateTypetoParameterTypeMap: FieldTypeMap = {
+ date: "date/single",
+ datetime: "date/single",
+ monthyear: "date/month-year",
+ quarteryear: "date/quarter-year",
+};
+
+export const fieldTypeToTagTypeMap: TagTypeMap = {
+ string: "text",
+ category: "text",
+ number: "number",
+ date: "date",
+};
diff --git a/frontend/src/metabase/entities/actions/utils.ts b/frontend/src/metabase/entities/actions/utils.ts
index 191e39f0ada..a437eaaef92 100644
--- a/frontend/src/metabase/entities/actions/utils.ts
+++ b/frontend/src/metabase/entities/actions/utils.ts
@@ -1,7 +1,21 @@
import _ from "underscore";
+import type {
+ ActionFormSettings,
+ FieldType,
+ InputType,
+ ParameterType,
+} from "metabase-types/api";
-import type { ActionFormSettings } from "metabase-types/api";
import type { Parameter as ParameterObject } from "metabase-types/types/Parameter";
+import type { TemplateTag, TemplateTagType } from "metabase-types/types/Query";
+import type NativeQuery from "metabase-lib/lib/queries/NativeQuery";
+import type Question from "metabase-lib/lib/Question";
+
+import {
+ fieldTypeToParameterTypeMap,
+ dateTypetoParameterTypeMap,
+ fieldTypeToTagTypeMap,
+} from "./constants";
export const removeOrphanSettings = (
settings: ActionFormSettings,
@@ -16,3 +30,54 @@ export const removeOrphanSettings = (
fields: _.omit(settings.fields, orphanIds),
};
};
+
+const getParameterTypeFromFieldSettings = (
+ fieldType: FieldType,
+ inputType: InputType,
+): ParameterType => {
+ if (fieldType === "date") {
+ return dateTypetoParameterTypeMap[inputType] ?? "date/single";
+ }
+
+ return fieldTypeToParameterTypeMap[fieldType] ?? "string/=";
+};
+
+const getTagTypeFromFieldSettings = (fieldType: FieldType): TemplateTagType => {
+ return fieldTypeToTagTypeMap[fieldType] ?? "text";
+};
+
+export const setParameterTypesFromFieldSettings = (
+ settings: ActionFormSettings,
+ parameters: ParameterObject[],
+): ParameterObject[] => {
+ const fields = settings.fields;
+ return parameters.map(parameter => {
+ const field = fields[parameter.id];
+ return {
+ ...parameter,
+ type: field
+ ? getParameterTypeFromFieldSettings(field.fieldType, field.inputType)
+ : "string/=",
+ };
+ });
+};
+
+export const setTemplateTagTypesFromFieldSettings = (
+ settings: ActionFormSettings,
+ question: Question,
+): Question => {
+ const fields = settings.fields;
+
+ (question.query() as NativeQuery)
+ .templateTagsWithoutSnippets()
+ .forEach((tag: TemplateTag) => {
+ question = question.setQuery(
+ (question.query() as NativeQuery).setTemplateTag(tag.name, {
+ ...tag,
+ type: getTagTypeFromFieldSettings(fields[tag.id].fieldType),
+ }),
+ );
+ });
+
+ return question;
+};
diff --git a/frontend/src/metabase/entities/actions/utils.unit.spec.ts b/frontend/src/metabase/entities/actions/utils.unit.spec.ts
index 3204219160c..b9abc923da1 100644
--- a/frontend/src/metabase/entities/actions/utils.unit.spec.ts
+++ b/frontend/src/metabase/entities/actions/utils.unit.spec.ts
@@ -1,52 +1,217 @@
-import { removeOrphanSettings } from "./utils";
-
import {
getDefaultFormSettings,
getDefaultFieldSettings,
} from "metabase/writeback/components/ActionCreator/FormCreator/utils";
+import Question from "metabase-lib/lib/Question";
+import { metadata } from "__support__/sample_database_fixture";
+
import type { Parameter as ParameterObject } from "metabase-types/types/Parameter";
+import {
+ removeOrphanSettings,
+ setParameterTypesFromFieldSettings,
+ setTemplateTagTypesFromFieldSettings,
+} from "./utils";
+
+import { NativeDatasetQuery } from "metabase-types/types/Card";
+
+const creatQuestionWithTemplateTags = (tagType: string) =>
+ new Question(
+ {
+ dataset_query: {
+ type: "native",
+ database: null,
+ native: {
+ query:
+ "INSERT INTO products (name, price) VALUES ({{name}}, {{price}});",
+ "template-tags": {
+ name: {
+ id: "aaa",
+ name: "name",
+ type: tagType,
+ },
+ price: {
+ id: "bbb",
+ name: "price",
+ type: tagType,
+ },
+ },
+ },
+ },
+ },
+ metadata,
+ );
+
describe("entities > actions > utils", () => {
- it("should remove orphan settings", () => {
- const formSettings = getDefaultFormSettings();
- formSettings.name = "test form";
- formSettings.fields.aaa = getDefaultFieldSettings();
- formSettings.fields.bbb = getDefaultFieldSettings();
- formSettings.fields.ccc = getDefaultFieldSettings();
-
- const parameters = [
- { id: "aaa", name: "foo" },
- { id: "ccc", name: "bar" },
- ] as ParameterObject[];
-
- const result = removeOrphanSettings(formSettings, parameters);
-
- expect(result.name).toEqual("test form");
- expect(result.fields).toHaveProperty("aaa");
- expect(result.fields).toHaveProperty("ccc");
- expect(result.fields).not.toHaveProperty("bbb");
+ describe("removeOrphanSettings", () => {
+ it("should remove orphan settings", () => {
+ const formSettings = getDefaultFormSettings({
+ name: "test form",
+ fields: {
+ aaa: getDefaultFieldSettings(),
+ bbb: getDefaultFieldSettings(),
+ ccc: getDefaultFieldSettings(),
+ },
+ });
+
+ const parameters = [
+ { id: "aaa", name: "foo" },
+ { id: "ccc", name: "bar" },
+ ] as ParameterObject[];
+
+ const result = removeOrphanSettings(formSettings, parameters);
+
+ expect(result.name).toEqual("test form");
+ expect(result.fields).toHaveProperty("aaa");
+ expect(result.fields).toHaveProperty("ccc");
+ expect(result.fields).not.toHaveProperty("bbb");
+ });
+
+ it("should leave non-orphan settings intact", () => {
+ const formSettings = getDefaultFormSettings({
+ name: "test form",
+ fields: {
+ aaa: getDefaultFieldSettings(),
+ bbb: getDefaultFieldSettings(),
+ ccc: getDefaultFieldSettings(),
+ },
+ });
+
+ const parameters = [
+ { id: "aaa", name: "foo" },
+ { id: "bbb", name: "foo" },
+ { id: "ccc", name: "bar" },
+ ] as ParameterObject[];
+
+ const result = removeOrphanSettings(formSettings, parameters);
+
+ expect(result.name).toEqual("test form");
+ expect(result.fields).toHaveProperty("aaa");
+ expect(result.fields).toHaveProperty("bbb");
+ expect(result.fields).toHaveProperty("ccc");
+ });
});
- it("should leave non-orphan settings intact", () => {
- const formSettings = getDefaultFormSettings();
- formSettings.name = "test form";
+ describe("setParameterTypesFromFieldSettings", () => {
+ it("should set string parameter types", () => {
+ const formSettings = getDefaultFormSettings({
+ name: "test form",
+ fields: {
+ aaa: getDefaultFieldSettings({ fieldType: "string" }),
+ bbb: getDefaultFieldSettings({ fieldType: "string" }),
+ ccc: getDefaultFieldSettings({ fieldType: "string" }),
+ },
+ });
+
+ const parameters = [
+ { id: "aaa", name: "foo", type: "number/=" },
+ { id: "bbb", name: "foo", type: "number/=" },
+ { id: "ccc", name: "bar", type: "number/=" },
+ ] as ParameterObject[];
+
+ const newParams = setParameterTypesFromFieldSettings(
+ formSettings,
+ parameters,
+ );
+
+ newParams.forEach(param => expect(param.type).toEqual("string/="));
+ });
+
+ it("should set number parameter types", () => {
+ const formSettings = getDefaultFormSettings({
+ name: "test form",
+ fields: {
+ aaa: getDefaultFieldSettings({ fieldType: "number" }),
+ bbb: getDefaultFieldSettings({ fieldType: "number" }),
+ ccc: getDefaultFieldSettings({ fieldType: "number" }),
+ },
+ });
+
+ const parameters = [
+ { id: "aaa", name: "foo", type: "string/=" },
+ { id: "bbb", name: "foo", type: "string/=" },
+ { id: "ccc", name: "bar", type: "string/=" },
+ ] as ParameterObject[];
+
+ const newParams = setParameterTypesFromFieldSettings(
+ formSettings,
+ parameters,
+ );
+
+ newParams.forEach(param => expect(param.type).toEqual("number/="));
+ });
+
+ it("should set date parameter types", () => {
+ const formSettings = getDefaultFormSettings({
+ name: "test form",
+ fields: {
+ aaa: getDefaultFieldSettings({ fieldType: "date" }),
+ bbb: getDefaultFieldSettings({ fieldType: "date" }),
+ ccc: getDefaultFieldSettings({ fieldType: "date" }),
+ },
+ });
+
+ const parameters = [
+ { id: "aaa", name: "foo", type: "string/=" },
+ { id: "bbb", name: "foo", type: "string/=" },
+ { id: "ccc", name: "bar", type: "string/=" },
+ ] as ParameterObject[];
+
+ const newParams = setParameterTypesFromFieldSettings(
+ formSettings,
+ parameters,
+ );
+
+ newParams.forEach(param => expect(param.type).toEqual("date/single"));
+ });
+ });
+
+ describe("setTemplateTagTypesFromFieldSettings", () => {
+ it("should set text and number template tag types", () => {
+ const question = creatQuestionWithTemplateTags("date");
+
+ const formSettings = getDefaultFormSettings({
+ name: "test form",
+ fields: {
+ aaa: getDefaultFieldSettings({ fieldType: "string" }),
+ bbb: getDefaultFieldSettings({ fieldType: "number" }),
+ },
+ });
+
+ const newQuestion = setTemplateTagTypesFromFieldSettings(
+ formSettings,
+ question,
+ );
+
+ const tags = (newQuestion.card().dataset_query as NativeDatasetQuery)
+ .native["template-tags"];
+
+ expect(tags.name.type).toEqual("text");
+ expect(tags.price.type).toEqual("number");
+ });
+
+ it("should set date template tag types", () => {
+ const question = creatQuestionWithTemplateTags("number");
- formSettings.fields.aaa = getDefaultFieldSettings();
- formSettings.fields.bbb = getDefaultFieldSettings();
- formSettings.fields.ccc = getDefaultFieldSettings();
+ const formSettings = getDefaultFormSettings({
+ name: "test form",
+ fields: {
+ aaa: getDefaultFieldSettings({ fieldType: "date" }),
+ bbb: getDefaultFieldSettings({ fieldType: "date" }),
+ },
+ });
- const parameters = [
- { id: "aaa", name: "foo" },
- { id: "bbb", name: "foo" },
- { id: "ccc", name: "bar" },
- ] as ParameterObject[];
+ const newQuestion = setTemplateTagTypesFromFieldSettings(
+ formSettings,
+ question,
+ );
- const result = removeOrphanSettings(formSettings, parameters);
+ const tags = (newQuestion.card().dataset_query as NativeDatasetQuery)
+ .native["template-tags"];
- expect(result.name).toEqual("test form");
- expect(result.fields).toHaveProperty("aaa");
- expect(result.fields).toHaveProperty("bbb");
- expect(result.fields).toHaveProperty("ccc");
+ expect(tags.name.type).toEqual("date");
+ expect(tags.price.type).toEqual("date");
+ });
});
});
diff --git a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/FieldSettingsPopover.tsx b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/FieldSettingsPopover.tsx
index c9d954ea3c5..fd7bde1d163 100644
--- a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/FieldSettingsPopover.tsx
+++ b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/FieldSettingsPopover.tsx
@@ -107,7 +107,7 @@ function InputTypeSelect({
<Radio
vertical
value={value}
- options={inputTypes[fieldType ?? "text"]}
+ options={inputTypes[fieldType ?? "string"]}
onChange={onChange}
/>
);
diff --git a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/constants.ts b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/constants.ts
index 384a7ed5045..4930527502a 100644
--- a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/constants.ts
+++ b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/constants.ts
@@ -8,7 +8,7 @@ interface FieldOptionType {
export const getFieldTypes = (): FieldOptionType[] => [
{
- value: "text",
+ value: "string",
name: t`text`,
},
{
@@ -31,7 +31,7 @@ interface InputOptionType {
}
interface InputOptionsMap {
- text: InputOptionType[];
+ string: InputOptionType[];
number: InputOptionType[];
date: InputOptionType[];
category: InputOptionType[];
@@ -60,7 +60,7 @@ const getSelectInputs = (): InputOptionType[] => [
];
export const getInputTypes = (): InputOptionsMap => ({
- text: [...getTextInputs(), ...getSelectInputs()],
+ string: [...getTextInputs(), ...getSelectInputs()],
number: [
{
value: "number",
diff --git a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts
index b8c9f5e929b..e34d488fb0d 100644
--- a/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts
+++ b/frontend/src/metabase/writeback/components/ActionCreator/FormCreator/utils.ts
@@ -1,21 +1,27 @@
import type { ActionFormSettings, FieldSettings } from "metabase-types/api";
-export const getDefaultFormSettings = (): ActionFormSettings => ({
+export const getDefaultFormSettings = (
+ overrides: Partial<ActionFormSettings> = {},
+): ActionFormSettings => ({
name: "",
type: "inline",
description: "",
fields: {},
confirmMessage: "",
+ ...overrides,
});
-export const getDefaultFieldSettings = (): FieldSettings => ({
+export const getDefaultFieldSettings = (
+ overrides: Partial<FieldSettings> = {},
+): FieldSettings => ({
name: "",
order: 0,
description: "",
placeholder: "",
- fieldType: "text",
+ fieldType: "string",
inputType: "string",
required: false,
hidden: false,
width: "medium",
+ ...overrides,
});
--
GitLab