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: [Plugin Action Editor] Combine Plugin Editor UI state #36651

Merged
merged 21 commits into from
Oct 8, 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 @@ -24,7 +24,7 @@ describe("Widget error state", { tags: ["@tag.Widget"] }, function () {
//Check if the current value is shown in the debugger

_.debuggerHelper.OpenDebugger();
cy.get("[data-testid=t--tab-ERROR]").click();
cy.get("[data-testid=t--tab-ERROR_TAB]").click();
//This feature is disabled in updated error log - epic 17720
// _.debuggerHelper.LogStateContains("Test");
});
Expand All @@ -37,7 +37,7 @@ describe("Widget error state", { tags: ["@tag.Widget"] }, function () {
cy.get(widgetLocators.buttonWidget).click();

cy.get(".t--toast-debug-button").click();
cy.get("[data-testid='t--tab-ERROR']").should(
cy.get("[data-testid='t--tab-ERROR_TAB']").should(
"have.attr",
"aria-selected",
"true",
Expand Down
2 changes: 1 addition & 1 deletion app/client/cypress/locators/commonlocators.json
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
"labelSectionTxt": ".CodeMirror-code .cm-variable",
"lintError": ".CodeMirror-lint-mark-error",
"debugger": ".t--debugger-count",
"errorTab": "[data-testid=t--tab-ERROR]",
"errorTab": "[data-testid=t--tab-ERROR_TAB]",
"debugErrorMsg": "[data-testid=t--debugger-log-message]",
"tableButtonVariant": ".t--property-control-buttonvariant",
"debuggerLabel": "span.debugger-label",
Expand Down
2 changes: 1 addition & 1 deletion app/client/cypress/support/ApiCommands.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ Cypress.Commands.add(
}
cy.get(".string-value").contains(baseurl.concat(path));
cy.get(".string-value").contains(verb);
cy.get("[data-testid=t--tab-response]").first().click({ force: true });
cy.get("[data-testid=t--tab-RESPONSE_TAB]").first().click({ force: true });
},
);

Expand Down
4 changes: 2 additions & 2 deletions app/client/cypress/support/Objects/CommonLocators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ export class CommonLocators {
_createNew = ".t--add-item";
_uploadFiles = "div.uppy-Dashboard-AddFiles input";
_uploadBtn = "button.uppy-StatusBar-actionBtn--upload";
_errorTab = "[data-testid=t--tab-ERROR]";
_responseTab = "[data-testid=t--tab-response]";
_errorTab = "[data-testid=t--tab-ERROR_TAB]";
_responseTab = "[data-testid=t--tab-RESPONSE_TAB]";
_modal = ".t--modal-widget";
_closeModal = "button:contains('Close')";
_entityProperties = (entityNameinLeftSidebar: string) =>
Expand Down
2 changes: 1 addition & 1 deletion app/client/cypress/support/Pages/ApiPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class ApiPage {
private _paginationTypeLabels = ".t--apiFormPaginationType label";
_saveAsDS = ".t--store-as-datasource";
_responseStatus = ".t--response-status-code";
public _responseTabHeader = "[data-testid=t--tab-headers]";
public _responseTabHeader = "[data-testid=t--tab-HEADERS_TAB]";
public _headersTabContent = ".t--headers-tab";
public _autoGeneratedHeaderInfoIcon = (key: string) =>
`.t--auto-generated-${key}-info`;
Expand Down
4 changes: 2 additions & 2 deletions app/client/cypress/support/Pages/DataSources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export class DataSources {
".t--datasource-name:contains('" + dsName + "')";
_mandatoryMark = "//span[text()='*']";
_deleteDSHostPort = ".t--delete-field";
_dsTabSchema = "[data-testid='t--tab-schema']";
_dsTabSchema = "[data-testid='t--tab-SCHEMA_TAB']";
private _pageSelectionMenu = "[data-testId='t--page-selection']";

private _pageSelectMenuItem = ".ads-v2-menu__menu-item";
Expand Down Expand Up @@ -1854,7 +1854,7 @@ export class DataSources {
cy.intercept("GET", "/api/v1/datasources/*/structure?ignoreCache=*").as(
`getDatasourceStructureUpdated_${ds_entity_name}`,
);
cy.get("[data-testid=t--tab-schema]").first().click({ force: true });
cy.get("[data-testid=t--tab-SCHEMA_TAB]").first().click({ force: true });
this.RefreshDatasourceSchema();
this.assertHelper
.WaitForNetworkCall(`@getDatasourceStructureUpdated_${ds_entity_name}`)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
import { useCallback } from "react";
import { useDispatch, useSelector } from "react-redux";
import { getApiPaneConfigSelectedTabIndex } from "selectors/apiPaneSelectors";
import { API_EDITOR_TABS } from "constants/ApiEditorConstants/CommonApiConstants";
import { setApiPaneConfigSelectedTabIndex } from "actions/apiPaneActions";
import {
getPluginActionConfigSelectedTab,
setPluginActionEditorSelectedTab,
} from "PluginActionEditor/store";

export function useSelectedFormTab(): [string, (id: string) => void] {
export function useSelectedFormTab(): [
string | undefined,
(id: string) => void,
] {
const dispatch = useDispatch();
// the redux form has been configured with indexes, but the new ads components need strings to work.
// these functions convert them back and forth as needed.
const selectedIndex = useSelector(getApiPaneConfigSelectedTabIndex);
const selectedValue = Object.values(API_EDITOR_TABS)[selectedIndex];
const setSelectedIndex = useCallback(
const selectedValue = useSelector(getPluginActionConfigSelectedTab);
const setSelectedTab = useCallback(
(value: string) => {
const index = Object.values(API_EDITOR_TABS).indexOf(
value as API_EDITOR_TABS,
);

dispatch(setApiPaneConfigSelectedTabIndex(index));
dispatch(setPluginActionEditorSelectedTab(value));
},
[dispatch],
);

return [selectedValue, setSelectedIndex];
return [selectedValue, setSelectedTab];
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,16 @@ import { renderHook } from "@testing-library/react-hooks/dom";
import { useDispatch } from "react-redux";
import { PluginType } from "entities/Action";
import { usePluginActionContext } from "PluginActionEditor";
import { changeApi } from "actions/apiPaneActions";
import { changeQuery } from "actions/queryPaneActions";
import { changeApi, changeQuery } from "../../../store";
ankitakinger marked this conversation as resolved.
Show resolved Hide resolved
import usePrevious from "utils/hooks/usePrevious";
import { useChangeActionCall } from "./useChangeActionCall";

jest.mock("react-redux", () => ({
useDispatch: jest.fn(),
}));

jest.mock("actions/apiPaneActions", () => ({
jest.mock("../../../store", () => ({
changeApi: jest.fn(),
}));

jest.mock("actions/queryPaneActions", () => ({
changeQuery: jest.fn(),
}));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { useEffect } from "react";
import { useDispatch } from "react-redux";
import { changeApi } from "actions/apiPaneActions";
import { changeQuery } from "actions/queryPaneActions";
import { PluginType } from "entities/Action";
import { usePluginActionContext } from "PluginActionEditor";
import { changeApi, changeQuery } from "PluginActionEditor/store";
import usePrevious from "utils/hooks/usePrevious";

export const useChangeActionCall = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { IDEBottomView, ViewHideBehaviour } from "IDE";
import { ActionExecutionResizerHeight } from "pages/Editor/APIEditor/constants";
import EntityBottomTabs from "components/editorComponents/EntityBottomTabs";
import { useDispatch, useSelector } from "react-redux";
import { getApiPaneDebuggerState } from "selectors/apiPaneSelectors";
import { setApiPaneDebuggerState } from "actions/apiPaneActions";
import { DEBUGGER_TAB_KEYS } from "components/editorComponents/Debugger/helpers";
import { setPluginActionEditorDebuggerState } from "../../store";
import { getPluginActionDebuggerState } from "../../store";
ankitakinger marked this conversation as resolved.
Show resolved Hide resolved
import { DEBUGGER_TAB_KEYS } from "components/editorComponents/Debugger/constants";
import AnalyticsUtil from "ee/utils/AnalyticsUtil";
import { usePluginActionResponseTabs } from "./hooks";

Expand All @@ -16,11 +16,11 @@ function PluginActionResponse() {

// TODO combine API and Query Debugger state
const { open, responseTabHeight, selectedTab } = useSelector(
getApiPaneDebuggerState,
getPluginActionDebuggerState,
);

const toggleHide = useCallback(
() => dispatch(setApiPaneDebuggerState({ open: !open })),
() => dispatch(setPluginActionEditorDebuggerState({ open: !open })),
[dispatch, open],
);

Expand All @@ -32,14 +32,18 @@ function PluginActionResponse() {
});
}

dispatch(setApiPaneDebuggerState({ open: true, selectedTab: tabKey }));
dispatch(
setPluginActionEditorDebuggerState({ open: true, selectedTab: tabKey }),
);
},
[dispatch],
);

const updateResponsePaneHeight = useCallback(
(height: number) => {
dispatch(setApiPaneDebuggerState({ responseTabHeight: height }));
dispatch(
setPluginActionEditorDebuggerState({ responseTabHeight: height }),
);
},
[dispatch],
);
Expand Down
1 change: 1 addition & 0 deletions app/client/src/PluginActionEditor/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const POST_BODY_FORM_DATA_KEY = "displayFormat";
2 changes: 2 additions & 0 deletions app/client/src/PluginActionEditor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export type {
PluginActionNameEditorProps,
} from "./components/PluginActionNameEditor";
export { default as PluginActionNameEditor } from "./components/PluginActionNameEditor";

export type { PluginActionEditorState } from "./store/pluginEditorReducer";
4 changes: 4 additions & 0 deletions app/client/src/PluginActionEditor/store/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export { default as pluginActionReducer } from "./pluginEditorReducer";

export * from "./pluginActionEditorActions";
export * from "./pluginActionEditorSelectors";
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import type { PluginEditorDebuggerState } from "./pluginEditorReducer";
Copy link
Contributor

Choose a reason for hiding this comment

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

store
actions
reducer
selector

^ is this folder pattern we are establishing to use/refactor consistently everywhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. There are so many other flows which we are not handling in Action redesign (like admin settings), won't the folder structure for those be different?

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 want to make sure the store is near the feature and this makes sense to me. This would be a gradual change as we start making more modular features. @ankitakinger can you share why this does not work ? Please suggest any alternatives you can think of

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not saying it won't work. My point is about consistency, since we are not going to touch other flows.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to Hetu, much easier to consume and refactor components if all related items in a domain level package. One question though, why not bring sagas into this? Also, let's publish this expectation to wider team also.

import {
type ReduxAction,
ReduxActionTypes,
} from "ee/constants/ReduxActionConstants";
import type { Action } from "entities/Action";

export const setPluginActionEditorDebuggerState = (
payload: Partial<PluginEditorDebuggerState>,
) => ({
type: ReduxActionTypes.SET_PLUGIN_ACTION_EDITOR_DEBUGGER_STATE,
payload,
});

export const setPluginActionEditorSelectedTab = (payload: string) => ({
type: ReduxActionTypes.SET_PLUGIN_ACTION_EDITOR_FORM_SELECTED_TAB,
payload: {
selectedTab: payload,
},
});

export const updatePostBodyContentType = (
title: string,
apiId: string,
): ReduxAction<{ title: string; apiId: string }> => ({
type: ReduxActionTypes.UPDATE_API_ACTION_BODY_CONTENT_TYPE,
payload: { title, apiId },
});

export const changeApi = (
id: string,
isSaas: boolean,
newApi?: boolean,
): ReduxAction<{ id: string; isSaas: boolean; newApi?: boolean }> => {
return {
type: ReduxActionTypes.API_PANE_CHANGE_API,
payload: { id, isSaas, newApi },
};
};

export interface ChangeQueryPayload {
baseQueryId: string;
packageId?: string;
applicationId?: string;
basePageId?: string;
moduleId?: string;
workflowId?: string;
newQuery?: boolean;
action?: Action;
}

export const changeQuery = (payload: ChangeQueryPayload) => {
return {
type: ReduxActionTypes.QUERY_PANE_CHANGE,
payload,
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import type { AppState } from "ee/reducers";
import { createSelector } from "reselect";

import { POST_BODY_FORM_DATA_KEY } from "../constants";

export const getActionEditorSavingMap = (state: AppState) =>
state.ui.pluginActionEditor.isSaving;
Comment on lines +6 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider Adding Explicit Return Types to Exported Functions for Clarity

It's beneficial to explicitly specify return types for your exported functions. This practice enhances code readability and type safety, making it clearer for others (and your future self) to understand what each function returns. It can also help catch unintended type errors during development.

For example, you can define the return type of getActionEditorSavingMap as Record<string, boolean>:

-export const getActionEditorSavingMap = (state: AppState) =>
+export const getActionEditorSavingMap = (state: AppState): Record<string, boolean> =>
  state.ui.pluginActionEditor.isSaving;
📝 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
export const getActionEditorSavingMap = (state: AppState) =>
state.ui.pluginActionEditor.isSaving;
export const getActionEditorSavingMap = (state: AppState): Record<string, boolean> =>
state.ui.pluginActionEditor.isSaving;


export const isActionSaving = (id: string) =>
createSelector([getActionEditorSavingMap], (savingMap) => {
return id in savingMap && savingMap[id];
});

const getActionDirtyState = (state: AppState) =>
state.ui.pluginActionEditor.isDirty;

export const isActionDirty = (id: string) =>
createSelector([getActionDirtyState], (actionDirtyMap) => {
return id in actionDirtyMap && actionDirtyMap[id];
});

const getActionRunningState = (state: AppState) =>
state.ui.pluginActionEditor.isRunning;

export const isActionRunning = (id: string) =>
createSelector(
[getActionRunningState],
(isRunningMap) => id in isRunningMap && isRunningMap[id],
);

const getActionDeletingState = (state: AppState) =>
state.ui.pluginActionEditor.isDeleting;

export const isActionDeleting = (id: string) =>
createSelector(
[getActionDeletingState],
(deletingMap) => id in deletingMap && deletingMap[id],
);

type GetFormData = (
state: AppState,
id: string,
) => { label: string; value: string } | undefined;

export const getPostBodyFormat: GetFormData = (state, id) => {
const formData = state.ui.pluginActionEditor.formData;

if (id in formData) {
return formData[id][POST_BODY_FORM_DATA_KEY];
}

return undefined;
};
export const getPluginActionConfigSelectedTab = (state: AppState) =>
state.ui.pluginActionEditor.selectedConfigTab;

export const getPluginActionDebuggerState = (state: AppState) =>
state.ui.pluginActionEditor.debugger;

export const isPluginActionCreating = (state: AppState) =>
state.ui.pluginActionEditor.isCreating;
Loading
Loading