From dee0a54227f85072f2e61cd7cec39d6252d3f21e Mon Sep 17 00:00:00 2001 From: arunvjn <32433245+arunvjn@users.noreply.github.com> Date: Wed, 22 Nov 2023 14:04:48 +0530 Subject: [PATCH] fix: Preserve query execution data on name refactor (#28973) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Code changes to preserve execution data when a Query/JS Object is renamed. > #### PR fixes following issue(s) Fixes #28985 > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change - Bug fix (non-breaking change which fixes an issue) > > > ## Testing > #### How Has This Been Tested? - [x] Manual - [x] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../ClientSide/BugTests/Bug28985_spec.ts | 88 +++++++++++++++++++ app/client/src/actions/pluginActionActions.ts | 23 ++--- .../ActionExecution/ActionExecutionSagas.ts | 12 +-- app/client/src/ce/sagas/JSActionSagas.ts | 19 +++- .../sagas/ActionExecution/PluginActionSaga.ts | 74 +++++++++------- app/client/src/sagas/ActionSagas.ts | 11 +++ app/client/src/sagas/OnboardingSagas.ts | 12 +-- .../Evaluation/handlers/updateActionData.ts | 11 ++- 8 files changed, 190 insertions(+), 60 deletions(-) create mode 100644 app/client/cypress/e2e/Regression/ClientSide/BugTests/Bug28985_spec.ts diff --git a/app/client/cypress/e2e/Regression/ClientSide/BugTests/Bug28985_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/BugTests/Bug28985_spec.ts new file mode 100644 index 0000000000..21bca5d8da --- /dev/null +++ b/app/client/cypress/e2e/Regression/ClientSide/BugTests/Bug28985_spec.ts @@ -0,0 +1,88 @@ +import * as _ from "../../../../support/Objects/ObjectsCore"; + +describe("Api execution results test cases", () => { + it("1. Check to see if API execution results are preserved after it is renamed", () => { + const { + agHelper, + apiPage, + dataManager, + entityExplorer, + jsEditor, + propPane, + } = _; + // Drag and drop a button widget + entityExplorer.DragDropWidgetNVerify(_.draggableWidgets.BUTTON, 200, 200); + entityExplorer.DragDropWidgetNVerify(_.draggableWidgets.BUTTON, 400, 400); + + entityExplorer.NavigateToSwitcher("Explorer"); + // Create a new API + apiPage.CreateAndFillApi( + dataManager.dsValues[dataManager.defaultEnviorment].mockApiUrl, + ); + + apiPage.RunAPI(); + + jsEditor.CreateJSObject( + `export default { + async func() { + return Api1.data + } + }`, + { + paste: true, + completeReplace: true, + toRun: true, + shouldCreateNewJSObj: true, + prettify: true, + }, + ); + + entityExplorer.SelectEntityByName("Button1"); + + // Set the label to the button + propPane.TypeTextIntoField( + "label", + "{{Api1.data ? 'Success 1' : 'Failed 1'}}", + ); + + propPane.ToggleJSMode("onClick", true); + + propPane.TypeTextIntoField("onClick", "{{showAlert('Successful')}}"); + + agHelper.ClickButton("Success 1"); + + agHelper.ValidateToastMessage("Successful"); + + entityExplorer.RenameEntityFromExplorer("Api1", "Api123"); + + entityExplorer.SelectEntityByName("Button1"); + + agHelper.ClickButton("Success 1"); + + agHelper.ValidateToastMessage("Successful"); + + entityExplorer.SelectEntityByName("Button2"); + + // Set the label to the button + propPane.TypeTextIntoField( + "label", + "{{JSObject1.func.data ? 'Success 2' : 'Failed 2'}}", + ); + + propPane.ToggleJSMode("onClick", true); + + propPane.TypeTextIntoField("onClick", "{{showAlert('Successful')}}"); + + agHelper.ClickButton("Success 2"); + + agHelper.ValidateToastMessage("Successful"); + + entityExplorer.RenameEntityFromExplorer("JSObject1", "JSObject123"); + + entityExplorer.SelectEntityByName("Button2"); + + agHelper.ClickButton("Success 2"); + + agHelper.ValidateToastMessage("Successful"); + }); +}); diff --git a/app/client/src/actions/pluginActionActions.ts b/app/client/src/actions/pluginActionActions.ts index 616d221f15..63bc11a30d 100644 --- a/app/client/src/actions/pluginActionActions.ts +++ b/app/client/src/actions/pluginActionActions.ts @@ -340,22 +340,17 @@ export const bindDataOnCanvas = (payload: { }; }; -export const updateActionData = ({ - data, - dataPath, - entityName, -}: { - entityName: string; - dataPath: string; - data: unknown; -}) => { +export const updateActionData = ( + payload: { + entityName: string; + dataPath: string; + data: unknown; + dataPathRef?: string; + }[], +) => { return { type: ReduxActionTypes.UPDATE_ACTION_DATA, - payload: { - entityName, - dataPath, - data, - }, + payload, }; }; diff --git a/app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts b/app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts index c2541d9720..fb9ce61d83 100644 --- a/app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts +++ b/app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts @@ -82,11 +82,13 @@ export function* executeActionTriggers( ); if (action) { yield put( - updateActionData({ - entityName: action.name, - dataPath: "data", - data: undefined, - }), + updateActionData([ + { + entityName: action.name, + dataPath: "data", + data: undefined, + }, + ]), ); } break; diff --git a/app/client/src/ce/sagas/JSActionSagas.ts b/app/client/src/ce/sagas/JSActionSagas.ts index 974b825360..b7a9b6ddee 100644 --- a/app/client/src/ce/sagas/JSActionSagas.ts +++ b/app/client/src/ce/sagas/JSActionSagas.ts @@ -7,7 +7,10 @@ import { ReduxActionTypes, } from "@appsmith/constants/ReduxActionConstants"; import { put, select, call } from "redux-saga/effects"; -import type { FetchActionsPayload } from "actions/pluginActionActions"; +import { + updateActionData, + type FetchActionsPayload, +} from "actions/pluginActionActions"; import type { JSAction, JSCollection } from "entities/JSCollection"; import { copyJSCollectionError, @@ -370,6 +373,20 @@ export function* refactorJSObjectName( actionId: id, }, }); + const jsObject: JSCollection = yield select((state) => + getJSCollection(state, id), + ); + const functions = jsObject.actions; + yield put( + updateActionData( + functions.map((f) => ({ + entityName: newName, + data: undefined, + dataPath: `${f.name}.data`, + dataPathRef: `${oldName}.${f.name}.data`, + })), + ), + ); if (currentPageId === pageId) { // @ts-expect-error: refactorResponse.data is of type unknown yield updateCanvasWithDSL(refactorResponse.data, pageId, layoutId); diff --git a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts index e0231fe9a0..7682124190 100644 --- a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts +++ b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts @@ -1175,11 +1175,13 @@ function* executePageLoadAction(pageAction: PageAction) { }), ); yield put( - updateActionData({ - entityName: action.name, - dataPath: "data", - data: payload.body, - }), + updateActionData([ + { + entityName: action.name, + dataPath: "data", + data: payload.body, + }, + ]), ); PerformanceTracker.stopAsyncTracking( PerformanceTransactionName.EXECUTE_ACTION, @@ -1230,11 +1232,13 @@ function* executePageLoadAction(pageAction: PageAction) { pageAction.id, ); yield put( - updateActionData({ - entityName: action.name, - dataPath: "data", - data: payload.body, - }), + updateActionData([ + { + entityName: action.name, + dataPath: "data", + data: payload.body, + }, + ]), ); yield take(ReduxActionTypes.SET_EVALUATED_TREE); } @@ -1392,11 +1396,13 @@ function* executePluginActionSaga( ); yield put( - updateActionData({ - entityName: pluginAction.name, - dataPath: "data", - data: isError ? undefined : payload.body, - }), + updateActionData([ + { + entityName: pluginAction.name, + dataPath: "data", + data: isError ? undefined : payload.body, + }, + ]), ); // TODO: Plugins are not always fetched before on page load actions are executed. try { @@ -1451,11 +1457,13 @@ function* executePluginActionSaga( }), ); yield put( - updateActionData({ - entityName: pluginAction.name, - dataPath: "data", - data: EMPTY_RESPONSE.body, - }), + updateActionData([ + { + entityName: pluginAction.name, + dataPath: "data", + data: EMPTY_RESPONSE.body, + }, + ]), ); if (e instanceof UserCancelledActionExecutionError) { // Case: user cancelled the request of file upload @@ -1526,11 +1534,13 @@ function* clearTriggerActionResponse() { if (action.data && !action.config.executeOnLoad) { yield put(clearActionResponse(action.config.id)); yield put( - updateActionData({ - entityName: action.config.name, - dataPath: "data", - data: undefined, - }), + updateActionData([ + { + entityName: action.config.name, + dataPath: "data", + data: undefined, + }, + ]), ); } } @@ -1587,16 +1597,14 @@ function* handleUpdateActionData( entityName: string; dataPath: string; data: unknown; + dataPathRef?: string; }>, ) { - const { data, dataPath, entityName } = action.payload; - yield call(evalWorker.request, EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA, [ - { - entityName, - dataPath, - data, - }, - ]); + yield call( + evalWorker.request, + EVAL_WORKER_ACTIONS.UPDATE_ACTION_DATA, + action.payload, + ); } export function* watchPluginActionExecutionSagas() { diff --git a/app/client/src/sagas/ActionSagas.ts b/app/client/src/sagas/ActionSagas.ts index e61be91bab..dca0ed5015 100644 --- a/app/client/src/sagas/ActionSagas.ts +++ b/app/client/src/sagas/ActionSagas.ts @@ -38,6 +38,7 @@ import { moveActionError, moveActionSuccess, updateAction, + updateActionData, updateActionProperty, updateActionSuccess, } from "actions/pluginActionActions"; @@ -747,6 +748,16 @@ export function* refactorActionName( actionId: id, }, }); + yield put( + updateActionData([ + { + entityName: newName, + dataPath: "data", + data: undefined, + dataPathRef: `${oldName}.data`, + }, + ]), + ); if (currentPageId === pageId) { // @ts-expect-error: refactorResponse is of type unknown yield updateCanvasWithDSL(refactorResponse.data, pageId, layoutId); diff --git a/app/client/src/sagas/OnboardingSagas.ts b/app/client/src/sagas/OnboardingSagas.ts index 8209721560..3252ef92f5 100644 --- a/app/client/src/sagas/OnboardingSagas.ts +++ b/app/client/src/sagas/OnboardingSagas.ts @@ -219,11 +219,13 @@ function* setUpTourAppSaga() { const query: ActionData | undefined = yield select(getQueryAction); yield put(clearActionResponse(query?.config.id ?? "")); yield put( - updateActionData({ - entityName: query?.config.name || "", - dataPath: "data", - data: undefined, - }), + updateActionData([ + { + entityName: query?.config.name || "", + dataPath: "data", + data: undefined, + }, + ]), ); const applicationId: string = yield select(getCurrentApplicationId); yield put( diff --git a/app/client/src/workers/Evaluation/handlers/updateActionData.ts b/app/client/src/workers/Evaluation/handlers/updateActionData.ts index d22a725d27..886cd02b81 100644 --- a/app/client/src/workers/Evaluation/handlers/updateActionData.ts +++ b/app/client/src/workers/Evaluation/handlers/updateActionData.ts @@ -1,6 +1,6 @@ import { dataTreeEvaluator } from "./evalTree"; import type { EvalWorkerSyncRequest } from "../types"; -import { set } from "lodash"; +import set from "lodash/set"; import { evalTreeWithChanges } from "../evalTreeWithChanges"; import DataStore from "../dataStore"; @@ -8,6 +8,7 @@ export interface UpdateActionProps { entityName: string; dataPath: string; data: unknown; + dataPathRef?: string; } export default function (request: EvalWorkerSyncRequest) { const actionsDataToUpdate: UpdateActionProps[] = request.data; @@ -21,7 +22,13 @@ export function handleActionsDataUpdate(actionsToUpdate: UpdateActionProps[]) { const evalTree = dataTreeEvaluator.getEvalTree(); for (const actionToUpdate of actionsToUpdate) { - const { data, dataPath, entityName } = actionToUpdate; + const { dataPath, dataPathRef, entityName } = actionToUpdate; + let { data } = actionToUpdate; + + if (dataPathRef) { + data = DataStore.getActionData(dataPathRef); + DataStore.deleteActionData(dataPathRef); + } // update the evaltree set(evalTree, `${entityName}.[${dataPath}]`, data); // Update context