Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import React from "react";
import APIEditorForm from "./components/APIEditorForm";
import { Flex } from "@appsmith/ads";
import { useChangeActionCall } from "./hooks/useChangeActionCall";

const PluginActionForm = () => {
useChangeActionCall();

return <div />;
return (
<Flex p="spaces-2" w="100%">
<APIEditorForm />
</Flex>
);
};

export default PluginActionForm;
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from "react";
import CommonEditorForm from "./CommonEditorForm";
import { usePluginActionContext } from "PluginActionEditor";
import { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig";
import { API_EDITOR_FORM_NAME } from "ee/constants/forms";
import { HTTP_METHOD_OPTIONS } from "constants/ApiEditorConstants/CommonApiConstants";
import PostBodyData from "pages/Editor/APIEditor/PostBodyData";
import { useFeatureFlag } from "utils/hooks/useFeatureFlag";
import { FEATURE_FLAG } from "ee/entities/FeatureFlag";
import { getHasManageActionPermission } from "ee/utils/BusinessFeatures/permissionPageHelpers";
import Pagination from "pages/Editor/APIEditor/Pagination";
import { noop } from "lodash";
import { reduxForm } from "redux-form";

const FORM_NAME = API_EDITOR_FORM_NAME;

const APIEditorForm = () => {
const { action } = usePluginActionContext();
const theme = EditorTheme.LIGHT;

const isFeatureEnabled = useFeatureFlag(FEATURE_FLAG.license_gac_enabled);
const isChangePermitted = getHasManageActionPermission(
isFeatureEnabled,
action.userPermissions,
);

return (
<CommonEditorForm
action={action}
bodyUIComponent={
<PostBodyData
dataTreePath={`${action.name}.config`}
theme={EditorTheme.LIGHT}
/>
}
formName={FORM_NAME}
headersCount={0}
httpMethodOptions={HTTP_METHOD_OPTIONS}
isChangePermitted={isChangePermitted}
paginationUiComponent={
<Pagination
actionName={action.name}
onTestClick={noop}
paginationType={action.actionConfiguration.paginationType}
theme={theme}
/>
}
paramsCount={0}
/>
);
};

export default reduxForm({ form: FORM_NAME, enableReinitialize: true })(
APIEditorForm,
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
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";

interface Props {
httpMethodOptions: { value: string }[];
action: Action;
formName: string;
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be modified to unknown?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to update these to the correct types here. Will do that next as I move this props to its usage point

// 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;

return (
<Flex flexDirection="column" gap="spaces-3" w="100%">
<InfoFields
actionName={action.name}
changePermitted={props.isChangePermitted}
formName={props.formName}
options={props.httpMethodOptions}
pluginId={action.pluginId}
theme={theme}
/>
<HintMessages hintMessages={hintMessages} />
<RequestTabs
actionConfigurationHeaders={props.actionConfigurationHeaders}
actionConfigurationParams={props.actionConfigurationParams}
actionName={action.name}
autogeneratedHeaders={props.autoGeneratedActionConfigHeaders}
bodyUIComponent={props.bodyUIComponent}
datasourceHeaders={props.datasourceHeaders}
datasourceParams={props.datasourceParams}
formName={props.formName}
headersCount={props.headersCount}
paginationUiComponent={props.paginationUiComponent}
paramsCount={props.paramsCount}
pushFields={props.isChangePermitted}
showSettings={false}
theme={theme}
/>
</Flex>
);
};

export default CommonEditorForm;
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from "react";
import { Callout } from "@appsmith/ads";

export function HintMessages(props: { hintMessages: string[] }) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering why we need this separate component since it is a very small one and doesn't have much added functionality

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this just a map on top of an ADS component

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intend to move the selector for this into this component as well so it will have some more logic.

I also believe this can change on its own in the future. It is a logical UI separation based on the current layout compared to the Request Info and Request tabs. Easy to manage this and find this when it is separated out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. Although I am still not able to clearly envision the use case since any place that might need this component will also need to pass it's own data and make things more complicated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a valid concern. Let me update this as well next

if (props.hintMessages.length === 0) {
return null;
}

return (
<>
{props.hintMessages.map((msg, i) => (
<Callout key={i} kind="info">
{msg}
</Callout>
))}
</>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from "react";
import type { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig";
import RequestDropdownField from "components/editorComponents/form/fields/RequestDropdownField";
import { replayHighlightClass } from "globalStyles/portals";
import EmbeddedDatasourcePathField from "components/editorComponents/form/fields/EmbeddedDatasourcePathField";
import styled from "styled-components";
import { Flex } from "@appsmith/ads";

const DatasourceWrapper = styled.div`
margin-left: 8px;
width: 100%;
`;

export function InfoFields(props: {
changePermitted: boolean;
options: { value: string }[];
actionName: string;
pluginId: string;
formName: string;
theme: EditorTheme.LIGHT;
}) {
return (
<Flex w="100%">
<div>
<RequestDropdownField
className={`t--apiFormHttpMethod ${replayHighlightClass}`}
data-location-id={btoa("actionConfiguration.httpMethod")}
disabled={!props.changePermitted}
name="actionConfiguration.httpMethod"
options={props.options}
placeholder="Method"
width={"110px"}
>
<div />
</RequestDropdownField>
</div>
<DatasourceWrapper className="t--dataSourceField">
<EmbeddedDatasourcePathField
actionName={props.actionName}
codeEditorVisibleOverflow
formName={props.formName}
name="actionConfiguration.path"
placeholder="https://mock-api.appsmith.com/users"
pluginId={props.pluginId}
theme={props.theme}
/>
</DatasourceWrapper>
</Flex>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import styled from "styled-components";
import { Tab, TabPanel, Tabs, TabsList } from "@appsmith/ads";
import FormLabel from "components/editorComponents/FormLabel";
import type { AutoGeneratedHeader } from "pages/Editor/APIEditor/helpers";
import type { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig";
import React from "react";
import { API_EDITOR_TABS } from "constants/ApiEditorConstants/CommonApiConstants";
import { DatasourceConfig } from "./components/DatasourceConfig";
import KeyValueFieldArray from "components/editorComponents/form/fields/KeyValueFieldArray";
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";

const SettingsWrapper = styled.div`
padding: var(--ads-v2-spaces-4) 0;
height: 100%;

${FormLabel} {
padding: 0;
}
`;
const TabsListWrapper = styled.div`
padding: 0 var(--ads-v2-spaces-7);
`;
const StyledTabPanel = styled(TabPanel)`
height: calc(100% - 50px);
overflow: auto;
padding: 0 var(--ads-v2-spaces-7);
`;

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
Comment on lines +34 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the TODO comments and fix the eslint warnings.

There are several TODO comments indicating eslint warnings that need to be fixed. It's important to address these warnings to maintain code quality and consistency.

Also applies to: 38-39, 45-46, 48-49, 56-57

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;
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;
bodyUIComponent: React.ReactNode;
paginationUiComponent: React.ReactNode;
formName: string;
showSettings: boolean;
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
actionSettingsConfig?: any;
}) {
Comment on lines +32 to +58
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider extracting some of the tab rendering logic into separate components.

The RequestTabs component is quite large and has a lot of props. To further improve modularity and readability, you could extract the rendering logic for each tab into separate components. This would make the RequestTabs component more focused and easier to maintain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats the plan for the next PR :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hetunandu, I'm glad to hear that you're planning to extract the tab rendering logic into separate components in your next PR. This approach will enhance the modularity and readability of your code. Would you like me to open a GitHub issue to track this enhancement?

(^v^)


Learnings added
Learnt from: hetunandu
PR: appsmithorg/appsmith#36380
File: app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/RequestTabs.tsx:32-58
Timestamp: 2024-09-19T10:42:56.375Z
Learning: The tab rendering logic of the `RequestTabs` component will be extracted into separate components in a future PR.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

const [value, onValueChange] = useSelectedFormTab();

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>
))}
</TabsList>
</TabsListWrapper>
<StyledTabPanel value={API_EDITOR_TABS.HEADERS}>
<DatasourceConfig
attributeName="header"
autogeneratedHeaders={props.autogeneratedHeaders}
data={props.datasourceHeaders}
/>
<KeyValueFieldArray
actionConfig={props.actionConfigurationHeaders}
dataTreePath={`${props.actionName}.config.headers`}
hideHeader
label="Headers"
name="actionConfiguration.headers"
placeholder="Value"
pushFields={props.pushFields}
theme={props.theme}
/>
</StyledTabPanel>
<StyledTabPanel value={API_EDITOR_TABS.PARAMS}>
<DatasourceConfig
attributeName={"param"}
data={props.datasourceParams}
/>
<KeyValueFieldArray
actionConfig={props.actionConfigurationParams}
dataTreePath={`${props.actionName}.config.queryParameters`}
hideHeader
label="Params"
name="actionConfiguration.queryParameters"
pushFields={props.pushFields}
theme={props.theme}
/>
</StyledTabPanel>
<StyledTabPanel className="h-full" value={API_EDITOR_TABS.BODY}>
{props.bodyUIComponent}
</StyledTabPanel>
<StyledTabPanel value={API_EDITOR_TABS.PAGINATION}>
{props.paginationUiComponent}
</StyledTabPanel>
<StyledTabPanel value={API_EDITOR_TABS.AUTHENTICATION}>
<ApiAuthentication formName={props.formName} />
</StyledTabPanel>
{props.showSettings ? (
<StyledTabPanel value={API_EDITOR_TABS.SETTINGS}>
<SettingsWrapper>
<ActionSettings
actionSettingsConfig={props.actionSettingsConfig}
formName={props.formName}
theme={props.theme}
/>
</SettingsWrapper>
</StyledTabPanel>
) : null}
</Tabs>
);
}
Loading