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.

Sign up for GitHub

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

Jump to bottom

chore: sending js patches #34290

Open
wants to merge 1 commit into
base: release
Choose a base branch
from
+568 −64
Open

chore: sending js patches #34290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 16 additions & 3 deletions app/client/src/ce/sagas/InferAffectedJSObjects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ import type { JSCollection } from "entities/JSCollection";
export function getAffectedJSObjectIdsFromJSAction(
action: ReduxAction<unknown> | BufferedReduxAction<unknown>,
): AffectedJSObjects {
// This is triggered during a page load and at that point of time we need to send all JSObjects
if (action.type === ReduxActionTypes.FETCH_ALL_PAGE_ENTITY_COMPLETION) {
return {
ids: [],
isAllAffected: true,
};
}

if (!JS_ACTIONS.includes(action.type)) {
return {
ids: [],
Expand All @@ -21,11 +29,16 @@ export function getAffectedJSObjectIdsFromJSAction(
}
// only JS actions here
action as ReduxAction<unknown>;
// When fetching JSActions fails, we need to diff all JSObjects because the reducer updates it
// to empty collection

if (
// When fetching JSActions fails, we need to diff all JSObjects because the reducer updates it
// to empty collection
action.type === ReduxActionErrorTypes.FETCH_JS_ACTIONS_ERROR ||
action.type === ReduxActionErrorTypes.FETCH_JS_ACTIONS_VIEW_MODE_ERROR
action.type === ReduxActionErrorTypes.FETCH_JS_ACTIONS_VIEW_MODE_ERROR ||
// for these two actions, we need to diff all JSObjects because the reducer updates allNodes
action.type === ReduxActionTypes.FETCH_JS_ACTIONS_FOR_PAGE_SUCCESS ||
action.type === ReduxActionTypes.FETCH_JS_ACTIONS_VIEW_MODE_SUCCESS ||
action.type === ReduxActionTypes.DELETE_JS_ACTION_SUCCESS
) {
return {
isAllAffected: true,
Expand Down
190 changes: 151 additions & 39 deletions app/client/src/sagas/EvaluationsSaga.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
} from "@appsmith/constants/ReduxActionConstants";
import { fetchPluginFormConfigsSuccess } from "actions/pluginActions";
import { createJSCollectionSuccess } from "actions/jsActionActions";
import type { UnEvalTree } from "entities/DataTree/dataTreeTypes";
import type { WidgetEntity } from "@appsmith/entities/DataTree/types";
jest.mock("loglevel");

describe("evaluateTreeSaga", () => {
Expand Down Expand Up @@ -50,7 +52,7 @@ describe("evaluateTreeSaga", () => {
appMode: false,
widgetsMeta: {},
shouldRespondWithLogs: true,
affectedJSObjects: { ids: [], isAllAffected: false },
jsPatches: { shouldReplaceAllNodes: false, patches: [] },
})
.run();
});
Expand Down Expand Up @@ -78,49 +80,159 @@ describe("evaluateTreeSaga", () => {
appMode: false,
widgetsMeta: {},
shouldRespondWithLogs: false,
affectedJSObjects: { ids: [], isAllAffected: false },
jsPatches: { shouldReplaceAllNodes: false, patches: [] },
})
.run();
});
test("should propagate affectedJSObjects property to evaluation action", async () => {
const unEvalAndConfigTree = { unEvalTree: {}, configTree: {} };
const affectedJSObjects = {
isAllAffected: false,
ids: ["1", "2"],
};
describe("affectedJSObjects", () => {
test("should send jsPatches property to during evaluation action", async () => {
const unEvalAndConfigTree = {
unEvalTree: {
jsObject1: {
ENTITY_TYPE: "JSACTION",
actionId: "1",
},
jsObject2: {
ENTITY_TYPE: "JSACTION",
actionId: "2",
},
jsObject3: {
ENTITY_TYPE: "JSACTION",
actionId: "3",
},
} as UnEvalTree,
configTree: {},
};
const affectedJSObjects = {
isAllAffected: false,
ids: ["1", "2"],
};

return expectSaga(
evaluateTreeSaga,
unEvalAndConfigTree,
[],
undefined,
undefined,
undefined,
affectedJSObjects,
)
.provide([
[select(getAllActionValidationConfig), {}],
[select(getWidgets), {}],
[select(getMetaWidgets), {}],
[select(getSelectedAppTheme), {}],
[select(getAppMode), false],
[select(getWidgetsMeta), {}],
])
.call(evalWorker.request, EVAL_WORKER_ACTIONS.EVAL_TREE, {
unevalTree: unEvalAndConfigTree,
widgetTypeConfigMap: undefined,
widgets: {},
theme: {},
shouldReplay: true,
allActionValidationConfig: {},
forceEvaluation: false,
metaWidgets: {},
appMode: false,
widgetsMeta: {},
shouldRespondWithLogs: false,
return expectSaga(
evaluateTreeSaga,
unEvalAndConfigTree,
[],
undefined,
undefined,
undefined,
affectedJSObjects,
})
.run();
)
.provide([
[select(getAllActionValidationConfig), {}],
[select(getWidgets), {}],
[select(getMetaWidgets), {}],
[select(getSelectedAppTheme), {}],
[select(getAppMode), false],
[select(getWidgetsMeta), {}],
])
.call(evalWorker.request, EVAL_WORKER_ACTIONS.EVAL_TREE, {
unevalTree: {
// The the JSObjects should be removed from the unevalTree
unEvalTree: {},
configTree: {},
},
widgetTypeConfigMap: undefined,
widgets: {},
theme: {},
shouldReplay: true,
allActionValidationConfig: {},
forceEvaluation: false,
metaWidgets: {},
appMode: false,
widgetsMeta: {},
shouldRespondWithLogs: false,
jsPatches: {
shouldReplaceAllNodes: false,
patches: [
{
path: "jsObject1",
value: {
ENTITY_TYPE: "JSACTION",
actionId: "1",
},
},
{
path: "jsObject2",
value: {
ENTITY_TYPE: "JSACTION",
actionId: "2",
},
},
],
},
})
.run();
});
test("should remove JS Objects from the unEvalTree for any evaluation, this should help in reducing the evalTree payload", async () => {
const unEvalAndConfigTree = {
unEvalTree: {
jsObject1: {
ENTITY_TYPE: "JSACTION",
actionId: "1",
},
jsObject2: {
ENTITY_TYPE: "JSACTION",
actionId: "2",
},
jsObject3: {
ENTITY_TYPE: "JSACTION",
actionId: "3",
},
widget1: {
ENTITY_TYPE: "WIDGET",
} as WidgetEntity,
} as UnEvalTree,
configTree: {},
};
const affectedJSObjects = {
isAllAffected: false,
ids: [],
};

return expectSaga(
evaluateTreeSaga,
unEvalAndConfigTree,
[],
undefined,
undefined,
undefined,
affectedJSObjects,
)
.provide([
[select(getAllActionValidationConfig), {}],
[select(getWidgets), {}],
[select(getMetaWidgets), {}],
[select(getSelectedAppTheme), {}],
[select(getAppMode), false],
[select(getWidgetsMeta), {}],
])
.call(evalWorker.request, EVAL_WORKER_ACTIONS.EVAL_TREE, {
unevalTree: {
// The the JSObjects should be removed from the unevalTree and non jsObject should be retained like widgets
unEvalTree: {
widget1: {
ENTITY_TYPE: "WIDGET",
},
},
configTree: {},
},
widgetTypeConfigMap: undefined,
widgets: {},
theme: {},
shouldReplay: true,
allActionValidationConfig: {},
forceEvaluation: false,
metaWidgets: {},
appMode: false,
widgetsMeta: {},
shouldRespondWithLogs: false,
jsPatches: {
shouldReplaceAllNodes: false,
patches: [],
},
})
.run();
});
});
});

Expand Down
15 changes: 13 additions & 2 deletions app/client/src/sagas/EvaluationsSaga.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import { logDynamicTriggerExecution } from "@appsmith/sagas/analyticsSaga";
import { selectFeatureFlags } from "@appsmith/selectors/featureFlagsSelectors";
import { fetchFeatureFlagsInit } from "actions/userActions";
import type { AffectedJSObjects } from "./EvaluationsSagaUtils";
import { seperateOutAffectedJSactions } from "./EvaluationsSagaUtils";
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider renaming the imported function for clarity.

The function seperateOutAffectedJSactions appears to be misspelled. It should be separateOutAffectedJSactions. This will improve code readability and maintainability.

- import { seperateOutAffectedJSactions } from "./EvaluationsSagaUtils";
+ import { separateOutAffectedJSactions } from "./EvaluationsSagaUtils";
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
import { seperateOutAffectedJSactions } from "./EvaluationsSagaUtils";
import { separateOutAffectedJSactions } from "./EvaluationsSagaUtils";

import {
getAffectedJSObjectIdsFromAction,
parseUpdatesAndDeleteUndefinedUpdates,
Expand Down Expand Up @@ -272,9 +273,19 @@ export function* evaluateTreeSaga(
yield select(getWidgetsMeta);

const shouldRespondWithLogs = log.getLevel() === log.levels.DEBUG;
// We are seperating out the JS actions from the unevalTree to reduce the payload size here
// we are sending JSPatches only when required
const { jsPatches, unevalTreeWithoutJSObjects } =
seperateOutAffectedJSactions(
unEvalAndConfigTree.unEvalTree,
affectedJSObjects,
);

const evalTreeRequestData: EvalTreeRequestData = {
unevalTree: unEvalAndConfigTree,
unevalTree: {
configTree: unEvalAndConfigTree.configTree,
unEvalTree: unevalTreeWithoutJSObjects,
},
widgetTypeConfigMap,
widgets,
theme,
Expand All @@ -285,7 +296,7 @@ export function* evaluateTreeSaga(
appMode,
widgetsMeta,
shouldRespondWithLogs,
affectedJSObjects,
jsPatches,
};

const workerResponse: EvalTreeResponseData = yield call(
Expand Down
37 changes: 31 additions & 6 deletions app/client/src/sagas/EvaluationsSagaUtils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { getAffectedJSObjectIdsFromAction } from "./EvaluationsSagaUtils";
import {
copyJSCollectionSuccess,
createJSCollectionSuccess,
deleteJSCollectionSuccess,
fetchJSCollectionsForPageSuccess,
moveJSCollectionSuccess,
} from "actions/jsActionActions";
import { updateJSCollectionBodySuccess } from "actions/jsPaneActions";
Expand All @@ -19,8 +17,8 @@ import {

describe("getAffectedJSObjectIdsFromAction", () => {
const jsObject1 = { id: "1234" } as JSCollection;
const jsObject2 = { id: "5678" } as JSCollection;
const jsCollection: JSCollection[] = [jsObject1, jsObject2];
// const jsObject2 = { id: "5678" } as JSCollection;
// const jsCollection: JSCollection[] = [jsObject1, jsObject2];

test("should return a default response for an empty action ", () => {
const result = getAffectedJSObjectIdsFromAction(
Expand Down Expand Up @@ -68,11 +66,9 @@ describe("getAffectedJSObjectIdsFromAction", () => {

test.each([
[createJSCollectionSuccess, jsObject1, ["1234"]],
[deleteJSCollectionSuccess, jsObject1, ["1234"]],
[copyJSCollectionSuccess, jsObject1, ["1234"]],
[moveJSCollectionSuccess, jsObject1, ["1234"]],
[updateJSCollectionBodySuccess, { data: jsObject1 }, ["1234"]],
[fetchJSCollectionsForPageSuccess, jsCollection, ["1234", "5678"]],
])(
"should return the correct affected JSObject ids for action %p with input %p and expected to be %p",
(action, input, expected) => {
Expand All @@ -93,4 +89,33 @@ describe("getAffectedJSObjectIdsFromAction", () => {
expect(result).toEqual({ isAllAffected: true, ids: [] });
});
});
test("should return isAllAffected to be true for a delete JS action", () => {
const result = getAffectedJSObjectIdsFromAction({
type: ReduxActionTypes.DELETE_JS_ACTION_SUCCESS,
} as ReduxAction<unknown>);
expect(result).toEqual({ isAllAffected: true, ids: [] });
});
test("should return isAllAffected to be true for all FETCH calls", () => {
[
ReduxActionTypes.FETCH_JS_ACTIONS_FOR_PAGE_SUCCESS,
ReduxActionTypes.FETCH_JS_ACTIONS_VIEW_MODE_SUCCESS,
].forEach((actionType) => {
const result = getAffectedJSObjectIdsFromAction({
type: actionType,
} as ReduxAction<unknown>);
expect(result).toEqual({ isAllAffected: true, ids: [] });
});
});
test("should return isAllAffected to be true for FETCH_ALL_PAGE_ENTITY_COMPLETION", () => {
const result = getAffectedJSObjectIdsFromAction({
type: ReduxActionTypes.FETCH_ALL_PAGE_ENTITY_COMPLETION,
} as ReduxAction<unknown>);
expect(result).toEqual({ isAllAffected: true, ids: [] });
});
test("should return the default response for any other action type", () => {
const result = getAffectedJSObjectIdsFromAction({
type: "SOME_RANDOM_ACTION",
} as ReduxAction<unknown>);
expect(result).toEqual({ isAllAffected: false, ids: [] });
});
});
Loading
Loading