Skip to content

Commit

Permalink
chore: refactor query duplication flow (#36915)
Browse files Browse the repository at this point in the history
## Description
This PR refactors the action duplication saga and action calls to remove
dependency on pageID. As far as CE code is concerned, this PR doesn't
change any functionality for the end user. Those changes are done for
workflows editor in EE.


Fixes #36886

## Automation

/test all

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11462274307>
> Commit: 04dfbfb
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11462274307&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Tue, 22 Oct 2024 16:21:32 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## 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

- **New Features**
- Introduced a new hook, `useHandleDuplicateClick`, enhancing action
duplication functionality.
- Added a new interface and function for generating destination ID
information.

- **Bug Fixes**
- Updated action request structure to use `destinationEntityId` for
consistency across components.

- **Documentation**
	- Enhanced success message flexibility for action copy notifications.

- **Tests**
- Added unit tests for the `ActionEntityContextMenu` component to ensure
proper functionality and rendering.

- **Refactor**
- Improved context menu handling based on entity types for better user
experience.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
ayushpahwa authored Oct 24, 2024
1 parent 23a5772 commit 0b93768
Show file tree
Hide file tree
Showing 10 changed files with 304 additions and 43 deletions.
1 change: 1 addition & 0 deletions app/client/src/PluginActionEditor/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export { useActionSettingsConfig } from "ee/PluginActionEditor/hooks/useActionSettingsConfig";
export { useHandleDeleteClick } from "ee/PluginActionEditor/hooks/useHandleDeleteClick";
export { useHandleDuplicateClick } from "./useHandleDuplicateClick";
export { useHandleRunClick } from "ee/PluginActionEditor/hooks/useHandleRunClick";
export { useBlockExecution } from "ee/PluginActionEditor/hooks/useBlockExecution";
export { useAnalyticsOnRunClick } from "ee/PluginActionEditor/hooks/useAnalyticsOnRunClick";
26 changes: 26 additions & 0 deletions app/client/src/PluginActionEditor/hooks/useHandleDuplicateClick.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { copyActionRequest } from "actions/pluginActionActions";
import { usePluginActionContext } from "PluginActionEditor/PluginActionContext";
import { useCallback } from "react";
import { useDispatch } from "react-redux";

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

const handleDuplicateClick = useCallback(
(destinationEntityId: string) => {
dispatch(
copyActionRequest({
id: action.id,
destinationEntityId,
name: action.name,
}),
);
},
[action.id, action.name, dispatch],
);

return { handleDuplicateClick };
}

export { useHandleDuplicateClick };
5 changes: 3 additions & 2 deletions app/client/src/actions/pluginActionActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type { ApiResponse } from "api/ApiResponses";
import type { JSCollection } from "entities/JSCollection";
import type { ErrorActionPayload } from "sagas/ErrorSagas";
import type { EventLocation } from "ee/utils/analyticsUtilTypes";
import type { GenerateDestinationIdInfoReturnType } from "ee/sagas/helpers";

