fix: resolve redundant JSObject action saving (#36958)
## Description In this change, we avoid storing duplicate actions or variables in the `parsedBody` after AST parsing. This prevents duplicate actions from being sent to the server on save. This change also removes `parsedFunction` from the action object of parsedBody as it is not being used. Fixes https://github.com/appsmithorg/appsmith/issues/36879 ## Test Cases - [x] After parsing JS Object make sure parsedBody doesn't have duplicate actions or variables ## Automation /ok-to-test tags="@tag.JS" ### 🔍 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/11485245933> > Commit: c12ee7007d8f860aa3bbf2cc1889bd4b762785a6 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11485245933&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.JS` > Spec: > <hr>Wed, 23 Oct 2024 18:20:11 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 enhanced methods for creating, updating, and moving action collections, improving validation and consistency. - Added a new interface for better handling of JavaScript actions and variables, enhancing data processing efficiency. - **Bug Fixes** - Improved error reporting from pages to collections, ensuring accurate tracking of action errors. - Added tests to ensure duplicate actions are correctly handled during updates. - **Documentation** - Updated comments and documentation for clarity on new functionalities and changes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rishabh Rathod
GitHub
parent
b7b5770a08
commit
5d571b9185
|
|
@ -114,18 +114,17 @@ export function saveResolvedFunctionsAndJSUpdates(
|
|||
JSObjectName: entityName,
|
||||
JSObjectASTParseTime,
|
||||
});
|
||||
// TODO: Fix this the next time the file is edited
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const actions: any = [];
|
||||
// TODO: Fix this the next time the file is edited
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const variables: any = [];
|
||||
|
||||
const actionsMap: Record<string, ParsedJSSubAction> = {};
|
||||
const variablesMap: Record<string, { name: string; value: unknown }> = {};
|
||||
|
||||
if (success) {
|
||||
if (!!parsedObject) {
|
||||
jsPropertiesState.update(entityName, parsedObject);
|
||||
parsedObject.forEach((parsedElement) => {
|
||||
if (isJSFunctionProperty(parsedElement)) {
|
||||
if (actionsMap[parsedElement.key]) return;
|
||||
|
||||
try {
|
||||
ExecutionMetaData.setExecutionMetaData({
|
||||
enableJSVarUpdateTracking: false,
|
||||
|
|
@ -164,12 +163,11 @@ export function saveResolvedFunctionsAndJSUpdates(
|
|||
`${entityName}.${parsedElement.key}`,
|
||||
functionString,
|
||||
);
|
||||
actions.push({
|
||||
actionsMap[parsedElement.key] = {
|
||||
name: parsedElement.key,
|
||||
body: functionString,
|
||||
arguments: params,
|
||||
parsedFunction: result,
|
||||
});
|
||||
};
|
||||
}
|
||||
} catch {
|
||||
// in case we need to handle error state
|
||||
|
|
@ -184,10 +182,10 @@ export function saveResolvedFunctionsAndJSUpdates(
|
|||
? parsedElement.key.slice(1, -1)
|
||||
: parsedElement.key;
|
||||
|
||||
variables.push({
|
||||
variablesMap[parsedKey] = {
|
||||
name: parsedKey,
|
||||
value: parsedElement.value,
|
||||
});
|
||||
};
|
||||
JSObjectCollection.updateUnEvalState(
|
||||
`${entityName}.${parsedElement.key}`,
|
||||
parsedElement.value,
|
||||
|
|
@ -196,8 +194,8 @@ export function saveResolvedFunctionsAndJSUpdates(
|
|||
});
|
||||
const parsedBody = {
|
||||
body: entity.body,
|
||||
actions: actions,
|
||||
variables,
|
||||
actions: Object.values(actionsMap),
|
||||
variables: Object.values(variablesMap),
|
||||
};
|
||||
|
||||
set(jsUpdates, `${entityName}`, {
|
||||
|
|
@ -312,8 +310,6 @@ export function parseJSActions(
|
|||
parsedBody.actions = parsedBody.actions.map((action) => {
|
||||
return {
|
||||
...action,
|
||||
// parsedFunction - used only to determine if function is async
|
||||
parsedFunction: undefined,
|
||||
} as ParsedJSSubAction;
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,14 @@
|
|||
import type { ConfigTree, UnEvalTree } from "entities/DataTree/dataTreeTypes";
|
||||
import { getUpdatedLocalUnEvalTreeAfterJSUpdates } from ".";
|
||||
import {
|
||||
getUpdatedLocalUnEvalTreeAfterJSUpdates,
|
||||
saveResolvedFunctionsAndJSUpdates,
|
||||
} from ".";
|
||||
import type { JSUpdate } from "utils/JSPaneUtils";
|
||||
import type {
|
||||
JSActionEntity,
|
||||
JSActionEntityConfig,
|
||||
} from "ee/entities/DataTree/types";
|
||||
import DataTreeEvaluator from "workers/common/DataTreeEvaluator";
|
||||
|
||||
describe("updateJSCollectionInUnEvalTree", function () {
|
||||
it("updates async value of jsAction", () => {
|
||||
|
|
@ -137,3 +146,119 @@ describe("updateJSCollectionInUnEvalTree", function () {
|
|||
expect(expectedResult).toStrictEqual(actualResult);
|
||||
});
|
||||
});
|
||||
|
||||
describe("saveResolvedFunctionsAndJSUpdates", function () {
|
||||
it("parses JSObject with duplicate actions, variables and updates jsUpdates correctly", () => {
|
||||
const dataTreeEvalRef = new DataTreeEvaluator({});
|
||||
const entity: JSActionEntity = {
|
||||
actionId: "64013546b956c26882acc587",
|
||||
body: "export default {\n\tmyVar1: [],\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t\t\n\t},\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}\n,\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}}",
|
||||
ENTITY_TYPE: "JSACTION",
|
||||
name: "JSObject1",
|
||||
pluginType: "JS",
|
||||
};
|
||||
const jsUpdates: Record<string, JSUpdate> = {};
|
||||
const unEvalDataTree: UnEvalTree = {
|
||||
JSObject1: {
|
||||
myVar1: "[]",
|
||||
myVar2: "{}",
|
||||
myFun1: new String("() => {}"),
|
||||
myFun2: new String("async () => {\n yeso;\n}"),
|
||||
body: "export default {\n\tmyVar1: [],\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t\t\n\t},\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}\n,\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}}",
|
||||
ENTITY_TYPE: "JSACTION",
|
||||
meta: {
|
||||
myFun1: {
|
||||
arguments: [],
|
||||
confirmBeforeExecute: false,
|
||||
},
|
||||
myFun2: {
|
||||
arguments: [],
|
||||
confirmBeforeExecute: false,
|
||||
},
|
||||
},
|
||||
dependencyMap: {
|
||||
body: ["myFun1", "myFun2"],
|
||||
},
|
||||
dynamicBindingPathList: [
|
||||
{
|
||||
key: "body",
|
||||
},
|
||||
{
|
||||
key: "myVar1",
|
||||
},
|
||||
{
|
||||
key: "myVar2",
|
||||
},
|
||||
{
|
||||
key: "myFun1",
|
||||
},
|
||||
{
|
||||
key: "myFun2",
|
||||
},
|
||||
{
|
||||
key: "myFun2",
|
||||
},
|
||||
],
|
||||
bindingPaths: {
|
||||
body: "SMART_SUBSTITUTE",
|
||||
myVar1: "SMART_SUBSTITUTE",
|
||||
myVar2: "SMART_SUBSTITUTE",
|
||||
myFun1: "SMART_SUBSTITUTE",
|
||||
myFun2: "SMART_SUBSTITUTE",
|
||||
},
|
||||
reactivePaths: {
|
||||
body: "SMART_SUBSTITUTE",
|
||||
myVar1: "SMART_SUBSTITUTE",
|
||||
myVar2: "SMART_SUBSTITUTE",
|
||||
myFun1: "SMART_SUBSTITUTE",
|
||||
myFun2: "SMART_SUBSTITUTE",
|
||||
},
|
||||
pluginType: "JS",
|
||||
name: "JSObject1",
|
||||
actionId: "64013546b956c26882acc587",
|
||||
} as JSActionEntityConfig,
|
||||
};
|
||||
const entityName = "JSObject1";
|
||||
|
||||
const result = saveResolvedFunctionsAndJSUpdates(
|
||||
dataTreeEvalRef,
|
||||
entity,
|
||||
jsUpdates,
|
||||
unEvalDataTree,
|
||||
entityName,
|
||||
);
|
||||
|
||||
const expectedJSUpdates = {
|
||||
JSObject1: {
|
||||
parsedBody: {
|
||||
body: "export default {\n\tmyVar1: [],\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t\t\n\t},\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}\n,\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}}",
|
||||
actions: [
|
||||
{
|
||||
name: "myFun1",
|
||||
body: "() => {}",
|
||||
arguments: [],
|
||||
},
|
||||
{
|
||||
name: "myFun2",
|
||||
body: "() => {\n yeso;\n}",
|
||||
arguments: [],
|
||||
},
|
||||
],
|
||||
variables: [
|
||||
{
|
||||
name: "myVar1",
|
||||
value: "[]",
|
||||
},
|
||||
{
|
||||
name: "myVar2",
|
||||
value: "{}",
|
||||
},
|
||||
],
|
||||
},
|
||||
id: "64013546b956c26882acc587",
|
||||
},
|
||||
};
|
||||
|
||||
expect(result).toStrictEqual(expectedJSUpdates);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user