From a93e87ba189fd3f627bc2747c6b6d36741074d08 Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Wed, 25 Sep 2024 13:31:38 +0530 Subject: [PATCH] chore: [Plugin Action Form] Common Editor State (#36512) ## Description Passes the correct states for the Common Editor form in the Plugin Action Form. - Uses a hook to pass the form states as already implemented - Uses helper functions to get the header and params count from the said state - As a side effect I can remove these from the original implementation since the invocation is now in the common `RequestTabs` components - Updates the `changeActionCall` hook to only be called if the action changes - Updates the tests to reflect this EE PR to track tests: https://github.com/appsmithorg/appsmith-ee/pull/5217 Parts of #36154 ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced a custom hook for streamlined retrieval of action-related values in forms. - Added utility functions for counting valid headers and parameters. - **Improvements** - Simplified component interfaces by removing unnecessary properties related to headers and parameters. - Enhanced type safety for header and parameter properties. - Refined tab rendering logic for better user experience. - **Bug Fixes** - Adjusted logic to ensure actions are dispatched only on changes to action IDs. - **Documentation** - Updated relevant documentation to reflect changes in component props and functionalities. --- .../components/APIEditorForm.tsx | 2 - .../CommonEditorForm/CommonEditorForm.tsx | 36 ++++----- .../CommonEditorForm/RequestTabs.tsx | 66 +++++++++------- .../hooks/useGetFormActionValues.ts | 78 +++++++++++++++++++ .../CommonEditorForm/utils/getHeadersCount.ts | 22 ++++++ .../CommonEditorForm/utils/getParamsCount.ts | 14 ++++ .../utils/getValidProperties.ts | 11 +++ .../CommonEditorForm/utils/index.ts | 2 + .../hooks/useChangeActionCall.test.tsx | 35 +++++++++ .../hooks/useChangeActionCall.ts | 6 +- .../Editor/APIEditor/CommonEditorForm.tsx | 6 -- .../APIEditor/GraphQL/GraphQLEditorForm.tsx | 44 ----------- .../pages/Editor/APIEditor/RestAPIForm.tsx | 51 ------------ 13 files changed, 216 insertions(+), 157 deletions(-) create mode 100644 app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts create mode 100644 app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getHeadersCount.ts create mode 100644 app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getParamsCount.ts create mode 100644 app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getValidProperties.ts create mode 100644 app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/index.ts diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/APIEditorForm.tsx b/app/client/src/PluginActionEditor/components/PluginActionForm/components/APIEditorForm.tsx index f3a8d8ffd62..d8cbdd88eb2 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionForm/components/APIEditorForm.tsx +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/APIEditorForm.tsx @@ -34,7 +34,6 @@ const APIEditorForm = () => { /> } formName={FORM_NAME} - headersCount={0} httpMethodOptions={HTTP_METHOD_OPTIONS} isChangePermitted={isChangePermitted} paginationUiComponent={ @@ -45,7 +44,6 @@ const APIEditorForm = () => { theme={theme} /> } - paramsCount={0} /> ); }; diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/CommonEditorForm.tsx b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/CommonEditorForm.tsx index 881fdc56147..a9ae550c0ec 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/CommonEditorForm.tsx +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/CommonEditorForm.tsx @@ -1,11 +1,11 @@ import React from "react"; import { type Action } from "entities/Action"; import { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig"; -import type { AutoGeneratedHeader } from "pages/Editor/APIEditor/helpers"; import { InfoFields } from "./InfoFields"; import { RequestTabs } from "./RequestTabs"; import { HintMessages } from "./HintMessages"; import { Flex } from "@appsmith/ads"; +import useGetFormActionValues from "./hooks/useGetFormActionValues"; interface Props { httpMethodOptions: { value: string }[]; @@ -14,27 +14,19 @@ interface Props { isChangePermitted: boolean; bodyUIComponent: React.ReactNode; paginationUiComponent: React.ReactNode; - headersCount: number; - paramsCount: number; - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - datasourceHeaders?: any; - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - datasourceParams?: any; - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - actionConfigurationHeaders?: any; - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - actionConfigurationParams?: any; - autoGeneratedActionConfigHeaders?: AutoGeneratedHeader[]; } const CommonEditorForm = (props: Props) => { const { action } = props; const hintMessages = action.messages || []; const theme = EditorTheme.LIGHT; + const { + actionHeaders, + actionParams, + autoGeneratedHeaders, + datasourceHeaders, + datasourceParams, + } = useGetFormActionValues(); return ( @@ -48,17 +40,15 @@ const CommonEditorForm = (props: Props) => { /> - {Object.values(API_EDITOR_TABS).map((tab) => ( - - {createMessage(API_EDITOR_TAB_TITLES[tab])} - - ))} + {Object.values(API_EDITOR_TABS) + .filter((tab) => { + return !(!props.showSettings && tab === API_EDITOR_TABS.SETTINGS); + }) + .map((tab) => ( + + {createMessage(API_EDITOR_TAB_TITLES[tab])} + + ))} diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts new file mode 100644 index 00000000000..1078d18282d --- /dev/null +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/hooks/useGetFormActionValues.ts @@ -0,0 +1,78 @@ +import { getFormValues } from "redux-form"; +import { API_EDITOR_FORM_NAME } from "ee/constants/forms"; +import { getCurrentEnvironmentId } from "ee/selectors/environmentSelectors"; +import get from "lodash/get"; +import { useSelector } from "react-redux"; +import { + type Action, + type ApiAction, + isAPIAction, + type Property, +} from "entities/Action"; +import { getDatasources } from "ee/selectors/entitiesSelector"; + +function useGetFormActionValues() { + const formValues = useSelector(getFormValues(API_EDITOR_FORM_NAME)) as Action; + const datasources = useSelector(getDatasources); + const currentEnvironment = useSelector(getCurrentEnvironmentId); + + // In an unlikely scenario where form is not initialised, + // return empty values to avoid form ui issues + if (!isAPIAction(formValues)) { + return { + actionHeaders: [], + actionParams: [], + autoGeneratedHeaders: [], + datasourceParams: [], + datasourceHeaders: [], + }; + } + + const actionHeaders = get( + formValues, + "actionConfiguration.headers", + [], + ) as Property[]; + + const autoGeneratedHeaders: ApiAction["actionConfiguration"]["autoGeneratedHeaders"] = + get(formValues, "actionConfiguration.autoGeneratedHeaders", []); + + const actionParams = get( + formValues, + "actionConfiguration.queryParameters", + [], + ) as Property[]; + + let datasourceFromAction: Action["datasource"] | undefined = get( + formValues, + "datasource", + ); + + if (datasourceFromAction && Object.hasOwn(datasourceFromAction, "id")) { + datasourceFromAction = datasources.find( + (d) => d.id === datasourceFromAction?.id, + ); + } + + const datasourceHeaders = get( + datasourceFromAction, + `datasourceStorages.${currentEnvironment}.datasourceConfiguration.headers`, + [], + ) as Property[]; + + const datasourceParams = get( + datasourceFromAction, + `datasourceStorages.${currentEnvironment}.datasourceConfiguration.queryParameters`, + [], + ) as Property[]; + + return { + actionHeaders, + autoGeneratedHeaders, + actionParams, + datasourceHeaders, + datasourceParams, + }; +} + +export default useGetFormActionValues; diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getHeadersCount.ts b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getHeadersCount.ts new file mode 100644 index 00000000000..a0b124525b4 --- /dev/null +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getHeadersCount.ts @@ -0,0 +1,22 @@ +import getValidProperties from "./getValidProperties"; +import type { Property } from "entities/Action"; + +function getHeadersCount( + actionHeaders?: Property[], + datasourceHeaders?: Property[], + autoGeneratedActionHeaders?: Property[], +): number { + const validActionHeaders = getValidProperties(actionHeaders); + const validDatasourceHeaders = getValidProperties(datasourceHeaders); + const validAutoGeneratedHeaders = getValidProperties( + autoGeneratedActionHeaders, + ); + + return ( + validActionHeaders.length + + validDatasourceHeaders.length + + validAutoGeneratedHeaders.length + ); +} + +export default getHeadersCount; diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getParamsCount.ts b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getParamsCount.ts new file mode 100644 index 00000000000..3d91bf946cf --- /dev/null +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getParamsCount.ts @@ -0,0 +1,14 @@ +import type { Property } from "entities/Action"; +import getValidProperties from "./getValidProperties"; + +function getParamsCount( + actionParams?: Property[], + datasourceParams?: Property[], +) { + const validActionParams = getValidProperties(actionParams); + const validDatasourceParams = getValidProperties(datasourceParams); + + return validActionParams.length + validDatasourceParams.length; +} + +export default getParamsCount; diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getValidProperties.ts b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getValidProperties.ts new file mode 100644 index 00000000000..a2822433bd9 --- /dev/null +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/getValidProperties.ts @@ -0,0 +1,11 @@ +import type { Property } from "entities/Action"; + +function getValidProperties(value?: Property[]) { + if (!Array.isArray(value)) { + return []; + } + + return value.filter((v) => v.key && v.key !== ""); +} + +export default getValidProperties; diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/index.ts b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/index.ts new file mode 100644 index 00000000000..76d68357ccc --- /dev/null +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/index.ts @@ -0,0 +1,2 @@ +export { default as getHeadersCount } from "./getHeadersCount"; +export { default as getParamsCount } from "./getParamsCount"; diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.test.tsx b/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.test.tsx index c7af1a52bbb..98c144af91a 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.test.tsx +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.test.tsx @@ -4,6 +4,7 @@ import { PluginType } from "entities/Action"; import { usePluginActionContext } from "PluginActionEditor"; import { changeApi } from "actions/apiPaneActions"; import { changeQuery } from "actions/queryPaneActions"; +import usePrevious from "utils/hooks/usePrevious"; import { useChangeActionCall } from "./useChangeActionCall"; jest.mock("react-redux", () => ({ @@ -22,6 +23,8 @@ jest.mock("PluginActionEditor", () => ({ usePluginActionContext: jest.fn(), })); +jest.mock("utils/hooks/usePrevious", () => jest.fn()); + describe("useChangeActionCall hook", () => { const dispatchMock = jest.fn(); @@ -105,4 +108,36 @@ describe("useChangeActionCall hook", () => { // Expect no action to be dispatched expect(dispatchMock).not.toHaveBeenCalled(); }); + + it("should not dispatch any action if the action Id has not changed", () => { + const actionMock = { id: "actionId" }; + const pluginMock = { id: "pluginId", type: PluginType.API }; + + // First we mount, so it should be called as previous action id was undefined + (usePluginActionContext as jest.Mock).mockReturnValueOnce({ + action: actionMock, + plugin: pluginMock, + }); + (usePrevious as jest.Mock).mockReturnValueOnce(undefined); + renderHook(() => useChangeActionCall()); + expect(changeApi).toHaveBeenCalledTimes(1); + + // Now we mock the action object to change but not the id. It should not be called again + (usePluginActionContext as jest.Mock).mockReturnValueOnce({ + action: { ...actionMock, testId: "test" }, + plugin: pluginMock, + }); + (usePrevious as jest.Mock).mockReturnValueOnce("actionId"); + renderHook(() => useChangeActionCall()); + expect(changeApi).toHaveBeenCalledTimes(1); + + // Now we change the action id, so it will be called the second time + (usePluginActionContext as jest.Mock).mockReturnValueOnce({ + action: { id: "actionId2", testId: "test" }, + plugin: pluginMock, + }); + (usePrevious as jest.Mock).mockReturnValueOnce("actionId"); + renderHook(() => useChangeActionCall()); + expect(changeApi).toHaveBeenCalledTimes(2); + }); }); diff --git a/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.ts b/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.ts index 562bbad8679..88a56e14d90 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.ts +++ b/app/client/src/PluginActionEditor/components/PluginActionForm/hooks/useChangeActionCall.ts @@ -4,14 +4,18 @@ import { changeApi } from "actions/apiPaneActions"; import { changeQuery } from "actions/queryPaneActions"; import { PluginType } from "entities/Action"; import { usePluginActionContext } from "PluginActionEditor"; +import usePrevious from "utils/hooks/usePrevious"; export const useChangeActionCall = () => { const { action, plugin } = usePluginActionContext(); + const prevActionId = usePrevious(action.id); const dispatch = useDispatch(); useEffect(() => { if (!plugin?.id || !action) return; + if (prevActionId === action.id) return; + switch (plugin?.type) { case PluginType.API: dispatch(changeApi(action?.id, false)); @@ -29,5 +33,5 @@ export const useChangeActionCall = () => { ); break; } - }, [action, plugin, dispatch]); + }, [action, plugin, dispatch, prevActionId]); }; diff --git a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx index 1696f619724..b542cdfbd04 100644 --- a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx +++ b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx @@ -147,8 +147,6 @@ export interface CommonFormProps { actionName: string; apiId: string; apiName: string; - headersCount: number; - paramsCount: number; // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any settingsConfig: any; @@ -217,11 +215,9 @@ function CommonEditorForm(props: CommonFormPropsWithExtraParams) { closeEditorLink, formName, handleSubmit, - headersCount, hintMessages, isRunning, onRunClick, - paramsCount, pluginId, settingsConfig, } = props; @@ -340,9 +336,7 @@ function CommonEditorForm(props: CommonFormPropsWithExtraParams) { datasourceHeaders={props.datasourceHeaders} datasourceParams={props.datasourceParams} formName={formName} - headersCount={headersCount} paginationUiComponent={props.paginationUIComponent} - paramsCount={paramsCount} pushFields={isChangePermitted} showSettings theme={EditorTheme.LIGHT} diff --git a/app/client/src/pages/Editor/APIEditor/GraphQL/GraphQLEditorForm.tsx b/app/client/src/pages/Editor/APIEditor/GraphQL/GraphQLEditorForm.tsx index 91ee9aeb719..cc502267e6f 100644 --- a/app/client/src/pages/Editor/APIEditor/GraphQL/GraphQLEditorForm.tsx +++ b/app/client/src/pages/Editor/APIEditor/GraphQL/GraphQLEditorForm.tsx @@ -203,48 +203,6 @@ export default connect( const currentActionDatasourceId = selector(state, "datasource.id"); - const headers = selector(state, "actionConfiguration.headers"); - let headersCount = 0; - - if (Array.isArray(headers)) { - const validHeaders = headers.filter( - (value) => value.key && value.key !== "", - ); - - headersCount += validHeaders.length; - } - - if (Array.isArray(datasourceHeaders)) { - const validHeaders = datasourceHeaders.filter( - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (value: any) => value.key && value.key !== "", - ); - - headersCount += validHeaders.length; - } - - const params = selector(state, "actionConfiguration.queryParameters"); - let paramsCount = 0; - - if (Array.isArray(params)) { - const validParams = params.filter( - (value) => value.key && value.key !== "", - ); - - paramsCount = validParams.length; - } - - if (Array.isArray(datasourceParams)) { - const validParams = datasourceParams.filter( - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (value: any) => value.key && value.key !== "", - ); - - paramsCount += validParams.length; - } - const actionConfigurationBody = selector(state, "actionConfiguration.body") || ""; @@ -274,8 +232,6 @@ export default connect( currentActionDatasourceId, datasourceHeaders, datasourceParams, - headersCount, - paramsCount, hintMessages, datasources: state.entities.datasources.list.filter( (d) => d.pluginId === props.pluginId, diff --git a/app/client/src/pages/Editor/APIEditor/RestAPIForm.tsx b/app/client/src/pages/Editor/APIEditor/RestAPIForm.tsx index 80f1fe349c2..1435dca07e8 100644 --- a/app/client/src/pages/Editor/APIEditor/RestAPIForm.tsx +++ b/app/client/src/pages/Editor/APIEditor/RestAPIForm.tsx @@ -107,55 +107,6 @@ export default connect((state: AppState, props: { pluginId: string }) => { const currentActionDatasourceId = selector(state, "datasource.id"); const actionName = getApiName(state, apiId) || ""; - const headers = selector(state, "actionConfiguration.headers"); - let headersCount = 0; - - if (Array.isArray(headers)) { - const validHeaders = headers.filter( - (value) => value.key && value.key !== "", - ); - - headersCount += validHeaders.length; - } - - if (Array.isArray(datasourceHeaders)) { - const validHeaders = datasourceHeaders.filter( - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (value: any) => value.key && value.key !== "", - ); - - headersCount += validHeaders.length; - } - - if (Array.isArray(autoGeneratedActionConfigHeaders)) { - const validHeaders = autoGeneratedActionConfigHeaders.filter( - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (value: any) => value.key && value.key !== "", - ); - - headersCount += validHeaders.length; - } - - const params = selector(state, "actionConfiguration.queryParameters"); - let paramsCount = 0; - - if (Array.isArray(params)) { - const validParams = params.filter((value) => value.key && value.key !== ""); - - paramsCount = validParams.length; - } - - if (Array.isArray(datasourceParams)) { - const validParams = datasourceParams.filter( - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (value: any) => value.key && value.key !== "", - ); - - paramsCount += validParams.length; - } const responses = getActionResponses(state); const actionResponse = responses[apiId]; @@ -184,8 +135,6 @@ export default connect((state: AppState, props: { pluginId: string }) => { currentActionDatasourceId, datasourceHeaders, datasourceParams, - headersCount, - paramsCount, hintMessages, datasources: state.entities.datasources.list.filter( (d) => d.pluginId === props.pluginId,