Skip to content

Commit

Permalink
chore: [Plugin Action Form] Common Editor State (#36512)
Browse files Browse the repository at this point in the history
## 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:
appsmithorg/appsmith-ee#5217


Parts of #36154 



## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
hetunandu committed Sep 25, 2024
1 parent 5ee7f83 commit a93e87b
Show file tree
Hide file tree
Showing 13 changed files with 216 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const APIEditorForm = () => {
/>
}
formName={FORM_NAME}
headersCount={0}
httpMethodOptions={HTTP_METHOD_OPTIONS}
isChangePermitted={isChangePermitted}
paginationUiComponent={
Expand All @@ -45,7 +44,6 @@ const APIEditorForm = () => {
theme={theme}
/>
}
paramsCount={0}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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 }[];
Expand All @@ -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 (
<Flex flexDirection="column" gap="spaces-3" w="100%">
Expand All @@ -48,17 +40,15 @@ const CommonEditorForm = (props: Props) => {
/>
<HintMessages hintMessages={hintMessages} />
<RequestTabs
actionConfigurationHeaders={props.actionConfigurationHeaders}
actionConfigurationParams={props.actionConfigurationParams}
actionConfigurationHeaders={actionHeaders}
actionConfigurationParams={actionParams}
actionName={action.name}
autogeneratedHeaders={props.autoGeneratedActionConfigHeaders}
autogeneratedHeaders={autoGeneratedHeaders}
bodyUIComponent={props.bodyUIComponent}
datasourceHeaders={props.datasourceHeaders}
datasourceParams={props.datasourceParams}
datasourceHeaders={datasourceHeaders}
datasourceParams={datasourceParams}
formName={props.formName}
headersCount={props.headersCount}
paginationUiComponent={props.paginationUiComponent}
paramsCount={props.paramsCount}
pushFields={props.isChangePermitted}
showSettings={false}
theme={theme}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import ApiAuthentication from "pages/Editor/APIEditor/ApiAuthentication";
import ActionSettings from "pages/Editor/ActionSettings";
import { API_EDITOR_TAB_TITLES, createMessage } from "ee/constants/messages";
import { useSelectedFormTab } from "./hooks/useSelectedFormTab";
import { getHeadersCount, getParamsCount } from "./utils";
import type { Property } from "entities/Action";

const SettingsWrapper = styled.div`
padding: var(--ads-v2-spaces-4) 0;
Expand All @@ -31,23 +33,13 @@ const StyledTabPanel = styled(TabPanel)`

export function RequestTabs(props: {
autogeneratedHeaders: AutoGeneratedHeader[] | undefined;
// 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
actionConfigurationHeaders: any;
headersCount: number;
datasourceHeaders: Property[];
actionConfigurationHeaders: Property[];
actionName: string;
pushFields: boolean;
theme: EditorTheme.LIGHT;
// 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
actionConfigurationParams: any;
paramsCount: number;
datasourceParams: Property[];
actionConfigurationParams: Property[];
bodyUIComponent: React.ReactNode;
paginationUiComponent: React.ReactNode;
formName: string;
Expand All @@ -57,27 +49,41 @@ export function RequestTabs(props: {
actionSettingsConfig?: any;
}) {
const [value, onValueChange] = useSelectedFormTab();
const headersCount = getHeadersCount(
props.actionConfigurationHeaders,
props.datasourceHeaders,
props.autogeneratedHeaders,
);

const paramsCount = getParamsCount(
props.actionConfigurationParams,
props.datasourceHeaders,
);

return (
<Tabs className="h-full" onValueChange={onValueChange} value={value}>
<TabsListWrapper>
<TabsList>
{Object.values(API_EDITOR_TABS).map((tab) => (
<Tab
data-testid={`t--api-editor-${tab}`}
key={tab}
notificationCount={
tab == "HEADERS"
? props.headersCount
: tab == "PARAMS"
? props.paramsCount
: undefined
}
value={tab}
>
{createMessage(API_EDITOR_TAB_TITLES[tab])}
</Tab>
))}
{Object.values(API_EDITOR_TABS)
.filter((tab) => {
return !(!props.showSettings && tab === API_EDITOR_TABS.SETTINGS);
})
.map((tab) => (
<Tab
data-testid={`t--api-editor-${tab}`}
key={tab}
notificationCount={
tab == "HEADERS"
? headersCount
: tab == "PARAMS"
? paramsCount
: undefined
}
value={tab}
>
{createMessage(API_EDITOR_TAB_TITLES[tab])}
</Tab>
))}
</TabsList>
</TabsListWrapper>
<StyledTabPanel value={API_EDITOR_TABS.HEADERS}>
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { default as getHeadersCount } from "./getHeadersCount";
export { default as getParamsCount } from "./getParamsCount";
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => ({
Expand All @@ -22,6 +23,8 @@ jest.mock("PluginActionEditor", () => ({
usePluginActionContext: jest.fn(),
}));

jest.mock("utils/hooks/usePrevious", () => jest.fn());

describe("useChangeActionCall hook", () => {
const dispatchMock = jest.fn();

Expand Down Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -29,5 +33,5 @@ export const useChangeActionCall = () => {
);
break;
}
}, [action, plugin, dispatch]);
}, [action, plugin, dispatch, prevActionId]);
};
Loading

0 comments on commit a93e87b

Please sign in to comment.