From 79f5e9375483359ac3900a9ed0075500b239f943 Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Mon, 29 Aug 2022 14:46:38 +0100
Subject: [PATCH] Data app settings form (#25028)
* Form framework: extract `getMaybeNestedValue` util
* Form framework: handle nested fields in initial values
* Use different forms for creating and updating apps
* Fix data app update endpoint
* Update underlying collection on app update
* Add app settings modal
* Make `EditableText` handle initial value change
* Simplify `getMaybeNestedValue`
* Use `useEffect` instead of `useLayoutEffect`
---
.../containers/FormikForm/FormikForm.tsx | 27 ++++++--
.../FormikForm/FormikFormViewAdapter.tsx | 12 +---
frontend/src/metabase/containers/formUtils.js | 10 +++
.../components/EditableText/EditableText.tsx | 11 ++++
.../metabase/entities/data-apps/data-apps.ts | 24 +++++--
.../src/metabase/entities/data-apps/forms.ts | 19 +++++-
.../MainNavbar/DataAppNavbarContainer.tsx | 63 +++++++++++++++----
.../MainNavbar/DataAppNavbarView.tsx | 14 ++++-
frontend/src/metabase/services.js | 2 +-
.../CreateDataAppModal/CreateDataAppModal.tsx | 2 +-
10 files changed, 149 insertions(+), 35 deletions(-)
diff --git a/frontend/src/metabase/containers/FormikForm/FormikForm.tsx b/frontend/src/metabase/containers/FormikForm/FormikForm.tsx
index 6de3c66b64c..5e8ef961ab9 100644
--- a/frontend/src/metabase/containers/FormikForm/FormikForm.tsx
+++ b/frontend/src/metabase/containers/FormikForm/FormikForm.tsx
@@ -1,7 +1,7 @@
import React, { useCallback, useMemo, useState } from "react";
import { t } from "ttag";
import _ from "underscore";
-import { merge } from "icepick";
+import { assocIn, getIn, merge } from "icepick";
// eslint-disable-next-line import/named
import { Formik, FormikProps, FormikErrors, FormikHelpers } from "formik";
@@ -16,7 +16,12 @@ import {
import FormikFormViewAdapter from "./FormikFormViewAdapter";
import useInlineFields from "./useInlineFields";
-import { makeFormObject, cleanObject } from "../formUtils";
+import {
+ makeFormObject,
+ cleanObject,
+ isNestedFieldName,
+ getMaybeNestedValue,
+} from "../formUtils";
interface FormContainerProps<Values extends BaseFieldValues> {
form?: FormObject<Values>;
@@ -125,14 +130,28 @@ function Form<Values extends BaseFieldValues>({
const initialValues = useMemo(() => {
const fieldNames = formObject.fieldNames(values);
+ const [nestedFieldNames, regularFieldNames] = _.partition(
+ fieldNames,
+ isNestedFieldName,
+ );
+
+ let filteredInitialValues: FieldValues = {};
- const filteredInitialValues: FieldValues = {};
Object.keys(initialValuesProp || {}).forEach(fieldName => {
- if (fieldNames.includes(fieldName)) {
+ if (regularFieldNames.includes(fieldName)) {
filteredInitialValues[fieldName] = initialValuesProp[fieldName];
}
});
+ nestedFieldNames.forEach(nestedFieldName => {
+ const fieldValuePath = (nestedFieldName as string).split(".");
+ filteredInitialValues = assocIn(
+ filteredInitialValues,
+ fieldValuePath,
+ getIn(initialValuesProp, fieldValuePath),
+ );
+ });
+
return merge(formObject.initial(values), filteredInitialValues);
}, [values, initialValuesProp, formObject]);
diff --git a/frontend/src/metabase/containers/FormikForm/FormikFormViewAdapter.tsx b/frontend/src/metabase/containers/FormikForm/FormikFormViewAdapter.tsx
index 3b187a928fe..1b7af57a409 100644
--- a/frontend/src/metabase/containers/FormikForm/FormikFormViewAdapter.tsx
+++ b/frontend/src/metabase/containers/FormikForm/FormikFormViewAdapter.tsx
@@ -1,6 +1,5 @@
import React, { useEffect, useState } from "react";
import _ from "underscore";
-import { getIn } from "icepick";
// eslint-disable-next-line import/named
import { FormikProps } from "formik";
@@ -11,6 +10,7 @@ import { usePrevious } from "metabase/hooks/use-previous";
import { BaseFieldValues, FormField } from "metabase-types/forms";
+import { getMaybeNestedValue } from "../formUtils";
import FormView from "./FormView";
type FormProps<Values extends BaseFieldValues> = Omit<
@@ -34,14 +34,6 @@ type FormProps<Values extends BaseFieldValues> = Omit<
| "submitFailed"
>;
-function getMaybeNestedValue<Value = string>(
- obj: Record<string, Value>,
- fieldName: string,
-): Value {
- const isNestedField = fieldName.includes(".");
- return isNestedField ? getIn(obj, fieldName.split(".")) : obj[fieldName];
-}
-
interface FormikFormViewAdapterOwnProps<Values> {
onValuesChange: (values: Values) => void;
}
@@ -88,7 +80,7 @@ function FormikFormViewAdapter<Values extends BaseFieldValues>({
const value = getMaybeNestedValue(values, name);
const initialValue = getMaybeNestedValue(initialValues, name);
const error = getMaybeNestedValue(errors as Record<string, string>, name);
- const isTouched = !!getMaybeNestedValue<boolean>(
+ const isTouched = !!getMaybeNestedValue(
touched as Record<string, boolean>,
name,
);
diff --git a/frontend/src/metabase/containers/formUtils.js b/frontend/src/metabase/containers/formUtils.js
index c3f96bc93c9..fe94b57d303 100644
--- a/frontend/src/metabase/containers/formUtils.js
+++ b/frontend/src/metabase/containers/formUtils.js
@@ -100,3 +100,13 @@ export function cleanObject(object) {
});
return result;
}
+
+export function isNestedFieldName(name) {
+ return name.includes(".");
+}
+
+export function getMaybeNestedValue(obj, fieldName) {
+ return isNestedFieldName(fieldName)
+ ? getIn(obj, fieldName.split("."))
+ : obj[fieldName];
+}
diff --git a/frontend/src/metabase/core/components/EditableText/EditableText.tsx b/frontend/src/metabase/core/components/EditableText/EditableText.tsx
index bb59cf66c84..3673abc4f3c 100644
--- a/frontend/src/metabase/core/components/EditableText/EditableText.tsx
+++ b/frontend/src/metabase/core/components/EditableText/EditableText.tsx
@@ -5,9 +5,13 @@ import React, {
HTMLAttributes,
Ref,
useCallback,
+ useEffect,
useState,
useRef,
} from "react";
+
+import { usePrevious } from "metabase/hooks/use-previous";
+
import { EditableTextArea, EditableTextRoot } from "./EditableText.styled";
export type EditableTextAttributes = Omit<
@@ -42,6 +46,13 @@ const EditableText = forwardRef(function EditableText(
const [submitValue, setSubmitValue] = useState(initialValue ?? "");
const displayValue = inputValue ? inputValue : placeholder;
const submitOnBlur = useRef(true);
+ const previousInitialValue = usePrevious(initialValue);
+
+ useEffect(() => {
+ if (initialValue && initialValue !== previousInitialValue) {
+ setInputValue(initialValue);
+ }
+ }, [initialValue, previousInitialValue]);
const handleBlur = useCallback(
e => {
diff --git a/frontend/src/metabase/entities/data-apps/data-apps.ts b/frontend/src/metabase/entities/data-apps/data-apps.ts
index b12333e22b1..a7ebd209249 100644
--- a/frontend/src/metabase/entities/data-apps/data-apps.ts
+++ b/frontend/src/metabase/entities/data-apps/data-apps.ts
@@ -2,13 +2,13 @@ import { color } from "metabase/lib/colors";
import { createEntity } from "metabase/lib/entities";
import { DataAppSchema } from "metabase/schema";
-import { DataAppsApi } from "metabase/services";
+import { CollectionsApi, DataAppsApi } from "metabase/services";
import { Collection, DataApp } from "metabase-types/api";
import { DEFAULT_COLLECTION_COLOR_ALIAS } from "../collections/constants";
-import { createForm } from "./forms";
+import { createNewAppForm, createAppSettingsForm } from "./forms";
import { getDataAppIcon, isDataAppCollection } from "./utils";
type EditableDataAppParams = Pick<
@@ -20,6 +20,10 @@ type EditableDataAppParams = Pick<
type CreateDataAppParams = Partial<EditableDataAppParams> &
Pick<EditableDataAppParams, "name">;
+type UpdateDataAppParams = Pick<DataApp, "id" | "collection_id"> & {
+ collection: Pick<Collection, "name" | "description">;
+};
+
const DataApps = createEntity({
name: "dataApps",
nameOne: "dataApp",
@@ -45,6 +49,15 @@ const DataApps = createEntity({
},
});
},
+ update: async ({
+ id,
+ collection,
+ collection_id,
+ ...rest
+ }: UpdateDataAppParams) => {
+ await CollectionsApi.update({ ...collection, id: collection_id });
+ return DataAppsApi.update({ id, ...rest });
+ },
},
objectSelectors: {
@@ -52,8 +65,11 @@ const DataApps = createEntity({
},
forms: {
- details: {
- fields: createForm,
+ create: {
+ fields: createNewAppForm,
+ },
+ settings: {
+ fields: createAppSettingsForm,
},
},
});
diff --git a/frontend/src/metabase/entities/data-apps/forms.ts b/frontend/src/metabase/entities/data-apps/forms.ts
index c7cd5738b60..fa931ecff50 100644
--- a/frontend/src/metabase/entities/data-apps/forms.ts
+++ b/frontend/src/metabase/entities/data-apps/forms.ts
@@ -1,5 +1,22 @@
import { createNameField, createDescriptionField } from "../collections/forms";
-export function createForm() {
+export function createNewAppForm() {
return [createNameField(), createDescriptionField()];
}
+
+export function createAppSettingsForm() {
+ return [
+ {
+ ...createNameField(),
+ name: "collection.name",
+ },
+ {
+ ...createDescriptionField(),
+ name: "collection.description",
+ },
+ {
+ name: "collection_id",
+ type: "hidden",
+ },
+ ];
+}
diff --git a/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarContainer.tsx b/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarContainer.tsx
index 7e38b9db4be..484497b4a40 100644
--- a/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarContainer.tsx
+++ b/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarContainer.tsx
@@ -1,4 +1,7 @@
-import React, { useMemo } from "react";
+import React, { useCallback, useMemo, useState } from "react";
+import { t } from "ttag";
+
+import Modal from "metabase/components/Modal";
import * as Urls from "metabase/lib/urls";
@@ -15,6 +18,8 @@ import DataAppNavbarView from "./DataAppNavbarView";
const FETCHING_SEARCH_MODELS = ["dashboard", "dataset", "card"];
const LIMIT = 100;
+type NavbarModal = "MODAL_APP_SETTINGS" | null;
+
interface Props extends MainNavbarProps {
dataApp: DataApp;
loading: boolean;
@@ -31,6 +36,8 @@ function DataAppNavbarContainer({
loading: loadingDataApp,
...props
}: Props) {
+ const [modal, setModal] = useState<NavbarModal>(null);
+
const collectionContentQuery = useMemo(() => {
if (!dataApp) {
return {};
@@ -42,22 +49,54 @@ function DataAppNavbarContainer({
};
}, [dataApp]);
+ const onEditAppSettings = useCallback(() => {
+ setModal("MODAL_APP_SETTINGS");
+ }, []);
+
+ const closeModal = useCallback(() => setModal(null), []);
+
+ const renderModalContent = useCallback(() => {
+ if (modal === "MODAL_APP_SETTINGS") {
+ return (
+ <DataApps.ModalForm
+ form={DataApps.forms.settings}
+ title={t`Settings`}
+ dataApp={dataApp}
+ onClose={closeModal}
+ onSaved={closeModal}
+ submitTitle={t`Save`}
+ />
+ );
+ }
+ return null;
+ }, [dataApp, modal, closeModal]);
+
if (loadingDataApp) {
return <NavbarLoadingView />;
}
return (
- <Search.ListLoader
- query={collectionContentQuery}
- loadingAndErrorWrapper={false}
- >
- {({ list = [], loading: loadingAppContent }: SearchRenderProps) => {
- if (loadingAppContent) {
- return <NavbarLoadingView />;
- }
- return <DataAppNavbarView {...props} dataApp={dataApp} items={list} />;
- }}
- </Search.ListLoader>
+ <>
+ <Search.ListLoader
+ query={collectionContentQuery}
+ loadingAndErrorWrapper={false}
+ >
+ {({ list = [], loading: loadingAppContent }: SearchRenderProps) => {
+ if (loadingAppContent) {
+ return <NavbarLoadingView />;
+ }
+ return (
+ <DataAppNavbarView
+ {...props}
+ dataApp={dataApp}
+ items={list}
+ onEditAppSettings={onEditAppSettings}
+ />
+ );
+ }}
+ </Search.ListLoader>
+ {modal && <Modal onClose={closeModal}>{renderModalContent()}</Modal>}
+ </>
);
}
diff --git a/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarView.tsx b/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarView.tsx
index fe2637d94fa..f47c3a6c064 100644
--- a/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarView.tsx
+++ b/frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarView.tsx
@@ -26,9 +26,15 @@ interface Props extends MainNavbarProps {
dataApp: DataApp;
items: any[];
selectedItems: SelectedItem[];
+ onEditAppSettings: () => void;
}
-function DataAppNavbarView({ dataApp, items, selectedItems }: Props) {
+function DataAppNavbarView({
+ dataApp,
+ items,
+ selectedItems,
+ onEditAppSettings,
+}: Props) {
const appPages = useMemo(
() => items.filter(item => item.model === "dashboard"),
[items],
@@ -63,7 +69,11 @@ function DataAppNavbarView({ dataApp, items, selectedItems }: Props) {
<DataAppActionButton icon="add" onlyIcon />
</Tooltip>
<Tooltip tooltip={t`Settings`}>
- <DataAppActionButton icon="gear" onlyIcon />
+ <DataAppActionButton
+ icon="gear"
+ onClick={onEditAppSettings}
+ onlyIcon
+ />
</Tooltip>
</ButtonGroup>
<ExitDataAppButton
diff --git a/frontend/src/metabase/services.js b/frontend/src/metabase/services.js
index 02e5564e60a..e24743cd747 100644
--- a/frontend/src/metabase/services.js
+++ b/frontend/src/metabase/services.js
@@ -183,7 +183,7 @@ export const CollectionsApi = {
export const DataAppsApi = {
list: GET("/api/app"),
create: POST("/api/app"),
- update: PUT("/api/app"),
+ update: PUT("/api/app/:id"),
};
const PIVOT_PUBLIC_PREFIX = "/api/public/pivot/";
diff --git a/frontend/src/metabase/writeback/containers/CreateDataAppModal/CreateDataAppModal.tsx b/frontend/src/metabase/writeback/containers/CreateDataAppModal/CreateDataAppModal.tsx
index 7dcdae8918b..7450e18cb23 100644
--- a/frontend/src/metabase/writeback/containers/CreateDataAppModal/CreateDataAppModal.tsx
+++ b/frontend/src/metabase/writeback/containers/CreateDataAppModal/CreateDataAppModal.tsx
@@ -35,7 +35,7 @@ function CreateDataAppModal({ onClose, onChangeLocation }: Props) {
return (
<DataApps.ModalForm
- form={DataApps.forms.details}
+ form={DataApps.forms.create}
onSaved={handleSave}
onClose={onClose}
/>
--
GitLab