fix: deleted JSObject names are not released for later use (#29148)
## Description > Repeated refactor operations seems to leave stale JS Objects in the dataTree causing the dataTree to occupy namespace that no longer exists. This seems to have stemmed from the `updateEvalTreeWithJSCollectionState` method in app/client/src/workers/common/DataTreeEvaluator/index.ts, where we update the existing evalTree with JSObjects and their variables from the previous unEvalTree. > This PR modifies `updateEvalTreeWithJSCollectionState` to not apply variable state to evalTree if the entity is not present. #### PR fixes following issue(s) Fixes #29162 > if no issue exists, please create an issue and ask the maintainers about this first > > #### 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 - [x] Repeated renaming operations should work. - [x] Deleting a JS Object should make the name available for use. - [x] Execution data should be preserved on rename operations > > #### 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 - [x] 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
This commit is contained in:
parent
56b4671baf
commit
6c81f07e79
|
|
@ -0,0 +1,57 @@
|
|||
import {
|
||||
agHelper,
|
||||
locators,
|
||||
entityExplorer,
|
||||
jsEditor,
|
||||
} from "../../../../support/Objects/ObjectsCore";
|
||||
import { EntityItems } from "../../../../support/Pages/AssertHelper";
|
||||
|
||||
const jsObjectBody = `export default {
|
||||
myVar1: [],
|
||||
myVar2: {},
|
||||
myFun1(){
|
||||
|
||||
},
|
||||
myFun2: async () => {
|
||||
//use async-await or promises
|
||||
}
|
||||
}`;
|
||||
|
||||
describe("Verifies JS object rename bug", () => {
|
||||
it("Verify that a JS Object name is up for taking after it is deleted", () => {
|
||||
jsEditor.CreateJSObject(jsObjectBody, {
|
||||
paste: true,
|
||||
completeReplace: true,
|
||||
toRun: false,
|
||||
shouldCreateNewJSObj: true,
|
||||
prettify: false,
|
||||
});
|
||||
|
||||
jsEditor.CreateJSObject(jsObjectBody, {
|
||||
paste: true,
|
||||
completeReplace: true,
|
||||
toRun: false,
|
||||
shouldCreateNewJSObj: true,
|
||||
prettify: false,
|
||||
});
|
||||
|
||||
jsEditor.RenameJSObjFromPane("JSObj2");
|
||||
|
||||
agHelper.ActionContextMenuWithInPane({
|
||||
action: "Delete",
|
||||
entityType: EntityItems.JSObject,
|
||||
});
|
||||
|
||||
jsEditor.CreateJSObject(jsObjectBody, {
|
||||
paste: true,
|
||||
completeReplace: true,
|
||||
toRun: false,
|
||||
shouldCreateNewJSObj: true,
|
||||
prettify: false,
|
||||
});
|
||||
|
||||
jsEditor.RenameJSObjFromPane("JSObj2");
|
||||
|
||||
entityExplorer.AssertEntityPresenceInExplorer("JSObj2");
|
||||
});
|
||||
});
|
||||
|
|
@ -377,19 +377,19 @@ export function* refactorJSObjectName(
|
|||
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);
|
||||
yield put(
|
||||
updateActionData(
|
||||
functions.map((f) => ({
|
||||
entityName: newName,
|
||||
data: undefined,
|
||||
dataPath: `${f.name}.data`,
|
||||
dataPathRef: `${oldName}.${f.name}.data`,
|
||||
})),
|
||||
),
|
||||
);
|
||||
} else {
|
||||
yield put(fetchJSCollectionsForPage(pageId));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -748,19 +748,19 @@ 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);
|
||||
yield put(
|
||||
updateActionData([
|
||||
{
|
||||
entityName: newName,
|
||||
dataPath: "data",
|
||||
data: undefined,
|
||||
dataPathRef: `${oldName}.data`,
|
||||
},
|
||||
]),
|
||||
);
|
||||
} else {
|
||||
yield put(fetchActionsForPage(pageId));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { get, isEmpty, merge, set } from "lodash";
|
||||
import { get, isEmpty, set } from "lodash";
|
||||
import type { JSActionEntity } from "@appsmith/entities/DataTree/types";
|
||||
import type { ConfigTree, DataTree } from "entities/DataTree/dataTreeTypes";
|
||||
import { EvalErrorTypes, getEvalValuePath } from "utils/DynamicBindingUtils";
|
||||
|
|
@ -297,22 +297,12 @@ export function getJSEntities(dataTree: DataTree) {
|
|||
return jsCollections;
|
||||
}
|
||||
|
||||
export function updateEvalTreeWithJSCollectionState(
|
||||
evalTree: DataTree,
|
||||
oldUnEvalTree: DataTree,
|
||||
) {
|
||||
export function updateEvalTreeWithJSCollectionState(evalTree: DataTree) {
|
||||
// loop through jsCollectionState and set all values to evalTree
|
||||
const jsCollections = JSObjectCollection.getVariableState();
|
||||
const jsCollectionEntries = Object.entries(jsCollections);
|
||||
for (const [jsObjectName, variableState] of jsCollectionEntries) {
|
||||
if (!evalTree[jsObjectName]) {
|
||||
evalTree[jsObjectName] = merge(
|
||||
{},
|
||||
oldUnEvalTree[jsObjectName],
|
||||
variableState,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if (!evalTree[jsObjectName]) continue;
|
||||
evalTree[jsObjectName] = Object.assign(
|
||||
evalTree[jsObjectName],
|
||||
variableState,
|
||||
|
|
|
|||
|
|
@ -714,7 +714,7 @@ export default class DataTreeEvaluator {
|
|||
updateDependencyMapTime = "0",
|
||||
} = extraParams;
|
||||
|
||||
updateEvalTreeWithJSCollectionState(this.evalTree, this.oldUnEvalTree);
|
||||
updateEvalTreeWithJSCollectionState(this.evalTree);
|
||||
|
||||
const calculateSortOrderStartTime = performance.now();
|
||||
const subTreeSortOrder: string[] = this.calculateSubTreeSortOrder(
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user