From c4134211ac873d7ab7dff7351a5172643f356790 Mon Sep 17 00:00:00 2001
From: Nick Fitzpatrick <nick@metabase.com>
Date: Wed, 24 Apr 2024 18:22:31 -0300
Subject: [PATCH] Migrate SMTPConnectionForm to Form Provider (#41703)
* Migrate SMTPConnectionForm to Form Provider
* e2e test adjustment
* updaing setting mock
* PR feedback
---
.../src/metabase-types/api/mocks/settings.ts | 5 +
frontend/src/metabase-types/api/settings.ts | 5 +
.../components/Email/SMTPConnectionForm.tsx | 298 ++++++++++++------
.../src/metabase/admin/settings/settings.js | 7 +-
4 files changed, 220 insertions(+), 95 deletions(-)
diff --git a/frontend/src/metabase-types/api/mocks/settings.ts b/frontend/src/metabase-types/api/mocks/settings.ts
index 0709dde5e9d..66555766035 100644
--- a/frontend/src/metabase-types/api/mocks/settings.ts
+++ b/frontend/src/metabase-types/api/mocks/settings.ts
@@ -156,6 +156,11 @@ export const createMockSettings = (
"ee-openai-model": "",
"ee-openai-api-key": "",
"email-configured?": false,
+ "email-smtp-host": null,
+ "email-smtp-port": null,
+ "email-smtp-security": "None",
+ "email-smtp-username": null,
+ "email-smtp-password": null,
"embedding-app-origin": "",
"enable-embedding": false,
"enable-enhancements?": false,
diff --git a/frontend/src/metabase-types/api/settings.ts b/frontend/src/metabase-types/api/settings.ts
index cec2b393be4..a3c47b3c0d2 100644
--- a/frontend/src/metabase-types/api/settings.ts
+++ b/frontend/src/metabase-types/api/settings.ts
@@ -192,6 +192,11 @@ export type HelpLinkSetting = "metabase" | "hidden" | "custom";
interface InstanceSettings {
"admin-email": string;
+ "email-smtp-host": string | null;
+ "email-smtp-port": number | null;
+ "email-smtp-security": "None" | "SSL" | "TLS" | "STARTTLS";
+ "email-smtp-username": string | null;
+ "email-smtp-password": string | null;
"enable-embedding": boolean;
"enable-nested-queries": boolean;
"enable-query-caching"?: boolean;
diff --git a/frontend/src/metabase/admin/settings/components/Email/SMTPConnectionForm.tsx b/frontend/src/metabase/admin/settings/components/Email/SMTPConnectionForm.tsx
index 2f6fd2bb9b4..1156bb50aec 100644
--- a/frontend/src/metabase/admin/settings/components/Email/SMTPConnectionForm.tsx
+++ b/frontend/src/metabase/admin/settings/components/Email/SMTPConnectionForm.tsx
@@ -1,16 +1,27 @@
-import { useCallback, useEffect, useRef, useState } from "react";
+import cx from "classnames";
+import { useCallback, useEffect, useState, useMemo } from "react";
import { push } from "react-router-redux";
import { t } from "ttag";
+import _ from "underscore";
+import * as Yup from "yup";
-import MarginHostingCTA from "metabase/admin/settings/components/widgets/MarginHostingCTA";
import type { SettingElement } from "metabase/admin/settings/types";
-import Button from "metabase/core/components/Button";
+import Breadcrumbs from "metabase/components/Breadcrumbs";
import CS from "metabase/css/core/index.css";
+import {
+ FormProvider,
+ Form,
+ FormTextInput,
+ FormRadioGroup,
+ FormSubmitButton,
+} from "metabase/forms";
import * as MetabaseAnalytics from "metabase/lib/analytics";
+import { color } from "metabase/lib/colors";
+import * as Errors from "metabase/lib/errors";
import { useDispatch, useSelector } from "metabase/lib/redux";
import { getIsPaidPlan } from "metabase/selectors/settings";
import { getIsEmailConfigured, getIsHosted } from "metabase/setup/selectors";
-import { Flex, Stack } from "metabase/ui";
+import { Group, Radio, Stack, Button, Text, Flex } from "metabase/ui";
import type { Settings } from "metabase-types/api";
import {
@@ -18,7 +29,7 @@ import {
updateEmailSettings,
clearEmailSettings,
} from "../../settings";
-import SettingsBatchForm from "../SettingsBatchForm";
+import MarginHostingCTA from "../widgets/MarginHostingCTA";
const BREADCRUMBS = [[t`Email`, "/admin/settings/email"], [t`SMTP`]];
@@ -29,35 +40,59 @@ const SEND_TEST_BUTTON_STATES = {
};
type ButtonStateType = keyof typeof SEND_TEST_BUTTON_STATES;
-interface FormRefType {
- handleFormErrors: (error: Error) => void;
- setFormErrors: (formErrors: any) => void;
- setState: ({ formData, dirty }: { formData: object; dirty: boolean }) => void;
-}
-
interface SMTPConnectionFormProps {
elements: SettingElement[];
settingValues: Settings;
}
+type FormValueProps = Pick<
+ Settings,
+ | "email-smtp-host"
+ | "email-smtp-port"
+ | "email-smtp-security"
+ | "email-smtp-username"
+ | "email-smtp-password"
+>;
+
+const FORM_VALUE_SCHEMA = Yup.object({
+ "email-smtp-host": Yup.string().required(Errors.required).default(""),
+ "email-smtp-port": Yup.number()
+ .positive()
+ .nullable()
+ .required(Errors.required)
+ .default(null),
+ "email-smtp-security": Yup.string(),
+ "email-smtp-username": Yup.string().default(""),
+ "email-smtp-password": Yup.string().default(""),
+});
+
export const SMTPConnectionForm = ({
elements,
settingValues,
}: SMTPConnectionFormProps) => {
const [sendingEmail, setSendingEmail] = useState<ButtonStateType>("default");
+ const [testEmailError, setTestEmailError] = useState<string | null>(null);
- const formRef = useRef<FormRefType>();
const isHosted = useSelector(getIsHosted);
const isPaidPlan = useSelector(getIsPaidPlan);
const isEmailConfigured = useSelector(getIsEmailConfigured);
-
const dispatch = useDispatch();
+ const elementMap = useMemo(() => _.indexBy(elements, "key"), [elements]);
+
+ const initialValues = useMemo<FormValueProps>(
+ () => ({
+ "email-smtp-host": settingValues["email-smtp-host"] || "",
+ "email-smtp-port": settingValues["email-smtp-port"],
+ "email-smtp-security": settingValues["email-smtp-security"] || "none",
+ "email-smtp-username": settingValues["email-smtp-username"] || "",
+ "email-smtp-password": settingValues["email-smtp-password"] || "",
+ }),
+ [settingValues],
+ );
+
const handleClearEmailSettings = useCallback(async () => {
await dispatch(clearEmailSettings());
- // NOTE: reaching into form component is not ideal
-
- formRef.current?.setState({ formData: {}, dirty: false });
}, [dispatch]);
const handleUpdateEmailSettings = useCallback(
@@ -71,40 +106,31 @@ export const SMTPConnectionForm = ({
[dispatch, isEmailConfigured],
);
- const handleSendTestEmail = useCallback(
- async (e: React.MouseEvent) => {
- e.preventDefault();
-
- setSendingEmail("working");
- // NOTE: reaching into form component is not ideal
- formRef.current?.setFormErrors(null);
-
- try {
- await dispatch(sendTestEmail());
- setSendingEmail("success");
- MetabaseAnalytics.trackStructEvent(
- "Email Settings",
- "Test Email",
- "success",
- );
-
- // show a confirmation for 3 seconds, then return to normal
- setTimeout(() => setSendingEmail("default"), 3000);
- } catch (error: any) {
- MetabaseAnalytics.trackStructEvent(
- "Email Settings",
- "Test Email",
- "error",
- );
- setSendingEmail("default");
- // NOTE: reaching into form component is not ideal
- formRef.current?.setFormErrors(
- formRef.current?.handleFormErrors(error),
- );
- }
- },
- [dispatch],
- );
+ const handleSendTestEmail = useCallback(async () => {
+ setSendingEmail("working");
+ setTestEmailError(null);
+
+ try {
+ await dispatch(sendTestEmail());
+ setSendingEmail("success");
+ MetabaseAnalytics.trackStructEvent(
+ "Email Settings",
+ "Test Email",
+ "success",
+ );
+
+ // show a confirmation for 3 seconds, then return to normal
+ setTimeout(() => setSendingEmail("default"), 3000);
+ } catch (error: any) {
+ MetabaseAnalytics.trackStructEvent(
+ "Email Settings",
+ "Test Email",
+ "error",
+ );
+ setSendingEmail("default");
+ setTestEmailError(error?.data?.message);
+ }
+ }, [dispatch]);
useEffect(() => {
if (isHosted) {
@@ -113,50 +139,136 @@ export const SMTPConnectionForm = ({
}, [dispatch, isHosted]);
return (
- <Stack spacing="sm">
- <Flex justify="space-between">
- <SettingsBatchForm
- ref={formRef}
- breadcrumbs={isEmailConfigured ? BREADCRUMBS : null}
- elements={elements}
- settingValues={settingValues}
- updateSettings={handleUpdateEmailSettings}
- renderExtraButtons={({
- disabled,
- valid,
- pristine,
- submitting,
- }: {
- disabled: boolean;
- valid: boolean;
- pristine: boolean;
- submitting: ButtonStateType;
- }) => (
- <>
- {valid && pristine && submitting === "default" ? (
- <Button
- className={CS.mr1}
- success={sendingEmail === "success"}
- disabled={disabled}
- onClick={handleSendTestEmail}
- >
- {SEND_TEST_BUTTON_STATES[sendingEmail]}
- </Button>
- ) : null}
- <Button
- className={CS.mr1}
- disabled={disabled}
- onClick={handleClearEmailSettings}
+ <Flex justify="space-between">
+ <Stack spacing="sm" maw={400} style={{ paddingInlineStart: "0.5rem" }}>
+ {isEmailConfigured && (
+ <Breadcrumbs crumbs={BREADCRUMBS} className={cx(CS.mb3)} />
+ )}
+ <FormProvider
+ initialValues={initialValues}
+ validationSchema={FORM_VALUE_SCHEMA}
+ onSubmit={handleUpdateEmailSettings}
+ enableReinitialize
+ >
+ {({ dirty, isValid, isSubmitting, values }) => (
+ <Form>
+ <FormTextInput
+ name="email-smtp-host"
+ label={elementMap["email-smtp-host"]["display_name"]}
+ description={elementMap["email-smtp-host"]["description"]}
+ placeholder={elementMap["email-smtp-host"]["placeholder"]}
+ mb="1.5rem"
+ labelProps={{
+ tt: "uppercase",
+ mb: "0.5rem",
+ }}
+ descriptionProps={{
+ fz: "0.75rem",
+ mb: "0.5rem",
+ }}
+ />
+ <FormTextInput
+ name="email-smtp-port"
+ label={elementMap["email-smtp-port"]["display_name"]}
+ description={elementMap["email-smtp-port"]["description"]}
+ placeholder={elementMap["email-smtp-port"]["placeholder"]}
+ mb="1.5rem"
+ labelProps={{
+ tt: "uppercase",
+ mb: "0.5rem",
+ }}
+ descriptionProps={{
+ fz: "0.75rem",
+ mb: "0.5rem",
+ }}
+ />
+ <FormRadioGroup
+ name="email-smtp-security"
+ label={elementMap["email-smtp-security"]["display_name"]}
+ description={elementMap["email-smtp-security"]["description"]}
+ mb="1.5rem"
+ labelProps={{
+ tt: "uppercase",
+ fz: "0.875rem",
+ c: "text-medium",
+ mb: "0.5rem",
+ }}
>
- {t`Clear`}
- </Button>
- </>
+ <Group>
+ {Object.entries(
+ elementMap["email-smtp-security"].options || {},
+ ).map(([value, name]) => (
+ <Radio
+ value={value}
+ label={name}
+ key={value}
+ styles={{
+ inner: { display: "none" },
+ label: {
+ paddingLeft: 0,
+ color:
+ values["email-smtp-security"] === value
+ ? color("brand")
+ : color("text-dark"),
+ },
+ }}
+ />
+ ))}
+ </Group>
+ </FormRadioGroup>
+ <FormTextInput
+ name="email-smtp-username"
+ label={elementMap["email-smtp-username"]["display_name"]}
+ description={elementMap["email-smtp-username"]["description"]}
+ placeholder={elementMap["email-smtp-username"]["placeholder"]}
+ mb="1.5rem"
+ labelProps={{
+ tt: "uppercase",
+ mb: "0.5rem",
+ }}
+ />
+ <FormTextInput
+ name="email-smtp-password"
+ type="password"
+ label={elementMap["email-smtp-password"]["display_name"]}
+ description={elementMap["email-smtp-password"]["description"]}
+ placeholder={elementMap["email-smtp-password"]["placeholder"]}
+ mb="1.5rem"
+ labelProps={{
+ tt: "uppercase",
+ mb: "0.5rem",
+ }}
+ />
+ {testEmailError && (
+ <Text
+ role="alert"
+ aria-label={testEmailError}
+ color="error"
+ mb="1rem"
+ >
+ {testEmailError}
+ </Text>
+ )}
+ <Flex mt="1rem" gap="1.5rem">
+ <FormSubmitButton
+ label={t`Save changes`}
+ disabled={!dirty}
+ variant="filled"
+ />
+ {!dirty && isValid && !isSubmitting && (
+ <Button onClick={handleSendTestEmail}>
+ {SEND_TEST_BUTTON_STATES[sendingEmail]}
+ </Button>
+ )}
+ <Button onClick={handleClearEmailSettings}>{t`Clear`}</Button>
+ </Flex>
+ </Form>
)}
- />
- {!isPaidPlan && (
- <MarginHostingCTA tagline={t`Have your email configured for you.`} />
- )}
- </Flex>
- </Stack>
+ </FormProvider>
+ </Stack>
+ {!isPaidPlan && (
+ <MarginHostingCTA tagline={t`Have your email configured for you.`} />
+ )}
+ </Flex>
);
};
diff --git a/frontend/src/metabase/admin/settings/settings.js b/frontend/src/metabase/admin/settings/settings.js
index 09f071fc0f0..d6450f31d52 100644
--- a/frontend/src/metabase/admin/settings/settings.js
+++ b/frontend/src/metabase/admin/settings/settings.js
@@ -116,8 +116,11 @@ export const sendTestEmail = createThunkAction(SEND_TEST_EMAIL, function () {
export const CLEAR_EMAIL_SETTINGS =
"metabase/admin/settings/CLEAR_EMAIL_SETTINGS";
-export const clearEmailSettings = createAction(CLEAR_EMAIL_SETTINGS, () =>
- EmailApi.clear(),
+export const clearEmailSettings = createThunkAction(
+ CLEAR_EMAIL_SETTINGS,
+ () => async dispatch => {
+ await EmailApi.clear(), await dispatch(reloadSettings());
+ },
);
export const UPDATE_SLACK_SETTINGS =
--
GitLab