From 3e81d738231b0dbfd4ab351cb81d55573e1a8162 Mon Sep 17 00:00:00 2001 From: Diljit Date: Thu, 13 Jun 2024 13:41:14 +0530 Subject: [PATCH] fix: Fix for regression in js query refactor in modules (#34186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #33545 removed the action `REFACTOR_JS_ACTION_NAME` and called the saga `handleRefactorJSActionNameSaga` directly. This caused a regression in the refactor of functions in js modules. This PR reverts this code. Fixes #34148 /ok-to-test tags="@tag.All" > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 3b7c67d7d82c4d15d7129f09a023d6475b835600 > Cypress dashboard url: Click here! Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No - **New Features** - Added new actions for refactoring JavaScript collections to improve management and updating of JS collections in the application. - **Refactor** - Simplified dummy JavaScript functions, removing comments and unnecessary code to streamline function bodies. - Updated variable initialization values for cleaner code. - **Tests** - Updated network status assertions for JS collections in test scripts. - Switched test specifications in the Cypress test suite to ensure better coverage. --- app/client/cypress/support/Pages/JSEditor.ts | 2 +- app/client/src/sagas/JSPaneSagas.ts | 55 +++++++++++--------- app/client/src/utils/JSPaneUtils.tsx | 10 ++-- 3 files changed, 35 insertions(+), 32 deletions(-) diff --git a/app/client/cypress/support/Pages/JSEditor.ts b/app/client/cypress/support/Pages/JSEditor.ts index 4ac86c773f..c870c001c0 100644 --- a/app/client/cypress/support/Pages/JSEditor.ts +++ b/app/client/cypress/support/Pages/JSEditor.ts @@ -112,7 +112,7 @@ export class JSEditor { Cypress.env("MESSAGES").ADD_QUERY_JS_TOOLTIP(), ); //Checking JS object was created successfully - this.assertHelper.AssertNetworkStatus("@jsCollections", 200); + this.assertHelper.AssertNetworkStatus("@createNewJSCollection", 201); this.agHelper.AssertElementVisibility(this._jsObjTxt); // Assert that the name of the JS Object is focused when newly created this.agHelper.PressEnter(); diff --git a/app/client/src/sagas/JSPaneSagas.ts b/app/client/src/sagas/JSPaneSagas.ts index f8fb06a013..dc659450a7 100644 --- a/app/client/src/sagas/JSPaneSagas.ts +++ b/app/client/src/sagas/JSPaneSagas.ts @@ -58,6 +58,7 @@ import { createNewJSCollection, jsSaveActionComplete, jsSaveActionStart, + refactorJSCollectionAction, } from "actions/jsPaneActions"; import { getCurrentWorkspaceId } from "@appsmith/selectors/selectedWorkspaceSelectors"; import { getPluginIdOfPackageName } from "sagas/selectors"; @@ -204,21 +205,22 @@ function* handleEachUpdateJSCollection(update: JSUpdate) { const parsedBody = update.parsedBody; if (parsedBody && !!jsAction) { const jsActionTobeUpdated = JSON.parse(JSON.stringify(jsAction)); - // jsActionTobeUpdated.body = jsAction.body; const data = getDifferenceInJSCollection(parsedBody, jsAction); + if (data.nameChangedActions.length) { for (let i = 0; i < data.nameChangedActions.length; i++) { - yield call( - handleRefactorJSActionNameSaga, - { - actionId: data.nameChangedActions[i].id, - collectionName: jsAction.name, - pageId: data.nameChangedActions[i].pageId, - moduleId: data.nameChangedActions[i].moduleId, - oldName: data.nameChangedActions[i].oldName, - newName: data.nameChangedActions[i].newName, - }, - jsActionTobeUpdated, + yield put( + refactorJSCollectionAction({ + refactorAction: { + actionId: data.nameChangedActions[i].id, + collectionName: jsAction.name, + pageId: data.nameChangedActions[i].pageId || "", + moduleId: data.nameChangedActions[i].moduleId, + oldName: data.nameChangedActions[i].oldName, + newName: data.nameChangedActions[i].newName, + }, + actionCollection: jsActionTobeUpdated, + }), ); } } else { @@ -292,23 +294,11 @@ export function* makeUpdateJSCollection( ) { const jsUpdates: Record = action.payload || {}; - yield all( - Object.keys(jsUpdates).map((key) => - put(jsSaveActionStart({ id: jsUpdates[key].id })), - ), - ); - yield all( Object.keys(jsUpdates).map((key) => call(handleEachUpdateJSCollection, jsUpdates[key]), ), ); - - yield all( - Object.keys(jsUpdates).map((key) => - put(jsSaveActionComplete({ id: jsUpdates[key].id })), - ), - ); } function* updateJSCollection(data: { @@ -325,6 +315,7 @@ function* updateJSCollection(data: { try { const { deletedActions, jsCollection, newActions } = data; if (jsCollection) { + yield put(jsSaveActionStart({ id: jsCollection.id })); const response: JSCollectionCreateUpdateResponse = yield JSActionAPI.updateJSCollection(jsCollection); const isValidResponse: boolean = yield validateResponse(response); @@ -362,6 +353,8 @@ function* updateJSCollection(data: { type: ReduxActionErrorTypes.UPDATE_JS_ACTION_ERROR, payload: { error, data: jsAction }, }); + } finally { + yield put(jsSaveActionComplete({ id: data.jsCollection.id })); } } @@ -637,9 +630,12 @@ function* handleUpdateJSCollectionBody( } function* handleRefactorJSActionNameSaga( - refactorAction: RefactorAction, - actionCollection: JSCollection, + data: ReduxAction<{ + refactorAction: RefactorAction; + actionCollection: JSCollection; + }>, ) { + const { actionCollection, refactorAction } = data.payload; const { pageId } = refactorAction; const layoutId: string | undefined = yield select(getCurrentLayoutId); if (!pageId || !layoutId) { @@ -653,6 +649,7 @@ function* handleRefactorJSActionNameSaga( }; // call to refactor action try { + yield put(jsSaveActionStart({ id: actionCollection.id })); const refactorResponse: ApiResponse = yield JSActionAPI.updateJSCollectionActionRefactor(requestData); @@ -680,6 +677,8 @@ function* handleRefactorJSActionNameSaga( type: ReduxActionErrorTypes.REFACTOR_JS_ACTION_NAME_ERROR, payload: { collectionId: actionCollection.id }, }); + } finally { + yield put(jsSaveActionComplete({ id: actionCollection.id })); } } @@ -825,6 +824,10 @@ export default function* root() { ReduxActionTypes.START_EXECUTE_JS_FUNCTION, handleStartExecuteJSFunctionSaga, ), + takeEvery( + ReduxActionTypes.REFACTOR_JS_ACTION_NAME, + handleRefactorJSActionNameSaga, + ), debounce( 100, ReduxActionTypes.UPDATE_JS_ACTION_BODY_INIT, diff --git a/app/client/src/utils/JSPaneUtils.tsx b/app/client/src/utils/JSPaneUtils.tsx index 0b359f0c60..bb1e1834f5 100644 --- a/app/client/src/utils/JSPaneUtils.tsx +++ b/app/client/src/utils/JSPaneUtils.tsx @@ -219,7 +219,7 @@ export const createDummyJSCollectionActions = ( workspaceId, executeOnLoad: false, actionConfiguration: { - body: "function (){\n\t\t//\twrite code here\n\t\t//\tthis.myVar1 = [1,2,3]\n\t}", + body: "function () {}", timeoutInMillisecond: 0, jsArguments: [], }, @@ -231,7 +231,7 @@ export const createDummyJSCollectionActions = ( workspaceId, executeOnLoad: false, actionConfiguration: { - body: "async function () {\n\t\t//\tuse async-await or promises\n\t\t//\tawait storeValue('varName', 'hello world')\n\t}", + body: "async function () {}", timeoutInMillisecond: 0, jsArguments: [], }, @@ -243,11 +243,11 @@ export const createDummyJSCollectionActions = ( const variables = [ { name: "myVar1", - value: [], + value: "[]", }, { name: "myVar2", - value: {}, + value: "{}", }, ]; @@ -271,7 +271,7 @@ export const createSingleFunctionJsCollection = ( workspaceId, executeOnLoad: false, actionConfiguration: { - body: "function (){\n\t\t//\twrite code here\n\t}", + body: "function () {}", timeoutInMillisecond: 0, jsArguments: [], },