From abc81162ec84d5607b8a14d56ccc081bd50066d9 Mon Sep 17 00:00:00 2001 From: Anton Kulyk <kuliks.anton@gmail.com> Date: Tue, 21 Mar 2023 12:45:30 +0000 Subject: [PATCH] Consolidate action form logic (#29326) * Make `ActionForm` accept an `action` prop * Make field settings inside `ActionForm` if missing * Clean date-time values inside `ActionForm` * WIP * Clean and filter values on submit in `ActionForm` * Sort `ActionVizForm` props * Sort `Action` props * Use more generic `WritebackAction` type * Move more logic into the `ActionForm` * Add `useActionForm` hook * Fix parameter type in tests * Fix submitting a form when all parameters mapped * Move utils to `useActionForm` * Fix submit button label in public action e2e tests * Filter changed values only for implicit update actions * Rename `cleanInitialValues` to `cleanedInitialValues` * Fix `shouldShowConfirmation` can be a string * Nice boring code * Fix types in tests * Small rename * Move `useActionForm` utils closer to source (#29328) * Fix `ActionForm` ignored submit errors * Fix default values display in the form editor * Fix names --- .../components/ActionForm/ActionForm.tsx | 83 ++-- .../ActionForm/ActionForm.unit.spec.tsx | 110 +++-- .../actions/components/ActionViz/Action.tsx | 51 +- .../components/ActionViz/Action.unit.spec.tsx | 2 +- .../components/ActionViz/ActionVizForm.tsx | 59 +-- .../actions/components/ActionViz/utils.ts | 13 +- .../ActionCreator/FormCreator/FormCreator.tsx | 16 +- .../FormCreator/FormCreator.unit.spec.tsx | 24 + .../ActionParametersInputForm.tsx | 100 ++-- .../ActionParametersInputForm.unit.spec.tsx | 78 ++-- .../ActionParametersInputModal.tsx | 13 +- .../ActionParametersInputForm/utils.ts | 58 --- .../utils.unit.spec.ts | 110 ----- .../actions/hooks/use-action-form/index.ts | 1 + .../hooks/use-action-form/use-action-form.ts | 76 +++ .../actions/hooks/use-action-form/utils.ts | 157 +++++++ .../hooks/use-action-form/utils.unit.spec.ts | 441 ++++++++++++++++++ frontend/src/metabase/actions/utils.ts | 103 ---- .../src/metabase/actions/utils.unit.spec.ts | 361 -------------- .../containers/PublicAction/PublicAction.tsx | 31 +- 20 files changed, 1004 insertions(+), 883 deletions(-) delete mode 100644 frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts delete mode 100644 frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.unit.spec.ts create mode 100644 frontend/src/metabase/actions/hooks/use-action-form/index.ts create mode 100644 frontend/src/metabase/actions/hooks/use-action-form/use-action-form.ts create mode 100644 frontend/src/metabase/actions/hooks/use-action-form/utils.ts create mode 100644 frontend/src/metabase/actions/hooks/use-action-form/utils.unit.spec.ts diff --git a/frontend/src/metabase/actions/components/ActionForm/ActionForm.tsx b/frontend/src/metabase/actions/components/ActionForm/ActionForm.tsx index 89c22fd2757..f2e44160091 100644 --- a/frontend/src/metabase/actions/components/ActionForm/ActionForm.tsx +++ b/frontend/src/metabase/actions/components/ActionForm/ActionForm.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from "react"; +import React, { useCallback, useMemo } from "react"; import { t } from "ttag"; import type { FormikHelpers } from "formik"; @@ -9,68 +9,84 @@ import FormProvider from "metabase/core/components/FormProvider"; import FormSubmitButton from "metabase/core/components/FormSubmitButton"; import FormErrorMessage from "metabase/core/components/FormErrorMessage"; -import { getForm, getFormValidationSchema } from "metabase/actions/utils"; +import useActionForm from "metabase/actions/hooks/use-action-form"; +import { + getSubmitButtonColor, + getSubmitButtonLabel, +} from "metabase/actions/utils"; import type { ActionFormInitialValues, - ActionFormSettings, - WritebackParameter, - Parameter, + ParameterId, ParametersForActionExecution, + WritebackAction, } from "metabase-types/api"; import ActionFormFieldWidget from "../ActionFormFieldWidget"; + import { ActionFormButtonContainer } from "./ActionForm.styled"; interface ActionFormProps { - parameters: WritebackParameter[] | Parameter[]; + action: WritebackAction; initialValues?: ActionFormInitialValues; - formSettings?: ActionFormSettings; - submitTitle?: string; - submitButtonColor?: string; + + // Parameters that shouldn't be displayed in the form + // Can be used to "lock" certain parameter values. + // E.g. when a value is coming from a dashboard filter. + // Hidden field values should still be included in initialValues, + // and they will be submitted together in batch. + hiddenFields?: ParameterId[]; + onSubmit: ( - params: ParametersForActionExecution, + parameters: ParametersForActionExecution, actions: FormikHelpers<ParametersForActionExecution>, ) => void; onClose?: () => void; } function ActionForm({ - parameters, - initialValues = {}, - formSettings, - submitTitle, - submitButtonColor = "primary", + action, + initialValues: rawInitialValues = {}, + hiddenFields = [], onSubmit, onClose, }: ActionFormProps): JSX.Element { - // allow us to change the color of the submit button - const submitButtonVariant = { [submitButtonColor]: true }; + const { initialValues, form, validationSchema, getCleanValues } = + useActionForm({ + action, + initialValues: rawInitialValues, + }); - const form = useMemo( - () => getForm(parameters, formSettings?.fields), - [parameters, formSettings?.fields], + const editableFields = useMemo( + () => form.fields.filter(field => !hiddenFields.includes(field.name)), + [form, hiddenFields], ); - const formValidationSchema = useMemo( - () => getFormValidationSchema(parameters, formSettings?.fields), - [parameters, formSettings?.fields], - ); + const submitButtonProps = useMemo(() => { + const variant = getSubmitButtonColor(action); + return { + title: getSubmitButtonLabel(action), + [variant]: true, + }; + }, [action]); - const formInitialValues = useMemo( - () => formValidationSchema.cast(initialValues), - [initialValues, formValidationSchema], + const handleSubmit = useCallback( + ( + values: ParametersForActionExecution, + actions: FormikHelpers<ParametersForActionExecution>, + ) => onSubmit(getCleanValues(values), actions), + [getCleanValues, onSubmit], ); return ( <FormProvider - initialValues={formInitialValues} - validationSchema={formValidationSchema} - onSubmit={onSubmit} + initialValues={initialValues} + validationSchema={validationSchema} + onSubmit={handleSubmit} enableReinitialize > <Form role="form" data-testid="action-form"> - {form.fields.map(field => ( + {editableFields.map(field => ( <ActionFormFieldWidget key={field.name} formField={field} /> ))} @@ -78,10 +94,7 @@ function ActionForm({ {onClose && ( <Button type="button" onClick={onClose}>{t`Cancel`}</Button> )} - <FormSubmitButton - title={submitTitle ?? t`Submit`} - {...submitButtonVariant} - /> + <FormSubmitButton {...submitButtonProps} /> </ActionFormButtonContainer> <FormErrorMessage /> diff --git a/frontend/src/metabase/actions/components/ActionForm/ActionForm.unit.spec.tsx b/frontend/src/metabase/actions/components/ActionForm/ActionForm.unit.spec.tsx index c76161eead0..84ebda01aec 100644 --- a/frontend/src/metabase/actions/components/ActionForm/ActionForm.unit.spec.tsx +++ b/frontend/src/metabase/actions/components/ActionForm/ActionForm.unit.spec.tsx @@ -8,7 +8,10 @@ import type { ParametersForActionExecution, WritebackParameter, } from "metabase-types/api"; -import { createMockActionParameter } from "metabase-types/api/mocks"; +import { + createMockActionParameter, + createMockQueryAction, +} from "metabase-types/api/mocks"; import ActionForm from "./ActionForm"; @@ -43,22 +46,29 @@ type SetupOpts = { initialValues?: ParametersForActionExecution; parameters: WritebackParameter[]; formSettings: ActionFormSettings; + onSubmit?: () => Promise<void>; }; -const setup = ({ initialValues, parameters, formSettings }: SetupOpts) => { - const onSubmit = jest.fn(); +const setup = ({ + initialValues, + parameters, + formSettings, + onSubmit = jest.fn(), +}: SetupOpts) => { + const action = createMockQueryAction({ + parameters, + visualization_settings: formSettings, + }); render( <ActionForm + action={action} initialValues={initialValues} - parameters={parameters} - formSettings={formSettings} - submitTitle="Save" onSubmit={onSubmit} />, ); - return { onSubmit }; + return { action, onSubmit }; }; describe("Actions > ActionForm", () => { @@ -218,7 +228,7 @@ describe("Actions > ActionForm", () => { }); it("can submit form field values", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -242,7 +252,7 @@ describe("Actions > ActionForm", () => { userEvent.type(screen.getByLabelText(/text input/i), "Murloc"); userEvent.type(screen.getByLabelText(/number input/i), "12345"); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => { expect(onSubmit).toHaveBeenCalledWith( @@ -254,11 +264,47 @@ describe("Actions > ActionForm", () => { ); }); }); + + it("shows an error if submit fails", async () => { + const message = "Something went wrong when submitting the form."; + const error = { success: false, error: message, message }; + const { action } = await setup({ + onSubmit: jest.fn().mockRejectedValue(error), + parameters: [ + makeParameter({ id: "abc-123" }), + makeParameter({ id: "def-456" }), + ], + formSettings: { + type: "form", + fields: { + "abc-123": makeFieldSettings({ + inputType: "string", + id: "abc-123", + title: "text input", + }), + "def-456": makeFieldSettings({ + inputType: "number", + id: "def-456", + title: "number input", + }), + }, + }, + }); + + userEvent.type(screen.getByLabelText(/text input/i), "Murloc"); + userEvent.type(screen.getByLabelText(/number input/i), "12345"); + userEvent.click(screen.getByRole("button", { name: action.name })); + + expect(await screen.findByText(message)).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: action.name }), + ).toHaveTextContent("Failed"); + }); }); describe("Form Validation", () => { it("allows form submission when required fields are provided", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -284,14 +330,14 @@ describe("Actions > ActionForm", () => { userEvent.type(await screen.findByLabelText(/foo input/i), "baz"); userEvent.type(await screen.findByLabelText(/bar input/i), "baz"); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => expect(onSubmit).toHaveBeenCalled()); expect(screen.queryByText(/required/i)).not.toBeInTheDocument(); }); it("disables form submission when required fields are not provided", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -318,17 +364,19 @@ describe("Actions > ActionForm", () => { userEvent.click(await screen.findByLabelText(/foo input/i)); // leave empty userEvent.type(await screen.findByLabelText(/bar input/i), "baz"); await waitFor(() => - expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(), + expect( + screen.getByRole("button", { name: action.name }), + ).toBeDisabled(), ); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); expect(await screen.findByText(/required/i)).toBeInTheDocument(); expect(onSubmit).not.toHaveBeenCalled(); }); it("allows form submission when all required fields are set", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -352,21 +400,21 @@ describe("Actions > ActionForm", () => { }, }); - expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + expect(screen.getByRole("button", { name: action.name })).toBeDisabled(); userEvent.type(screen.getByLabelText(/foo input/i), "baz"); await waitFor(() => { - expect(screen.getByRole("button", { name: "Save" })).toBeEnabled(); + expect(screen.getByRole("button", { name: action.name })).toBeEnabled(); }); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => { expect(onSubmit).toHaveBeenCalled(); }); }); it("allows form submission when all fields are optional", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -390,9 +438,9 @@ describe("Actions > ActionForm", () => { }, }); - expect(screen.getByRole("button", { name: "Save" })).toBeEnabled(); + expect(screen.getByRole("button", { name: action.name })).toBeEnabled(); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => expect(onSubmit).toHaveBeenCalled()); }); @@ -415,7 +463,7 @@ describe("Actions > ActionForm", () => { }); it("allows submission of a null non-required boolean field", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -441,14 +489,14 @@ describe("Actions > ActionForm", () => { userEvent.type(await screen.findByLabelText(/foo input/i), "baz"); userEvent.type(await screen.findByLabelText(/bar input/i), "baz"); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => expect(onSubmit).toHaveBeenCalled()); expect(screen.queryByText(/required/i)).not.toBeInTheDocument(); }); it("sets a default value for an empty field", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -474,14 +522,14 @@ describe("Actions > ActionForm", () => { }, }); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => expect(onSubmit).toHaveBeenCalled()); expect(screen.queryByText(/required/i)).not.toBeInTheDocument(); }); it("sets types on form submissions correctly", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ parameters: [ makeParameter({ id: "abc-123" }), makeParameter({ id: "def-456" }), @@ -515,7 +563,7 @@ describe("Actions > ActionForm", () => { userEvent.type(await screen.findByLabelText(/foo input/i), "1"); userEvent.type(await screen.findByLabelText(/bar input/i), "1"); userEvent.type(await screen.findByLabelText(/baz input/i), "1"); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => { expect(onSubmit).toHaveBeenCalledWith( @@ -535,7 +583,7 @@ describe("Actions > ActionForm", () => { const inputTypes = ["string", "number", "text", "date", "datetime", "time"]; inputTypes.forEach(inputType => { it(`casts empty optional ${inputType} field to null`, async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ initialValues: { "abc-123": 1 }, parameters: [makeParameter({ id: "abc-123" })], formSettings: { @@ -555,7 +603,7 @@ describe("Actions > ActionForm", () => { fireEvent.change(screen.getByLabelText(/input/i), { target: { value: "" }, }); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => { expect(onSubmit).toHaveBeenCalledWith( @@ -571,7 +619,7 @@ describe("Actions > ActionForm", () => { // bug repro: https://github.com/metabase/metabase/issues/27377 // eslint-disable-next-line jest/no-disabled-tests it.skip("casts empty optional category fields to null", async () => { - const { onSubmit } = setup({ + const { action, onSubmit } = setup({ initialValues: { "abc-123": "aaa" }, parameters: [makeParameter({ id: "abc-123" })], formSettings: { @@ -588,7 +636,7 @@ describe("Actions > ActionForm", () => { }); userEvent.clear(screen.getByLabelText(/input/i)); - userEvent.click(screen.getByRole("button", { name: "Save" })); + userEvent.click(screen.getByRole("button", { name: action.name })); await waitFor(() => { expect(onSubmit).toHaveBeenCalledWith( diff --git a/frontend/src/metabase/actions/components/ActionViz/Action.tsx b/frontend/src/metabase/actions/components/ActionViz/Action.tsx index adf1cf0bfa5..a41c092e772 100644 --- a/frontend/src/metabase/actions/components/ActionViz/Action.tsx +++ b/frontend/src/metabase/actions/components/ActionViz/Action.tsx @@ -8,18 +8,14 @@ import { getResponseErrorMessage } from "metabase/core/utils/errors"; import Databases from "metabase/entities/databases"; -import { - generateFieldSettingsFromParameters, - setNumericValues, -} from "metabase/actions/utils"; import { executeRowAction } from "metabase/dashboard/actions"; import { getEditingDashcardId } from "metabase/dashboard/selectors"; import type { ActionDashboardCard, - ParametersForActionExecution, - WritebackQueryAction, Dashboard, + ParametersForActionExecution, + WritebackAction, } from "metabase-types/api"; import type { VisualizationProps } from "metabase-types/types/Visualization"; import type { ParameterValueOrArray } from "metabase-types/types/Parameter"; @@ -30,6 +26,7 @@ import type Database from "metabase-lib/metadata/Database"; import { getDashcardParamValues, getNotProvidedActionParameters, + getMappedActionParameters, shouldShowConfirmation, } from "./utils"; @@ -80,6 +77,16 @@ function ActionComponent({ ); }, [dashcard, dashcardParamValues]); + const mappedParameters = useMemo(() => { + if (!dashcard.action) { + return []; + } + return getMappedActionParameters( + dashcard.action, + dashcardParamValues ?? [], + ); + }, [dashcard, dashcardParamValues]); + const shouldConfirm = shouldShowConfirmation(dashcard?.action); const shouldDisplayButton = !!( @@ -89,40 +96,30 @@ function ActionComponent({ ); const onSubmit = useCallback( - (parameterMap: ParametersForActionExecution) => { - const action = dashcard.action; - const fieldSettings = - action?.visualization_settings?.fields || - generateFieldSettingsFromParameters(action?.parameters ?? []); - - const params = setNumericValues( - { ...dashcardParamValues, ...parameterMap }, - fieldSettings, - ); - - return executeRowAction({ + (parameters: ParametersForActionExecution) => + executeRowAction({ dashboard, dashcard, - parameters: params, + parameters, dispatch, shouldToast: shouldDisplayButton, - }); - }, - [dashboard, dashcard, dashcardParamValues, dispatch, shouldDisplayButton], + }), + [dashboard, dashcard, dispatch, shouldDisplayButton], ); return ( <ActionVizForm - onSubmit={onSubmit} - dashcard={dashcard} + action={dashcard.action as WritebackAction} dashboard={dashboard} - settings={settings} - isSettings={isSettings} + dashcard={dashcard} missingParameters={missingParameters} + mappedParameters={mappedParameters} dashcardParamValues={dashcardParamValues} - action={dashcard.action as WritebackQueryAction} + settings={settings} + isSettings={isSettings} shouldDisplayButton={shouldDisplayButton} isEditingDashcard={isEditingDashcard} + onSubmit={onSubmit} /> ); } 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 cf916c6a177..be0613a20c3 100644 --- a/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx +++ b/frontend/src/metabase/actions/components/ActionViz/Action.unit.spec.tsx @@ -273,7 +273,7 @@ describe("Actions > ActionViz > Action", () => { it("should render as a button if no parameters are missing", async () => { await setup({ settings: formSettings, - parameterValues: { "dash-param-1": "foo", "dash-param-2": "bar" }, + parameterValues: { "dash-param-1": "foo", "dash-param-2": 2 }, }); expect( diff --git a/frontend/src/metabase/actions/components/ActionViz/ActionVizForm.tsx b/frontend/src/metabase/actions/components/ActionViz/ActionVizForm.tsx index 986b9517fa0..daf6c53ce42 100644 --- a/frontend/src/metabase/actions/components/ActionViz/ActionVizForm.tsx +++ b/frontend/src/metabase/actions/components/ActionViz/ActionVizForm.tsx @@ -1,15 +1,16 @@ import React, { useState } from "react"; +import useActionForm from "metabase/actions/hooks/use-action-form"; import { getFormTitle } from "metabase/actions/utils"; import type { - WritebackQueryAction, - WritebackParameter, - OnSubmitActionForm, ActionDashboardCard, + OnSubmitActionForm, + Dashboard, ParametersForActionExecution, VisualizationSettings, - Dashboard, + WritebackAction, + WritebackParameter, } from "metabase-types/api"; import ActionParametersInputForm, { @@ -21,32 +22,38 @@ import { shouldShowConfirmation } from "./utils"; import { FormWrapper, FormTitle } from "./ActionForm.styled"; interface ActionFormProps { - onSubmit: OnSubmitActionForm; + action: WritebackAction; dashcard: ActionDashboardCard; - settings: VisualizationSettings; - isSettings: boolean; dashboard: Dashboard; - missingParameters: WritebackParameter[]; + missingParameters?: WritebackParameter[]; + mappedParameters?: WritebackParameter[]; dashcardParamValues: ParametersForActionExecution; - action: WritebackQueryAction; + settings: VisualizationSettings; + isSettings: boolean; shouldDisplayButton: boolean; isEditingDashcard: boolean; + onSubmit: OnSubmitActionForm; } function ActionVizForm({ - onSubmit, + action, dashcard, - settings, - isSettings, dashboard, - missingParameters, + settings, + missingParameters = [], + mappedParameters = [], dashcardParamValues, - action, + isSettings, shouldDisplayButton, isEditingDashcard, + onSubmit, }: ActionFormProps) { const [showModal, setShowModal] = useState(false); const title = getFormTitle(action); + const { getCleanValues } = useActionForm({ + action, + initialValues: dashcardParamValues, + }); // only show confirmation if there are no missing parameters const showConfirmMessage = @@ -56,7 +63,7 @@ function ActionVizForm({ if (missingParameters.length > 0 || showConfirmMessage) { setShowModal(true); } else { - onSubmit({}); + onSubmit(getCleanValues()); } }; @@ -72,24 +79,24 @@ function ActionVizForm({ return ( <> <ActionButtonView - onClick={onClick} settings={settings} isFullHeight={!isSettings} focus={isEditingDashcard} + onClick={onClick} /> {showModal && ( <ActionParametersInputModal - onClose={() => setShowModal(false)} - title={title} - onSubmit={onModalSubmit} - showConfirmMessage={!!showConfirmMessage} - confirmMessage={action.visualization_settings?.confirmMessage} + action={action} dashboard={dashboard} dashcard={dashcard} - missingParameters={missingParameters} + mappedParameters={mappedParameters} dashcardParamValues={dashcardParamValues} + title={title} + showConfirmMessage={showConfirmMessage} + confirmMessage={action.visualization_settings?.confirmMessage} + onSubmit={onModalSubmit} + onClose={() => setShowModal(false)} onCancel={() => setShowModal(false)} - action={action} /> )} </> @@ -100,12 +107,12 @@ function ActionVizForm({ <FormWrapper> <FormTitle>{title}</FormTitle> <ActionParametersInputForm - onSubmit={onSubmit} + action={action} dashboard={dashboard} dashcard={dashcard} - missingParameters={missingParameters} + mappedParameters={mappedParameters} dashcardParamValues={dashcardParamValues} - action={action} + onSubmit={onSubmit} /> </FormWrapper> ); diff --git a/frontend/src/metabase/actions/components/ActionViz/utils.ts b/frontend/src/metabase/actions/components/ActionViz/utils.ts index cdd240a754d..1ab0b93f451 100644 --- a/frontend/src/metabase/actions/components/ActionViz/utils.ts +++ b/frontend/src/metabase/actions/components/ActionViz/utils.ts @@ -84,11 +84,22 @@ export function getNotProvidedActionParameters( }); } +export function getMappedActionParameters( + action: WritebackAction, + dashboardParamValues: ParametersForActionExecution, +) { + const parameters = action.parameters ?? []; + return parameters.filter(parameter => { + return isMappedParameter(parameter, dashboardParamValues); + }); +} + export const shouldShowConfirmation = (action?: WritebackAction) => { if (!action) { return false; } - const hasConfirmationMessage = action.visualization_settings?.confirmMessage; + const hasConfirmationMessage = + !!action.visualization_settings?.confirmMessage; const isImplicitDelete = action.type === "implicit" && action.kind === "row/delete"; return hasConfirmationMessage || isImplicitDelete; diff --git a/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.tsx b/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.tsx index 27a30a44928..9cfdcd09b52 100644 --- a/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.tsx +++ b/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.tsx @@ -25,6 +25,7 @@ import type { import { getForm, + getFormValidationSchema, getDefaultFormSettings, sortActionParams, } from "../../../utils"; @@ -40,7 +41,6 @@ import { } from "./FormCreator.styled"; // FormEditor's can't be submitted as it serves as a form preview -const BLANK_INITIAL_VALUES = {}; const ON_SUBMIT_NOOP = _.noop; interface FormCreatorProps { @@ -76,6 +76,18 @@ function FormCreator({ [parameters, formSettings?.fields], ); + // Validation schema here should only be used to get default values + // for a form preview. We don't want error messages on the preview though. + const validationSchema = useMemo( + () => getFormValidationSchema(parameters, formSettings.fields), + [parameters, formSettings], + ); + + const displayValues = useMemo( + () => validationSchema.getDefault(), + [validationSchema], + ); + const sortedParams = useMemo( () => parameters.sort(sortActionParams(formSettings)), [parameters, formSettings], @@ -147,7 +159,7 @@ function FormCreator({ </InfoText> <FormProvider enableReinitialize - initialValues={BLANK_INITIAL_VALUES} + initialValues={displayValues} onSubmit={ON_SUBMIT_NOOP} > <Form role="form" data-testid="action-form-editor"> diff --git a/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.unit.spec.tsx b/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.unit.spec.tsx index 7c7c5e5d8af..23788f2524a 100644 --- a/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.unit.spec.tsx +++ b/frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FormCreator.unit.spec.tsx @@ -189,6 +189,7 @@ describe("actions > containers > ActionCreator > FormCreator", () => { }); }); }); + it("can toggle required state", async () => { const formSettings: ActionFormSettings = { type: "form", @@ -216,4 +217,27 @@ describe("actions > containers > ActionCreator > FormCreator", () => { }); }); }); + + it("displays default values", async () => { + const defaultValue = "foo bar"; + const parameter = makeParameter(); + const fieldSettings = makeFieldSettings({ + inputType: "string", + required: true, + defaultValue, + }); + setup({ + parameters: [parameter], + formSettings: { + type: "form", + fields: { + [parameter.id]: fieldSettings, + }, + }, + }); + + expect(await screen.findByLabelText(fieldSettings.title)).toHaveValue( + defaultValue, + ); + }); }); diff --git a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx index 7ca1f4d967b..1f369ec507d 100644 --- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx +++ b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx @@ -1,16 +1,12 @@ import React, { useCallback, useMemo, useState, useEffect } from "react"; import { t } from "ttag"; +import _ from "underscore"; import EmptyState from "metabase/components/EmptyState"; import { ActionsApi, PublicApi } from "metabase/services"; import ActionForm from "metabase/actions/components/ActionForm"; -import { - generateFieldSettingsFromParameters, - getSubmitButtonColor, - getSubmitButtonLabel, -} from "metabase/actions/utils"; import { getDashboardType } from "metabase/dashboard/utils"; import type { @@ -19,23 +15,18 @@ import type { Dashboard, ActionDashboardCard, ParametersForActionExecution, - ActionFormSettings, WritebackAction, } from "metabase-types/api"; -import type Field from "metabase-lib/metadata/Field"; - -import { getChangedValues, getInitialValues } from "./utils"; export interface ActionParametersInputFormProps { action: WritebackAction; - missingParameters?: WritebackParameter[]; - dashcardParamValues?: ParametersForActionExecution; dashboard?: Dashboard; dashcard?: ActionDashboardCard; - onCancel?: () => void; - submitButtonColor?: string; + mappedParameters?: WritebackParameter[]; + dashcardParamValues?: ParametersForActionExecution; onSubmit: OnSubmitActionForm; onSubmitSuccess?: () => void; + onCancel?: () => void; } const shouldPrefetchValues = (action: WritebackAction) => @@ -43,7 +34,7 @@ const shouldPrefetchValues = (action: WritebackAction) => function ActionParametersInputForm({ action, - missingParameters = action.parameters, + mappedParameters = [], dashcardParamValues = {}, dashboard, dashcard, @@ -51,37 +42,51 @@ function ActionParametersInputForm({ onSubmit, onSubmitSuccess, }: ActionParametersInputFormProps) { - const [prefetchValues, setPrefetchValues] = + const [prefetchedValues, setPrefetchedValues] = useState<ParametersForActionExecution>({}); + const hasPrefetchedValues = Object.keys(prefetchedValues).length > 0; const shouldPrefetch = useMemo( () => shouldPrefetchValues(action) && dashboard && dashcard, [action, dashboard, dashcard], ); - const prefetchEndpoint = - getDashboardType(dashboard?.id) === "public" - ? PublicApi.prefetchValues - : ActionsApi.prefetchValues; + const initialValues = useMemo( + () => ({ + ...prefetchedValues, + ...dashcardParamValues, + }), + [prefetchedValues, dashcardParamValues], + ); + + const hiddenFields = useMemo( + () => mappedParameters.map(parameter => parameter.id), + [mappedParameters], + ); const fetchInitialValues = useCallback(async () => { + const prefetchEndpoint = + getDashboardType(dashboard?.id) === "public" + ? PublicApi.prefetchValues + : ActionsApi.prefetchValues; + const fetchedValues = await prefetchEndpoint({ dashboardId: dashboard?.id, dashcardId: dashcard?.id, parameters: JSON.stringify(dashcardParamValues), - }).catch(() => false); + }).catch(_.noop); if (fetchedValues) { - setPrefetchValues(fetchedValues); + setPrefetchedValues(fetchedValues); } - }, [dashboard?.id, dashcard?.id, dashcardParamValues, prefetchEndpoint]); + }, [dashboard?.id, dashcard?.id, dashcardParamValues]); useEffect(() => { const hasValueFromDashboard = Object.keys(dashcardParamValues).length > 0; const canPrefetch = hasValueFromDashboard && dashboard && dashcard; if (shouldPrefetch) { - setPrefetchValues({}); + setPrefetchedValues({}); canPrefetch && fetchInitialValues(); } }, [ @@ -92,69 +97,32 @@ function ActionParametersInputForm({ fetchInitialValues, ]); - const fieldSettings = useMemo( - () => - action.visualization_settings?.fields ?? - // if there are no field settings, we generate them from the parameters and field metadata - generateFieldSettingsFromParameters( - missingParameters, - dashcard?.card?.result_metadata as unknown as Field[], - ), - [action, missingParameters, dashcard], - ); - - const initialValues = useMemo( - () => getInitialValues(fieldSettings, prefetchValues), - [fieldSettings, prefetchValues], - ); - const handleSubmit = useCallback( - async (params, actions) => { + async (parameters, actions) => { actions.setSubmitting(true); - const paramsWithChangedValues = getChangedValues(params, initialValues); - - const { success, error } = await onSubmit(paramsWithChangedValues); - + const { success, error } = await onSubmit(parameters); if (success) { actions.setErrors({}); onSubmitSuccess?.(); - shouldPrefetch ? fetchInitialValues() : actions.resetForm(); } else { throw new Error(error); } }, - [ - onSubmit, - onSubmitSuccess, - initialValues, - fetchInitialValues, - shouldPrefetch, - ], + [shouldPrefetch, onSubmit, onSubmitSuccess, fetchInitialValues], ); - const hasPrefetchedValues = !!Object.keys(prefetchValues).length; - if (shouldPrefetch && !hasPrefetchedValues) { return <EmptyState message={t`Choose a record to update`} />; } - const submitButtonLabel = getSubmitButtonLabel(action); - - const formSettings: ActionFormSettings = action.visualization_settings ?? { - type: "button", - fields: fieldSettings, - }; - return ( <ActionForm - parameters={missingParameters} - formSettings={formSettings} + action={action} initialValues={initialValues} - onClose={onCancel} + hiddenFields={hiddenFields} onSubmit={handleSubmit} - submitTitle={submitButtonLabel} - submitButtonColor={getSubmitButtonColor(action)} + onClose={onCancel} /> ); } 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 17f6883c3cd..1678acbea4d 100644 --- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx +++ b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.unit.spec.tsx @@ -7,37 +7,46 @@ import { waitFor } from "@testing-library/react"; import { render, screen } from "__support__/ui"; import { + createMockActionDashboardCard, createMockActionParameter, createMockQueryAction, createMockImplicitQueryAction, createMockDashboard, } from "metabase-types/api/mocks"; -import ActionParametersInputForm from "./ActionParametersInputForm"; -import ActionParametersInputModal from "./ActionParametersInputModal"; - -const mockAction = createMockQueryAction(); -const defaultProps = { - missingParameters: [ - createMockActionParameter({ - id: "parameter_1", - type: "type/Text", - }), - createMockActionParameter({ - id: "parameter_2", - type: "type/Text", - }), - ], - dashcardParamValues: {}, +import ActionParametersInputForm, { + ActionParametersInputFormProps, +} from "./ActionParametersInputForm"; +import ActionParametersInputModal, { + ActionParametersInputModalProps, +} from "./ActionParametersInputModal"; + +const parameter1 = createMockActionParameter({ + id: "parameter_1", + type: "type/Text", +}); + +const parameter2 = createMockActionParameter({ + id: "parameter_2", + type: "type/Text", +}); + +const mockAction = createMockQueryAction({ + parameters: [parameter1, parameter2], +}); + +const defaultProps: ActionParametersInputFormProps = { action: mockAction, + mappedParameters: [], dashboard: createMockDashboard({ id: 123 }), - dashcard: createMockDashboard({ id: 456 }), - onSubmit: jest.fn(() => ({ success: true })), + dashcard: createMockActionDashboardCard({ id: 456, action: mockAction }), + dashcardParamValues: {}, onCancel: _.noop, onSubmitSuccess: _.noop, + onSubmit: jest.fn().mockResolvedValue({ success: true }), }; -async function setup(options?: any) { +function setup(options?: Partial<ActionParametersInputModalProps>) { render(<ActionParametersInputForm {...defaultProps} {...options} />); } @@ -75,7 +84,7 @@ describe("Actions > ActionParametersInputForm", () => { }); it("passes form values to submit handler", async () => { - const submitSpy = jest.fn(() => ({ success: true })); + const submitSpy = jest.fn().mockResolvedValue({ success: true }); await setup({ onSubmit: submitSpy, }); @@ -101,17 +110,19 @@ describe("Actions > ActionParametersInputForm", () => { }); it("should generate field types from parameter types", async () => { - const missingParameters = [ - createMockActionParameter({ - id: "parameter_1", - type: "type/Text", - }), - createMockActionParameter({ - id: "parameter_2", - type: "type/Integer", - }), - ]; - await setup({ missingParameters }); + const action = createMockQueryAction({ + parameters: [ + createMockActionParameter({ + id: "parameter_1", + type: "type/Text", + }), + createMockActionParameter({ + id: "parameter_2", + type: "type/Integer", + }), + ], + }); + await setup({ action }); expect(screen.getByPlaceholderText("Parameter 1")).toHaveAttribute( "type", @@ -126,11 +137,15 @@ describe("Actions > ActionParametersInputForm", () => { it("should fetch and load existing values from API for implicit update actions", async () => { setupPrefetch(); + const idParameter = createMockActionParameter({ id: "id" }); + await setup({ action: createMockImplicitQueryAction({ type: "implicit", kind: "row/update", + parameters: [idParameter, parameter1, parameter2], }), + mappedParameters: [idParameter], dashcardParamValues: { id: 888, }, @@ -163,7 +178,6 @@ describe("Actions > ActionParametersInputForm", () => { type: "implicit", kind: "row/delete", }), - missingParameters: [], showConfirmMessage: true, }); diff --git a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputModal.tsx b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputModal.tsx index 59328aba35f..d0f7eea0453 100644 --- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputModal.tsx +++ b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputModal.tsx @@ -8,19 +8,22 @@ import ActionParametersInputForm, { } from "./ActionParametersInputForm"; interface ModalProps { - onClose: () => void; title: string; showConfirmMessage?: boolean; confirmMessage?: string; + onClose: () => void; } -export default function ActionParametersInputModal({ - onClose, +export type ActionParametersInputModalProps = ModalProps & + ActionParametersInputFormProps; + +function ActionParametersInputModal({ title, showConfirmMessage, confirmMessage, + onClose, ...formProps -}: ModalProps & ActionParametersInputFormProps) { +}: ActionParametersInputModalProps) { return ( <Modal onClose={onClose}> <ModalContent title={title} onClose={onClose}> @@ -36,3 +39,5 @@ export default function ActionParametersInputModal({ const ConfirmMessage = ({ message }: { message?: string }) => ( <div>{message ?? t`This action cannot be undone.`}</div> ); + +export default ActionParametersInputModal; diff --git a/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts b/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts deleted file mode 100644 index a215b99acaa..00000000000 --- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts +++ /dev/null @@ -1,58 +0,0 @@ -import moment from "moment-timezone"; -import { isEmpty } from "metabase/lib/validate"; - -import type { - InputSettingType, - ParametersForActionExecution, - FieldSettingsMap, -} from "metabase-types/api"; - -export const getChangedValues = ( - newValues: ParametersForActionExecution, - oldValues: Partial<ParametersForActionExecution>, -) => { - const changedValues = Object.entries(newValues).filter( - ([newKey, newValue]) => { - const oldValue = oldValues[newKey]; - return newValue !== oldValue; - }, - ); - - return Object.fromEntries(changedValues); -}; - -export const formatValue = ( - value: string | number | null, - inputType?: InputSettingType, -) => { - if (!isEmpty(value) && typeof value === "string") { - if (inputType === "date" && moment(value).isValid()) { - return moment(stripTZInfo(value)).format("YYYY-MM-DD"); - } - if (inputType === "datetime" && moment(value).isValid()) { - return moment(stripTZInfo(value)).format("YYYY-MM-DDTHH:mm:ss"); - } - if (inputType === "time") { - return moment(stripTZInfo(`2020-01-10T${value}`)).format("HH:mm:ss"); - } - } - return value; -}; - -// maps initial values, if any, into an initialValues map -export const getInitialValues = ( - fieldSettings: FieldSettingsMap, - prefetchValues: ParametersForActionExecution, -) => { - return Object.fromEntries( - Object.values(fieldSettings).map(field => [ - field.id, - formatValue(prefetchValues[field.id], field.inputType), - ]), - ); -}; - -export function stripTZInfo(dateOrTimeString: string) { - // strip everything after a trailing tz (e.g. +08:00) - return moment(dateOrTimeString.replace(/(\+|-)\d{2}:\d{2}$/, "")).utc(true); -} diff --git a/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.unit.spec.ts b/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.unit.spec.ts deleted file mode 100644 index d1fb9fc0948..00000000000 --- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.unit.spec.ts +++ /dev/null @@ -1,110 +0,0 @@ -import { getChangedValues, formatValue, stripTZInfo } from "./utils"; - -describe("actions > containers > ActionParametersInputForm > utils", () => { - describe("stripTZInfo", () => { - it("should strip timezone info from dateTimes", () => { - const result = stripTZInfo("2020-05-01T03:30:00-02:00"); - expect(result.format()).toEqual("2020-05-01T03:30:00Z"); - }); - - it("should strip timezone info from times", () => { - const result = stripTZInfo("2020-05-01T03:30:00+08:00"); - expect(result.format()).toEqual("2020-05-01T03:30:00Z"); - }); - - it("should not do anything to strings without time info", () => { - const result = stripTZInfo("2020-05-01"); - expect(result.format()).toEqual("2020-05-01T00:00:00Z"); - }); - - it("should not do anything to strings without timezone info", () => { - const result = stripTZInfo("2020-05-01T03:30:00"); - expect(result.format()).toEqual("2020-05-01T03:30:00Z"); - }); - }); - - describe("formatValue", () => { - it("ignores null values", () => { - const result = formatValue(null); - expect(result).toEqual(null); - }); - - it("ignores numeric values", () => { - const result = formatValue(123); - expect(result).toEqual(123); - }); - - it("ignores string values", () => { - const result = formatValue("123"); - expect(result).toEqual("123"); - }); - - const timezones = ["-02:00", "-07:00", "+01:00", "+09:00"]; - - timezones.forEach(offset => { - describe(`with timezone ${offset}`, () => { - it("formats dates", () => { - const result = formatValue(`2020-05-01T00:00:00${offset}`, "date"); - expect(result).toEqual("2020-05-01"); - }); - - it("formats datetimes", () => { - const result = formatValue( - `2020-05-01T05:00:00${offset}`, - "datetime", - ); - expect(result).toEqual("2020-05-01T05:00:00"); - }); - - it("formats times", () => { - const result = formatValue(`05:25:30${offset}`, "time"); - expect(result).toEqual("05:25:30"); - }); - }); - }); - }); - - describe("getChangedValues", () => { - it("should flag changed fields from null to a value", () => { - const oldValues = { - "abc-def": null, - }; - - const newValues = { - "abc-def": "abc", - }; - - const result = getChangedValues(newValues, oldValues); - - expect(result).toEqual(newValues); - }); - - it("should flag changed fields from undefined to a value", () => { - const oldValues = { - "abc-def": undefined, - }; - - const newValues = { - "abc-def": "abc", - }; - - const result = getChangedValues(newValues, oldValues); - - expect(result).toEqual(newValues); - }); - - it("should flag fields changed from null to empty string", () => { - const oldValues = { - "abc-def": null, - }; - - const newValues = { - "abc-def": "", - }; - - const result = getChangedValues(newValues, oldValues); - - expect(result).toEqual({ "abc-def": "" }); - }); - }); -}); diff --git a/frontend/src/metabase/actions/hooks/use-action-form/index.ts b/frontend/src/metabase/actions/hooks/use-action-form/index.ts new file mode 100644 index 00000000000..39e9b02a036 --- /dev/null +++ b/frontend/src/metabase/actions/hooks/use-action-form/index.ts @@ -0,0 +1 @@ +export { default } from "./use-action-form"; diff --git a/frontend/src/metabase/actions/hooks/use-action-form/use-action-form.ts b/frontend/src/metabase/actions/hooks/use-action-form/use-action-form.ts new file mode 100644 index 00000000000..9b365744b56 --- /dev/null +++ b/frontend/src/metabase/actions/hooks/use-action-form/use-action-form.ts @@ -0,0 +1,76 @@ +import { useCallback, useMemo } from "react"; +import _ from "underscore"; + +import { getForm, getFormValidationSchema } from "metabase/actions/utils"; + +import type { + ActionFormInitialValues, + ParametersForActionExecution, + WritebackAction, +} from "metabase-types/api"; + +import { + formatInitialValue, + formatSubmitValues, + getChangedValues, + generateFieldSettingsFromParameters, +} from "./utils"; + +type Opts = { + action: WritebackAction; + initialValues?: ActionFormInitialValues; +}; + +function useActionForm({ action, initialValues = {} }: Opts) { + const fieldSettings = useMemo( + () => + action.visualization_settings?.fields || + generateFieldSettingsFromParameters(action.parameters), + [action], + ); + + const form = useMemo( + () => getForm(action.parameters, fieldSettings), + [action.parameters, fieldSettings], + ); + + const validationSchema = useMemo( + () => getFormValidationSchema(action.parameters, fieldSettings), + [action.parameters, fieldSettings], + ); + + const cleanedInitialValues = useMemo(() => { + const values = validationSchema.cast(initialValues); + return _.mapObject(values, (value, fieldId) => { + const formField = fieldSettings[fieldId]; + return formatInitialValue(value, formField?.inputType); + }); + }, [initialValues, fieldSettings, validationSchema]); + + const getCleanValues = useCallback( + (values: ParametersForActionExecution = {}) => { + const allValues = { ...cleanedInitialValues, ...values }; + const formatted = formatSubmitValues(allValues, fieldSettings); + + const isImplicitUpdate = + action.type === "implicit" && action.kind === "row/update"; + + // For implicit update actions, we sometimes prefetch selected row values, + // and pass them as initial values to prefill the form. + // In that case, we want to return only changed values + return isImplicitUpdate + ? getChangedValues(formatted, initialValues) + : formatted; + }, + [action, initialValues, cleanedInitialValues, fieldSettings], + ); + + return { + form, + validationSchema, + initialValues: cleanedInitialValues, + getCleanValues, + }; +} + +export default useActionForm; diff --git a/frontend/src/metabase/actions/hooks/use-action-form/utils.ts b/frontend/src/metabase/actions/hooks/use-action-form/utils.ts new file mode 100644 index 00000000000..96b8df14d47 --- /dev/null +++ b/frontend/src/metabase/actions/hooks/use-action-form/utils.ts @@ -0,0 +1,157 @@ +import moment from "moment-timezone"; + +import { slugify, humanize } from "metabase/lib/formatting"; +import { isEmpty } from "metabase/lib/validate"; + +import { getDefaultFieldSettings } from "metabase/actions/utils"; + +import type { + FieldSettingsMap, + InputSettingType, + Parameter, + ParameterId, + ParametersForActionExecution, +} from "metabase-types/api"; +import type { FieldSettings as LocalFieldSettings } from "metabase/actions/types"; + +import Field from "metabase-lib/metadata/Field"; +import { TYPE } from "metabase-lib/types/constants"; + +export function stripTZInfo(dateOrTimeString: string) { + // strip everything after a trailing tz (e.g. +08:00) + return moment(dateOrTimeString.replace(/(\+|-)\d{2}:\d{2}$/, "")).utc(true); +} + +export const formatInitialValue = ( + value: string | number | null, + inputType?: InputSettingType, +) => { + if (!isEmpty(value) && typeof value === "string") { + if (inputType === "date" && moment(value).isValid()) { + return moment(stripTZInfo(value)).format("YYYY-MM-DD"); + } + if (inputType === "datetime" && moment(value).isValid()) { + return moment(stripTZInfo(value)).format("YYYY-MM-DDTHH:mm:ss"); + } + if (inputType === "time") { + return moment(stripTZInfo(`2020-01-10T${value}`)).format("HH:mm:ss"); + } + } + return value; +}; + +export const formatSubmitValues = ( + rawValues: ParametersForActionExecution, + fieldSettings: FieldSettingsMap, +) => { + const values: ParametersForActionExecution = {}; + + Object.entries(rawValues).forEach(([fieldId, fieldValue]) => { + values[fieldId] = fieldValue; + + const formField = fieldSettings[fieldId]; + const isNumericField = formField?.fieldType === "number"; + if (isNumericField && !isEmpty(fieldValue)) { + values[fieldId] = Number(fieldValue) ?? null; + } + }); + + return values; +}; + +export const getChangedValues = ( + values: ParametersForActionExecution, + initialValues: Partial<ParametersForActionExecution>, +) => { + const changedValues = Object.entries(values).filter(([key, value]) => { + const initialValue = initialValues[key]; + return value !== initialValue; + }); + return Object.fromEntries(changedValues); +}; + +const isNumericParameter = (param: Parameter): boolean => + /integer|float/gi.test(param.type); + +const getFieldType = (param: Parameter): "number" | "string" => { + return isNumericParameter(param) ? "number" : "string"; +}; + +export const getInputType = (param: Parameter, field?: Field) => { + if (!field) { + return isNumericParameter(param) ? "number" : "string"; + } + + if (field.isFK()) { + return field.isNumeric() ? "number" : "string"; + } + if (field.isNumeric()) { + return "number"; + } + if (field.isBoolean()) { + return "boolean"; + } + if (field.isTime()) { + return "time"; + } + if (field.isDate()) { + return field.isDateWithoutTime() ? "date" : "datetime"; + } + if ( + field.semantic_type === TYPE.Description || + field.semantic_type === TYPE.Comment || + field.base_type === TYPE.Structured + ) { + return "text"; + } + if ( + field.semantic_type === TYPE.Title || + field.semantic_type === TYPE.Email + ) { + return "string"; + } + if (field.isCategory() && field.semantic_type !== TYPE.Name) { + return "string"; + } + return "string"; +}; + +export const generateFieldSettingsFromParameters = ( + params: Parameter[], + fields?: Field[], +) => { + 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; + + fieldSettings[param.id] = getDefaultFieldSettings({ + id: param.id, + name, + title: displayName, + placeholder: displayName, + required: !!param?.required, + order: index, + description: field?.description ?? "", + fieldType: getFieldType(param), + inputType: getInputType(param, field), + }); + }); + return fieldSettings; +}; 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 new file mode 100644 index 00000000000..e1561455864 --- /dev/null +++ b/frontend/src/metabase/actions/hooks/use-action-form/utils.unit.spec.ts @@ -0,0 +1,441 @@ +import Field from "metabase-lib/metadata/Field"; +import { + formatInitialValue, + getInputType, + generateFieldSettingsFromParameters, + stripTZInfo, +} from "./utils"; + +const createParameter = (options?: any) => { + return { + id: "test_parameter", + name: "Test Parameter", + type: "type/Text", + ...options, + }; +}; + +const createField = (options?: any) => { + return new Field({ + name: "test_field", + display_name: "Test Field", + base_type: "type/Text", + semantic_type: "type/Text", + ...options, + }); +}; + +const getFirstEntry = (obj: any): any => { + return Object.entries(obj)[0]; +}; + +describe("actions > containers > ActionParametersInputForm > utils", () => { + describe("stripTZInfo", () => { + it("should strip timezone info from dateTimes", () => { + const result = stripTZInfo("2020-05-01T03:30:00-02:00"); + expect(result.format()).toEqual("2020-05-01T03:30:00Z"); + }); + + it("should strip timezone info from times", () => { + const result = stripTZInfo("2020-05-01T03:30:00+08:00"); + expect(result.format()).toEqual("2020-05-01T03:30:00Z"); + }); + + it("should not do anything to strings without time info", () => { + const result = stripTZInfo("2020-05-01"); + expect(result.format()).toEqual("2020-05-01T00:00:00Z"); + }); + + it("should not do anything to strings without timezone info", () => { + const result = stripTZInfo("2020-05-01T03:30:00"); + expect(result.format()).toEqual("2020-05-01T03:30:00Z"); + }); + }); + + describe("formatInitialValue", () => { + it("ignores null values", () => { + const result = formatInitialValue(null); + expect(result).toEqual(null); + }); + + it("ignores numeric values", () => { + const result = formatInitialValue(123); + expect(result).toEqual(123); + }); + + it("ignores string values", () => { + const result = formatInitialValue("123"); + expect(result).toEqual("123"); + }); + + const timezones = ["-02:00", "-07:00", "+01:00", "+09:00"]; + + timezones.forEach(offset => { + describe(`with timezone ${offset}`, () => { + it("formats dates", () => { + const result = formatInitialValue( + `2020-05-01T00:00:00${offset}`, + "date", + ); + expect(result).toEqual("2020-05-01"); + }); + + it("formats datetimes", () => { + const result = formatInitialValue( + `2020-05-01T05:00:00${offset}`, + "datetime", + ); + expect(result).toEqual("2020-05-01T05:00:00"); + }); + + it("formats times", () => { + const result = formatInitialValue(`05:25:30${offset}`, "time"); + expect(result).toEqual("05:25:30"); + }); + }); + }); + }); + + describe("getInputType", () => { + it('should return "number" for numeric parameters', () => { + const intParam = createParameter({ type: "type/Integer" }); + expect(getInputType(intParam)).toEqual("number"); + + const floatParam = createParameter({ type: "type/Float" }); + expect(getInputType(floatParam)).toEqual("number"); + }); + + it('should return "string" for non-numeric parameters', () => { + const textParam = createParameter({ type: "type/Text" }); + expect(getInputType(textParam)).toEqual("string"); + + const turtleParam = createParameter({ type: "type/Turtle" }); + expect(getInputType(turtleParam)).toEqual("string"); + }); + + it('should return "number" for numeric foreign keys', () => { + const field = createField({ + semantic_type: "type/FK", + base_type: "type/Integer", + }); + expect(getInputType(createParameter(), field)).toEqual("number"); + }); + + it('should return "string" for string foreign keys', () => { + const field = createField({ + semantic_type: "type/FK", + base_type: "type/Text", + }); + expect(getInputType(createParameter(), field)).toEqual("string"); + }); + + it('should return "number" for floating point numbers', () => { + const field = createField({ + base_type: "type/Float", + }); + expect(getInputType(createParameter(), field)).toEqual("number"); + }); + + it('should return "boolean" for booleans', () => { + const field = createField({ + base_type: "type/Boolean", + }); + expect(getInputType(createParameter(), field)).toEqual("boolean"); + }); + + it('should return "date" for dates', () => { + const dateTypes = ["type/Date"]; + const param = createParameter(); + + dateTypes.forEach(type => { + const field = createField({ base_type: type }); + expect(getInputType(param, field)).toEqual("date"); + }); + }); + + it('should return "datetime" for datetimes', () => { + const dateTypes = ["type/DateTime", "type/DateTimeWithLocalTZ"]; + const param = createParameter(); + + dateTypes.forEach(type => { + const field = createField({ base_type: type }); + expect(getInputType(param, field)).toEqual("datetime"); + }); + }); + + it('should return "time" for times', () => { + const dateTypes = ["type/Time", "type/TimeWithLocalTZ"]; + const param = createParameter(); + + dateTypes.forEach(type => { + const field = createField({ base_type: type }); + expect(getInputType(param, field)).toEqual("time"); + }); + }); + + it('should return "string" for categories', () => { + const field = createField({ + semantic_type: "type/Category", + }); + expect(getInputType(createParameter(), field)).toEqual("string"); + }); + + it('should return "text" for description', () => { + const field = createField({ + semantic_type: "type/Description", + }); + expect(getInputType(createParameter(), field)).toEqual("text"); + }); + }); + + describe("generateFieldSettingsFromParameters", () => { + it("should generate settings for a string field", () => { + const fields = [createField({ name: "test-field" })]; + const params = [createParameter({ id: "test-field" })]; + const [_id, settings] = getFirstEntry( + generateFieldSettingsFromParameters(params, fields), + ); + + expect(settings.fieldType).toBe("string"); + expect(settings.inputType).toBe("string"); + }); + + 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), + ); + + expect(settings.fieldType).toBe("number"); + expect(settings.inputType).toBe("number"); + }); + + 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), + ); + + 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" }), + ]; + const params = [createParameter({ id: "test-field", type: "type/Text" })]; + const [_id, settings] = getFirstEntry( + generateFieldSettingsFromParameters(params, fields), + ); + + expect(settings.fieldType).toBe("string"); + expect(settings.inputType).toBe("string"); + }); + + 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" })]; + 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" })]; + const [_id, settings] = getFirstEntry( + generateFieldSettingsFromParameters(params, fields), + ); + + expect(settings.placeholder).toBe("Test Field"); + expect(settings.title).toBe("Test Field"); + }); + + it("matches field names to parameter ids case-insensitively", () => { + const fields = [createField({ name: "TEST-field" })]; + const params = [createParameter({ id: "test-field" })]; + const [id, settings] = getFirstEntry( + generateFieldSettingsFromParameters(params, fields), + ); + + 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 })]; + const [_id, settings] = getFirstEntry( + generateFieldSettingsFromParameters(params, fields), + ); + + expect(settings.placeholder).toBe("Test-field"); + expect(settings.title).toBe("Test-field"); + expect(settings.name).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), + ); + + 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), + ); + + 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/actions/utils.ts b/frontend/src/metabase/actions/utils.ts index df04631c96b..352ead011ca 100644 --- a/frontend/src/metabase/actions/utils.ts +++ b/frontend/src/metabase/actions/utils.ts @@ -2,8 +2,6 @@ import { t } from "ttag"; import * as Yup from "yup"; import * as Errors from "metabase/core/utils/errors"; -import { slugify, humanize } from "metabase/lib/formatting"; -import { isEmpty } from "metabase/lib/validate"; import type { ActionDashboardCard, @@ -17,8 +15,6 @@ import type { InputComponentType, InputSettingType, Parameter, - ParameterId, - ParametersForActionExecution, WritebackAction, WritebackActionBase, WritebackImplicitQueryAction, @@ -149,92 +145,6 @@ export const getDefaultFieldSettings = ( ...overrides, }); -export const generateFieldSettingsFromParameters = ( - params: Parameter[], - fields?: Field[], -) => { - 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; - - fieldSettings[param.id] = getDefaultFieldSettings({ - id: param.id, - name, - title: displayName, - placeholder: displayName, - required: !!param?.required, - order: index, - description: field?.description ?? "", - fieldType: getFieldType(param), - inputType: getInputType(param, field), - }); - }); - return fieldSettings; -}; - -const getFieldType = (param: Parameter): "number" | "string" => { - return isNumericParameter(param) ? "number" : "string"; -}; - -const isNumericParameter = (param: Parameter): boolean => - /integer|float/gi.test(param.type); - -export const getInputType = (param: Parameter, field?: Field) => { - if (!field) { - return isNumericParameter(param) ? "number" : "string"; - } - - if (field.isFK()) { - return field.isNumeric() ? "number" : "string"; - } - if (field.isNumeric()) { - return "number"; - } - if (field.isBoolean()) { - return "boolean"; - } - if (field.isTime()) { - return "time"; - } - if (field.isDate()) { - return field.isDateWithoutTime() ? "date" : "datetime"; - } - if ( - field.semantic_type === TYPE.Description || - field.semantic_type === TYPE.Comment || - field.base_type === TYPE.Structured - ) { - return "text"; - } - if ( - field.semantic_type === TYPE.Title || - field.semantic_type === TYPE.Email - ) { - return "string"; - } - if (field.isCategory() && field.semantic_type !== TYPE.Name) { - return "string"; - } - return "string"; -}; - export function isSavedAction( action?: Partial<WritebackActionBase>, ): action is WritebackAction { @@ -254,19 +164,6 @@ export const getFormTitle = (action: WritebackAction): string => { return action.visualization_settings?.name || action.name || t`Action form`; }; -export function setNumericValues( - params: ParametersForActionExecution, - fieldSettings: FieldSettingsMap, -) { - Object.entries(params).forEach(([key, value]) => { - if (fieldSettings[key]?.fieldType === "number" && !isEmpty(value)) { - params[key] = Number(value) ?? null; - } - }); - - return params; -} - function hasDataFromExplicitAction(result: any) { const isInsert = result["created-row"]; const isUpdate = diff --git a/frontend/src/metabase/actions/utils.unit.spec.ts b/frontend/src/metabase/actions/utils.unit.spec.ts index 687eabaf219..3e4b741b0df 100644 --- a/frontend/src/metabase/actions/utils.unit.spec.ts +++ b/frontend/src/metabase/actions/utils.unit.spec.ts @@ -1,11 +1,7 @@ -import Field from "metabase-lib/metadata/Field"; - import { getDefaultFieldSettings, getDefaultFormSettings, sortActionParams, - generateFieldSettingsFromParameters, - getInputType, } from "./utils"; const createParameter = (options?: any) => { @@ -17,20 +13,6 @@ const createParameter = (options?: any) => { }; }; -const createField = (options?: any) => { - return new Field({ - name: "test_field", - display_name: "Test Field", - base_type: "type/Text", - semantic_type: "type/Text", - ...options, - }); -}; - -const getFirstEntry = (obj: any): any => { - return Object.entries(obj)[0]; -}; - describe("sortActionParams", () => { const formSettings = getDefaultFormSettings({ fields: { @@ -60,347 +42,4 @@ describe("sortActionParams", () => { expect(sortedParams[1].id).toEqual("b"); expect(sortedParams[2].id).toEqual("c"); }); - - describe("generateFieldSettingsFromParameters", () => { - it("should generate settings for a string field", () => { - const fields = [createField({ name: "test-field" })]; - const params = [createParameter({ id: "test-field" })]; - const [_id, settings] = getFirstEntry( - generateFieldSettingsFromParameters(params, fields), - ); - - expect(settings.fieldType).toBe("string"); - expect(settings.inputType).toBe("string"); - }); - - 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), - ); - - expect(settings.fieldType).toBe("number"); - expect(settings.inputType).toBe("number"); - }); - - 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), - ); - - 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" }), - ]; - const params = [createParameter({ id: "test-field", type: "type/Text" })]; - const [_id, settings] = getFirstEntry( - generateFieldSettingsFromParameters(params, fields), - ); - - expect(settings.fieldType).toBe("string"); - expect(settings.inputType).toBe("string"); - }); - - 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" })]; - 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" })]; - const [_id, settings] = getFirstEntry( - generateFieldSettingsFromParameters(params, fields), - ); - - expect(settings.placeholder).toBe("Test Field"); - expect(settings.title).toBe("Test Field"); - }); - - it("matches field names to parameter ids case-insensitively", () => { - const fields = [createField({ name: "TEST-field" })]; - const params = [createParameter({ id: "test-field" })]; - const [id, settings] = getFirstEntry( - generateFieldSettingsFromParameters(params, fields), - ); - - 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 })]; - const [_id, settings] = getFirstEntry( - generateFieldSettingsFromParameters(params, fields), - ); - - expect(settings.placeholder).toBe("Test-field"); - expect(settings.title).toBe("Test-field"); - expect(settings.name).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), - ); - - 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), - ); - - 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"); - }); - }); - }); - - describe("getInputType", () => { - it('should return "number" for numeric parameters', () => { - const intParam = createParameter({ type: "type/Integer" }); - expect(getInputType(intParam)).toEqual("number"); - - const floatParam = createParameter({ type: "type/Float" }); - expect(getInputType(floatParam)).toEqual("number"); - }); - - it('should return "string" for non-numeric parameters', () => { - const textParam = createParameter({ type: "type/Text" }); - expect(getInputType(textParam)).toEqual("string"); - - const turtleParam = createParameter({ type: "type/Turtle" }); - expect(getInputType(turtleParam)).toEqual("string"); - }); - - it('should return "number" for numeric foreign keys', () => { - const field = createField({ - semantic_type: "type/FK", - base_type: "type/Integer", - }); - expect(getInputType(createParameter(), field)).toEqual("number"); - }); - - it('should return "string" for string foreign keys', () => { - const field = createField({ - semantic_type: "type/FK", - base_type: "type/Text", - }); - expect(getInputType(createParameter(), field)).toEqual("string"); - }); - - it('should return "number" for floating point numbers', () => { - const field = createField({ - base_type: "type/Float", - }); - expect(getInputType(createParameter(), field)).toEqual("number"); - }); - - it('should return "boolean" for booleans', () => { - const field = createField({ - base_type: "type/Boolean", - }); - expect(getInputType(createParameter(), field)).toEqual("boolean"); - }); - - it('should return "date" for dates', () => { - const dateTypes = ["type/Date"]; - const param = createParameter(); - - dateTypes.forEach(type => { - const field = createField({ base_type: type }); - expect(getInputType(param, field)).toEqual("date"); - }); - }); - - it('should return "datetime" for datetimes', () => { - const dateTypes = ["type/DateTime", "type/DateTimeWithLocalTZ"]; - const param = createParameter(); - - dateTypes.forEach(type => { - const field = createField({ base_type: type }); - expect(getInputType(param, field)).toEqual("datetime"); - }); - }); - - it('should return "time" for times', () => { - const dateTypes = ["type/Time", "type/TimeWithLocalTZ"]; - const param = createParameter(); - - dateTypes.forEach(type => { - const field = createField({ base_type: type }); - expect(getInputType(param, field)).toEqual("time"); - }); - }); - - it('should return "string" for categories', () => { - const field = createField({ - semantic_type: "type/Category", - }); - expect(getInputType(createParameter(), field)).toEqual("string"); - }); - - it('should return "text" for description', () => { - const field = createField({ - semantic_type: "type/Description", - }); - expect(getInputType(createParameter(), field)).toEqual("text"); - }); - }); }); diff --git a/frontend/src/metabase/public/containers/PublicAction/PublicAction.tsx b/frontend/src/metabase/public/containers/PublicAction/PublicAction.tsx index 16ff9c46c4b..d6783552b60 100644 --- a/frontend/src/metabase/public/containers/PublicAction/PublicAction.tsx +++ b/frontend/src/metabase/public/containers/PublicAction/PublicAction.tsx @@ -1,14 +1,10 @@ -import React, { useCallback, useMemo, useState } from "react"; +import React, { useCallback, useState } from "react"; import title from "metabase/hoc/Title"; import { PublicApi } from "metabase/services"; import ActionForm from "metabase/actions/components/ActionForm"; -import { - generateFieldSettingsFromParameters, - getSuccessMessage, - setNumericValues, -} from "metabase/actions/utils"; +import { getSuccessMessage } from "metabase/actions/utils"; import type { ParametersForActionExecution, @@ -32,28 +28,16 @@ function PublicAction({ action, publicId, onError }: Props) { const [isSubmitted, setSubmitted] = useState(false); const successMessage = getSuccessMessage(action); - const formSettings = useMemo(() => { - const actionSettings = action.visualization_settings || {}; - const fieldSettings = - actionSettings.fields || - generateFieldSettingsFromParameters(action.parameters); - return { - ...actionSettings, - fields: fieldSettings, - }; - }, [action]); - const handleSubmit = useCallback( - async (values: ParametersForActionExecution) => { + async (parameters: ParametersForActionExecution) => { try { - const parameters = setNumericValues(values, formSettings.fields); await PublicApi.executeAction({ uuid: publicId, parameters }); setSubmitted(true); } catch (error) { onError(error as AppErrorDescriptor); } }, - [publicId, formSettings, onError], + [publicId, onError], ); if (isSubmitted) { @@ -63,12 +47,7 @@ function PublicAction({ action, publicId, onError }: Props) { return ( <FormContainer> <FormTitle>{action.name}</FormTitle> - <ActionForm - submitTitle={action.name} - parameters={action.parameters} - formSettings={formSettings} - onSubmit={handleSubmit} - /> + <ActionForm action={action} onSubmit={handleSubmit} /> </FormContainer> ); } -- GitLab