export const createActionRequest = (payload: Partial<Action>) => {
return {
Expand Down Expand Up @@ -225,7 +226,7 @@ export const moveActionError = (

export const copyActionRequest = (payload: {
id: string;
destinationPageId: string;
destinationEntityId: string;
name: string;
}) => {
return {
Expand All @@ -244,7 +245,7 @@ export const copyActionSuccess = (payload: Action) => {
export const copyActionError = (
payload: {
id: string;
destinationPageId: string;
destinationEntityIdInfo: GenerateDestinationIdInfoReturnType;
} & ErrorActionPayload,
) => {
return {
Expand Down
3 changes: 2 additions & 1 deletion app/client/src/ce/constants/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export const ACTION_MOVE_SUCCESS = (actionName: string, pageName: string) =>
export const ERROR_ACTION_MOVE_FAIL = (actionName: string) =>
`Error while moving action ${actionName}`;
export const ACTION_COPY_SUCCESS = (actionName: string, pageName: string) =>
`${actionName} action copied to page ${pageName} successfully`;
`${actionName} action copied ${pageName.length > 0 ? "to page " + pageName : ""} successfully`;
export const ERROR_ACTION_COPY_FAIL = (actionName: string) =>
`Error while copying action ${actionName}`;
export const ERROR_ACTION_RENAME_FAIL = (actionName: string) =>
Expand Down Expand Up @@ -1731,6 +1731,7 @@ export const CONTEXT_RENAME = () => "Rename";
export const CONTEXT_SHOW_BINDING = () => "Show bindings";
export const CONTEXT_MOVE = () => "Move to page";
export const CONTEXT_COPY = () => "Copy to page";
export const CONTEXT_DUPLICATE = () => "Duplicate";
export const CONTEXT_DELETE = () => "Delete";
export const CONFIRM_CONTEXT_DELETE = () => "Are you sure?";
export const CONFIRM_CONTEXT_DELETING = () => "Deleting";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const Copy = () => {
dispatch(
copyActionRequest({
id: action.id,
destinationPageId: pageId,
destinationEntityId: pageId,
name: action.name,
}),
),
Expand Down
17 changes: 17 additions & 0 deletions app/client/src/ce/sagas/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,23 @@ export interface ResolveParentEntityMetadataReturnType {
parentEntityKey?: CreateNewActionKeyInterface;
}

// This function is extended in EE. Please check the EE implementation before any modification.
export interface GenerateDestinationIdInfoReturnType {
pageId?: string;
}

// This function is extended in EE. Please check the EE implementation before any modification.
export function generateDestinationIdInfoForQueryDuplication(
destinationEntityId: string,
parentEntityKey: CreateNewActionKeyInterface,
): GenerateDestinationIdInfoReturnType {
if (parentEntityKey === CreateNewActionKey.PAGE) {
return { pageId: destinationEntityId };
}

return {};
}

// This function is extended in EE. Please check the EE implementation before any modification.
export const resolveParentEntityMetadata = (
action: Partial<Action>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import React from "react";
import { fireEvent, render, waitFor } from "@testing-library/react";
import "@testing-library/jest-dom/extend-expect";
import { Provider } from "react-redux";
import { lightTheme } from "selectors/themeSelectors";
import { ThemeProvider } from "styled-components";
import configureStore from "redux-mock-store";
import { PluginType } from "entities/Action";
import {
ActionEntityContextMenu,
type EntityContextMenuProps,
} from "./ActionEntityContextMenu";
import { FilesContextProvider } from "../Files/FilesContextProvider";
import { ActionParentEntityType } from "ee/entities/Engine/actionHelpers";
import { act } from "react-dom/test-utils";
import {
CONTEXT_COPY,
CONTEXT_DELETE,
CONTEXT_MOVE,
CONTEXT_RENAME,
CONTEXT_SHOW_BINDING,
createMessage,
} from "ee/constants/messages";
import {
ReduxActionTypes,
type ReduxAction,
} from "ee/constants/ReduxActionConstants";

const mockStore = configureStore([]);

const page1Id = "605c435a91dea93f0eaf91ba";
const page2Id = "605c435a91dea93f0eaf91bc";
const basePage2Id = "605c435a91dea93f0eaf91bc";
const defaultState = {
ui: {
selectedWorkspace: {
workspace: {},
},
workspaces: {
packagesList: [],
},
users: {
featureFlag: {
data: {},
},
},
},
entities: {
actions: [],
pageList: {
pages: [
{
pageId: page1Id,
basePageId: page2Id,
pageName: "Page1",
isDefault: true,
slug: "page-1",
},
{
pageId: page2Id,
basePageId: basePage2Id,
pageName: "Page2",
isDefault: false,
slug: "page-2",
},
],
},
},
};

const defaultProps: EntityContextMenuProps = {
id: "test-action-id",
name: "test-action",
canManageAction: true,
canDeleteAction: true,
pluginType: PluginType.DB,
};

const defaultContext = {
editorId: "test-editor-id",
canCreateActions: true,
parentEntityId: "test-parent-entity-id",
parentEntityType: ActionParentEntityType.PAGE,
};

describe("ActionEntityContextMenu", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let store: any;

beforeEach(() => {
jest.clearAllMocks();
});

it("renders context menu with correct options for application editor", async () => {
store = mockStore(defaultState);
const { findByText, getByRole } = render(
<Provider store={store}>
<ThemeProvider theme={lightTheme}>
<FilesContextProvider {...defaultContext}>
<ActionEntityContextMenu {...defaultProps} />
</FilesContextProvider>
</ThemeProvider>
</Provider>,
);
const triggerToOpenMenu = getByRole("button");

act(() => {
fireEvent.click(triggerToOpenMenu);
});

await waitFor(() => {
expect(triggerToOpenMenu.parentNode).toHaveAttribute(
"aria-expanded",
"true",
);
});

// In context of pages, the copy to page option will be shown
const copyQueryToPageOptions = await findByText(
createMessage(CONTEXT_COPY),
);

expect(await findByText(createMessage(CONTEXT_RENAME))).toBeInTheDocument();
expect(await findByText(createMessage(CONTEXT_DELETE))).toBeInTheDocument();
expect(await findByText(createMessage(CONTEXT_MOVE))).toBeInTheDocument();
expect(
await findByText(createMessage(CONTEXT_SHOW_BINDING)),
).toBeInTheDocument();
expect(copyQueryToPageOptions).toBeInTheDocument();

// Now we click on the copy to page option
act(() => {
fireEvent.click(copyQueryToPageOptions);
});

// Now a menu with the list of pages will show up
const copyQueryToPageSubOptionPage1 = await findByText("Page1");

expect(copyQueryToPageSubOptionPage1).toBeInTheDocument();
expect(await findByText("Page2")).toBeInTheDocument();

// Clicking on the page will trigger the correct action
act(() => {
fireEvent.click(copyQueryToPageSubOptionPage1);
});

let actions: Array<
ReduxAction<{
payload: {
id: string;
destinationEntityId: string;
name: string;
};
}>
> = [];

await waitFor(() => {
actions = store.getActions();
});

expect(actions.length).toBe(1);
expect(actions[0].type).toBe(ReduxActionTypes.COPY_ACTION_INIT);
expect(actions[0].payload).toEqual({
destinationEntityId: page1Id,
id: "test-action-id",
name: "test-action",
});
});
// TODO: add tests for all options rendered in the context menu
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
CONTEXT_NO_PAGE,
CONTEXT_SHOW_BINDING,
createMessage,
CONTEXT_DUPLICATE,
} from "ee/constants/messages";
import { builderURL } from "ee/RouteBuilder";

Expand All @@ -33,8 +34,9 @@ import { useConvertToModuleOptions } from "ee/pages/Editor/Explorer/hooks";
import { MODULE_TYPE } from "ee/constants/ModuleConstants";
import { PluginType } from "entities/Action";
import { convertToBaseParentEntityIdSelector } from "selectors/pageListSelectors";
import { ActionParentEntityType } from "ee/entities/Engine/actionHelpers";

interface EntityContextMenuProps {
export interface EntityContextMenuProps {
id: string;
name: string;
className?: string;
Expand All @@ -45,20 +47,20 @@ interface EntityContextMenuProps {
export function ActionEntityContextMenu(props: EntityContextMenuProps) {
// Import the context
const context = useContext(FilesContext);
const { menuItems, parentEntityId } = context;
const { menuItems, parentEntityId, parentEntityType } = context;
const baseParentEntityId = useSelector((state) =>
convertToBaseParentEntityIdSelector(state, parentEntityId),
);

const { canDeleteAction, canManageAction } = props;
const dispatch = useDispatch();
const [confirmDelete, setConfirmDelete] = useState(false);
const copyActionToPage = useCallback(
(actionId: string, actionName: string, pageId: string) =>
const copyAction = useCallback(
(actionId: string, actionName: string, destinationEntityId: string) =>
dispatch(
copyActionRequest({
id: actionId,
destinationPageId: pageId,
destinationEntityId,
name: actionName,
}),
),
Expand Down Expand Up @@ -129,14 +131,26 @@ export function ActionEntityContextMenu(props: EntityContextMenuProps) {
menuItems.includes(ActionEntityContextMenuItemsEnum.COPY) &&
canManageAction && {
value: "copy",
onSelect: noop,
label: createMessage(CONTEXT_COPY),
children: menuPages.map((page) => {
return {
...page,
onSelect: () => copyActionToPage(props.id, props.name, page.id),
};
}),
onSelect:
parentEntityType === ActionParentEntityType.PAGE
? noop
: () => {
copyAction(props.id, props.name, parentEntityId);
},
label: createMessage(
parentEntityType === ActionParentEntityType.PAGE
? CONTEXT_COPY
: CONTEXT_DUPLICATE,
),
children:
parentEntityType === ActionParentEntityType.PAGE &&
menuPages.length > 0 &&
menuPages.map((page) => {
return {
...page,
onSelect: () => copyAction(props.id, props.name, page.id),
};
}),
},
menuItems.includes(ActionEntityContextMenuItemsEnum.MOVE) &&
canManageAction && {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export function MoreActionsMenu(props: EntityContextMenuProps) {
dispatch(
copyActionRequest({
id: actionId,
destinationPageId: pageId,
destinationEntityId: pageId,
name: actionName,
}),
),
Expand Down
Loading

0 comments on commit 0b93768

Please sign in to comment.