Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Adding the functionality of running and deleting an action under modularised flow #36746

Merged
merged 8 commits into from
Oct 9, 2024
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
Expand Up @@ -9,13 +9,14 @@ 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";
import { useHandleRunClick } from "PluginActionEditor/hooks";

const FORM_NAME = API_EDITOR_FORM_NAME;

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

const isFeatureEnabled = useFeatureFlag(FEATURE_FLAG.license_gac_enabled);
Expand All @@ -39,7 +40,7 @@ const APIEditorForm = () => {
paginationUiComponent={
<Pagination
actionName={action.name}
onTestClick={noop}
onTestClick={handleRunClick}
paginationType={action.actionConfiguration.paginationType}
theme={theme}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { usePluginActionContext } from "PluginActionEditor";
import { useFeatureFlag } from "utils/hooks/useFeatureFlag";
import { FEATURE_FLAG } from "ee/entities/FeatureFlag";
import { getHasManageActionPermission } from "ee/utils/BusinessFeatures/permissionPageHelpers";
import { noop } from "lodash";
import { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig";
import useGetFormActionValues from "../CommonEditorForm/hooks/useGetFormActionValues";

Expand Down Expand Up @@ -38,7 +37,6 @@ function GraphQLEditorForm() {
<Pagination
actionName={action.name}
formName={FORM_NAME}
onTestClick={noop}
paginationType={action.actionConfiguration.paginationType}
query={actionConfigurationBody}
theme={theme}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import React, { useCallback } from "react";
import React from "react";
import { IDEToolbar } from "IDE";
import { Button, Menu, MenuContent, MenuTrigger, Tooltip } from "@appsmith/ads";
import { modText } from "utils/helpers";
import { usePluginActionContext } from "../PluginActionContext";
import { useDispatch } from "react-redux";
import AnalyticsUtil from "ee/utils/AnalyticsUtil";
import { runAction } from "actions/pluginActionActions";
import { useHandleRunClick } from "PluginActionEditor/hooks";
import { useToggle } from "@mantine/hooks";

interface PluginActionToolbarProps {
runOptions?: React.ReactNode;
Expand All @@ -14,25 +13,9 @@ interface PluginActionToolbarProps {
}

const PluginActionToolbar = (props: PluginActionToolbarProps) => {
const { action, datasource, plugin } = usePluginActionContext();
const dispatch = useDispatch();
const handleRunClick = useCallback(() => {
AnalyticsUtil.logEvent("RUN_QUERY_CLICK", {
actionName: action.name,
actionId: action.id,
pluginName: plugin.name,
datasourceId: datasource?.id,
isMock: datasource?.isMock,
});
dispatch(runAction(action.id));
}, [
action.id,
action.name,
datasource?.id,
datasource?.isMock,
dispatch,
plugin.name,
]);
const { action } = usePluginActionContext();
const { handleRunClick } = useHandleRunClick();
const [isMenuOpen, toggleMenuOpen] = useToggle([false, true]);

return (
<IDEToolbar>
Expand All @@ -44,7 +27,7 @@ const PluginActionToolbar = (props: PluginActionToolbarProps) => {
placement="topRight"
showArrow={false}
>
<Button kind="primary" onClick={handleRunClick} size="sm">
<Button kind="primary" onClick={() => handleRunClick()} size="sm">
Run
Copy link
Contributor

Choose a reason for hiding this comment

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

This creates a new function on every render and should be avoided () => handleRunClick(), instead it's better to use handleRunClick.

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Let's turn our attention to the Run button, students.

I see you've made a change to the onClick handler, but I'm afraid we've taken a step backward here. Remember our lesson on React performance? Creating a new function on every render with () => handleRunClick() is like rewriting your notes every time you want to read them - unnecessary and inefficient!

Let's go back to the previous implementation:

- <Button kind="primary" onClick={() => handleRunClick()} size="sm">
+ <Button kind="primary" onClick={handleRunClick} size="sm">

This way, we're passing the function directly, which is more efficient. Always remember, in React, every little optimization counts!

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button kind="primary" onClick={() => handleRunClick()} size="sm">
<Button kind="primary" onClick={handleRunClick} size="sm">

</Button>
</Tooltip>
Expand All @@ -54,7 +37,7 @@ const PluginActionToolbar = (props: PluginActionToolbarProps) => {
size="sm"
startIcon="settings-2-line"
/>
<Menu key={action.id}>
<Menu onOpenChange={toggleMenuOpen} open={isMenuOpen}>
<MenuTrigger>
<Button
isIconButton
Expand Down
2 changes: 2 additions & 0 deletions app/client/src/PluginActionEditor/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export { useActionSettingsConfig } from "ee/PluginActionEditor/hooks/useActionSettingsConfig";
export { useHandleDeleteClick } from "ee/PluginActionEditor/hooks/useHandleDeleteClick";
export { useHandleRunClick } from "ee/PluginActionEditor/hooks/useHandleRunClick";
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ import Schema from "components/editorComponents/Debugger/Schema";
import QueryResponseTab from "pages/Editor/QueryEditor/QueryResponseTab";
import type { SourceEntity } from "entities/AppsmithConsole";
import { ENTITY_TYPE as SOURCE_ENTITY_TYPE } from "ee/entities/AppsmithConsole/utils";
import { useHandleRunClick } from "PluginActionEditor/hooks";

function usePluginActionResponseTabs() {
const { action, actionResponse, datasource, plugin } =
usePluginActionContext();
const { handleRunClick } = useHandleRunClick();

const IDEViewMode = useSelector(getIDEViewMode);
const errorCount = useSelector(getErrorCount);
Expand Down Expand Up @@ -69,7 +71,7 @@ function usePluginActionResponseTabs() {
actionResponse={actionResponse}
isRunDisabled={false}
isRunning={false}
onRunClick={noop}
onRunClick={handleRunClick}
responseTabHeight={responseTabHeight}
theme={EditorTheme.LIGHT}
/>
Expand All @@ -84,7 +86,7 @@ function usePluginActionResponseTabs() {
isRunDisabled={false}
isRunning={false}
onDebugClick={noop}
onRunClick={noop}
onRunClick={handleRunClick}
/>
),
},
Expand Down Expand Up @@ -131,7 +133,7 @@ function usePluginActionResponseTabs() {
actionSource={actionSource}
currentActionConfig={action}
isRunning={false}
onRunClick={noop}
onRunClick={handleRunClick}
runErrorMessage={""} // TODO
/>
),
Expand Down
26 changes: 26 additions & 0 deletions app/client/src/ce/PluginActionEditor/hooks/useHandleDeleteClick.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { deleteAction } from "actions/pluginActionActions";
import { usePluginActionContext } from "PluginActionEditor/PluginActionContext";
import { useCallback } from "react";
import { useDispatch } from "react-redux";

function useHandleDeleteClick() {
const { action } = usePluginActionContext();
const dispatch = useDispatch();

const handleDeleteClick = useCallback(
({ onSuccess }: { onSuccess?: () => void }) => {
dispatch(
deleteAction({
id: action?.id ?? "",
name: action?.name ?? "",
onSuccess,
}),
);
},
[action.id, action.name, dispatch],
ankitakinger marked this conversation as resolved.
Show resolved Hide resolved
);

return { handleDeleteClick };
}

export { useHandleDeleteClick };
21 changes: 21 additions & 0 deletions app/client/src/ce/PluginActionEditor/hooks/useHandleRunClick.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { runAction } from "actions/pluginActionActions";
import type { PaginationField } from "api/ActionAPI";
import { usePluginActionContext } from "PluginActionEditor/PluginActionContext";
import { useCallback } from "react";
import { useDispatch } from "react-redux";

function useHandleRunClick() {
const { action } = usePluginActionContext();
const dispatch = useDispatch();

const handleRunClick = useCallback(
(paginationField?: PaginationField) => {
dispatch(runAction(action?.id ?? "", paginationField));
},
[action.id, dispatch],
);

return { handleRunClick };
}

export { useHandleRunClick };
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ import {
import { useDispatch, useSelector } from "react-redux";
import {
copyActionRequest,
deleteAction,
moveActionRequest,
} from "actions/pluginActionActions";
import { getCurrentPageId } from "selectors/editorSelectors";
import type { Page } from "entities/Page";
import { getPageList } from "ee/selectors/entitiesSelector";
import { ConvertToModuleCTA } from "./ConvertToModule";
import { useHandleDeleteClick } from "PluginActionEditor/hooks";

const PageMenuItem = (props: {
page: Page;
Expand Down Expand Up @@ -126,18 +126,16 @@ const Move = () => {
};

const Delete = () => {
const dispatch = useDispatch();
const { action } = usePluginActionContext();

const { handleDeleteClick } = useHandleDeleteClick();
const [confirmDelete, setConfirmDelete] = useState(false);

const deleteActionFromPage = useCallback(() => {
dispatch(deleteAction({ id: action.id, name: action.name }));
}, [action.id, action.name, dispatch]);

const handleSelect = useCallback(() => {
confirmDelete ? deleteActionFromPage() : setConfirmDelete(true);
}, [confirmDelete, deleteActionFromPage]);
const handleSelect = useCallback(
(e?: Event) => {
e?.preventDefault();
ankitakinger marked this conversation as resolved.
Show resolved Hide resolved
confirmDelete ? handleDeleteClick({}) : setConfirmDelete(true);
ankitakinger marked this conversation as resolved.
Show resolved Hide resolved
},
[confirmDelete, handleDeleteClick],
);

const menuLabel = confirmDelete
? createMessage(CONFIRM_CONTEXT_DELETE)
Expand Down
1 change: 0 additions & 1 deletion app/client/src/ce/utils/analyticsUtilTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export type EventName =
| "RUN_API_CLICK"
| "RUN_API_SHORTCUT"
| "DELETE_API"
| "DELETE_API_CLICK"
| "IMPORT_API"
| "EXPAND_API"
| "IMPORT_API_CLICK"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "ce/PluginActionEditor/hooks/useHandleDeleteClick";
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "ce/PluginActionEditor/hooks/useHandleRunClick";
4 changes: 0 additions & 4 deletions app/client/src/pages/Editor/APIEditor/ApiEditorContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type { SaveActionNameParams } from "PluginActionEditor";

interface ApiEditorContextContextProps {
moreActionsMenu?: React.ReactNode;
handleDeleteClick: () => void;
handleRunClick: (paginationField?: PaginationField) => void;
actionRightPaneBackLink?: React.ReactNode;
// TODO: Fix this the next time the file is edited
Expand All @@ -30,7 +29,6 @@ export function ApiEditorContextProvider({
actionRightPaneAdditionSections,
actionRightPaneBackLink,
children,
handleDeleteClick,
handleRunClick,
moreActionsMenu,
notification,
Expand All @@ -42,7 +40,6 @@ export function ApiEditorContextProvider({
() => ({
actionRightPaneAdditionSections,
actionRightPaneBackLink,
handleDeleteClick,
showRightPaneTabbedSection,
handleRunClick,
moreActionsMenu,
Expand All @@ -53,7 +50,6 @@ export function ApiEditorContextProvider({
[
actionRightPaneBackLink,
actionRightPaneAdditionSections,
handleDeleteClick,
showRightPaneTabbedSection,
handleRunClick,
moreActionsMenu,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ export interface CommonFormProps {
actionResponse?: ActionResponse;
pluginId: string;
onRunClick: (paginationField?: PaginationField) => void;
onDeleteClick: () => void;
isRunning: boolean;
isDeleting: boolean;
paginationType: PaginationType;
Expand Down
2 changes: 0 additions & 2 deletions app/client/src/pages/Editor/APIEditor/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ class ApiEditor extends React.Component<Props> {
}
isDeleting={isDeleting}
isRunning={isRunning}
onDeleteClick={this.context.handleDeleteClick}
onRunClick={this.context.handleRunClick}
paginationType={paginationType}
pluginId={pluginId}
Expand All @@ -200,7 +199,6 @@ class ApiEditor extends React.Component<Props> {
isDeleting={isDeleting}
isRunning={isRunning}
match={this.props.match}
onDeleteClick={this.context.handleDeleteClick}
onRunClick={this.context.handleRunClick}
paginationType={paginationType}
pluginId={pluginId}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ function GraphQLEditorForm(props: Props) {
<Pagination
actionName={actionName}
formName={FORM_NAME}
onTestClick={props.onRunClick}
paginationType={props.paginationType}
query={props.actionConfigurationBody}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const PAGINATION_PREFIX =

interface PaginationProps {
actionName: string;
onTestClick: (test?: "PREV" | "NEXT") => void;
paginationType: PaginationType;
theme?: EditorTheme;
query: string;
Expand Down
16 changes: 1 addition & 15 deletions app/client/src/pages/Editor/APIEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ import {
getPluginSettingConfigs,
getPlugins,
} from "ee/selectors/entitiesSelector";
import {
deleteAction,
runAction,
saveActionName,
} from "actions/pluginActionActions";
import { runAction, saveActionName } from "actions/pluginActionActions";
import AnalyticsUtil from "ee/utils/AnalyticsUtil";
import Editor from "./Editor";
import BackToCanvas from "components/common/BackToCanvas";
Expand Down Expand Up @@ -162,15 +158,6 @@ function ApiEditorWrapper(props: ApiEditorWrapperProps) {
return <BackToCanvas basePageId={basePageId} />;
}, [basePageId]);

const handleDeleteClick = useCallback(() => {
AnalyticsUtil.logEvent("DELETE_API_CLICK", {
apiName,
apiID: action?.id,
pageName,
});
dispatch(deleteAction({ id: action?.id ?? "", name: apiName }));
}, [pages, basePageId, apiName, action?.id, dispatch, pageName]);

const notification = useMemo(() => {
if (!isConverting) return null;

Expand All @@ -188,7 +175,6 @@ function ApiEditorWrapper(props: ApiEditorWrapperProps) {
return (
<ApiEditorContextProvider
actionRightPaneBackLink={actionRightPaneBackLink}
handleDeleteClick={handleDeleteClick}
handleRunClick={handleRunClick}
moreActionsMenu={moreActionsMenu}
notification={notification}
Expand Down
Loading