From 08a882be24e8889a2d6dd3e690d2e1dd664aab7e Mon Sep 17 00:00:00 2001 From: balajisoundar Date: Fri, 6 May 2022 11:12:35 +0530 Subject: [PATCH] chore: Update property from property pane to use improved get strategy & BATCH_UPDATE_WIDGET_PROPERTY to be processed serially (#12544) --- app/client/src/sagas/WidgetOperationSagas.tsx | 27 +- .../src/sagas/WidgetOperationUtils.test.ts | 700 ++++++++++++++++++ app/client/src/sagas/WidgetOperationUtils.ts | 60 ++ 3 files changed, 782 insertions(+), 5 deletions(-) diff --git a/app/client/src/sagas/WidgetOperationSagas.tsx b/app/client/src/sagas/WidgetOperationSagas.tsx index 8aef191adb..7bb10e3599 100644 --- a/app/client/src/sagas/WidgetOperationSagas.tsx +++ b/app/client/src/sagas/WidgetOperationSagas.tsx @@ -11,8 +11,10 @@ import { } from "reducers/entityReducers/canvasWidgetsReducer"; import { getWidget, getWidgets } from "./selectors"; import { + actionChannel, all, call, + take, fork, put, select, @@ -110,6 +112,7 @@ import { getWidgetsFromIds, getDefaultCanvas, isDropTarget, + getValueFromTree, } from "./WidgetOperationUtils"; import { getSelectedWidgets } from "selectors/ui"; import { widgetSelectionSagas } from "./WidgetSelectionSagas"; @@ -434,7 +437,7 @@ export function getPropertiesToUpdate( } = getAllPathsFromPropertyConfig(widgetWithUpdates, widgetConfig, {}); Object.keys(updatePaths).forEach((propertyPath) => { - const propertyValue = _.get(updates, propertyPath); + const propertyValue = getValueFromTree(updates, propertyPath); // only check if if (!_.isString(propertyValue)) { return; @@ -1611,10 +1614,28 @@ export function* groupWidgetsSaga() { } } +function* widgetBatchUpdatePropertySaga() { + /* + * BATCH_UPDATE_WIDGET_PROPERTY should be processed serially as + * it updates the state. We want the state updates from previous + * batch update to be flushed out to the store before processing + * the another batch update. + */ + const batchUpdateWidgetPropertyChannel = yield actionChannel( + ReduxActionTypes.BATCH_UPDATE_WIDGET_PROPERTY, + ); + + while (true) { + const action = yield take(batchUpdateWidgetPropertyChannel); + yield call(batchUpdateWidgetPropertySaga, action); + } +} + export default function* widgetOperationSagas() { yield fork(widgetAdditionSagas); yield fork(widgetDeletionSagas); yield fork(widgetSelectionSagas); + yield fork(widgetBatchUpdatePropertySaga); yield all([ takeEvery(ReduxActionTypes.ADD_SUGGESTED_WIDGET, addSuggestedWidget), takeLatest(WidgetReduxActionTypes.WIDGET_RESIZE, resizeSaga), @@ -1634,10 +1655,6 @@ export default function* widgetOperationSagas() { ReduxActionTypes.RESET_CHILDREN_WIDGET_META, resetChildrenMetaSaga, ), - takeEvery( - ReduxActionTypes.BATCH_UPDATE_WIDGET_PROPERTY, - batchUpdateWidgetPropertySaga, - ), takeEvery( ReduxActionTypes.BATCH_UPDATE_MULTIPLE_WIDGETS_PROPERTY, batchUpdateMultipleWidgetsPropertiesSaga, diff --git a/app/client/src/sagas/WidgetOperationUtils.test.ts b/app/client/src/sagas/WidgetOperationUtils.test.ts index 126fa671eb..fff2de7936 100644 --- a/app/client/src/sagas/WidgetOperationUtils.test.ts +++ b/app/client/src/sagas/WidgetOperationUtils.test.ts @@ -18,6 +18,7 @@ import { getPastePositionMapFromMousePointer, getReflowedPositions, getWidgetsFromIds, + getValueFromTree, } from "./WidgetOperationUtils"; describe("WidgetOperationSaga", () => { @@ -979,3 +980,702 @@ describe("WidgetOperationSaga", () => { ]); }); }); + +describe("getValueFromTree - ", () => { + it("should test that value is correctly plucked from a valid path when object keys do not have dot", () => { + [ + //Path that has a primitive value as leaf node + { + inputObj: { + path1: { + path2: "value", + }, + someotherPath: "testValue", + }, + path: "path1.path2", + output: "value", + defaultValue: "will not be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + path1: { + path2: { + path3: "value", + }, + }, + someotherPath: "testValue", + }, + path: "path1.path2", + output: { + path3: "value", + }, + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + path1: [ + { + path2: "value", + }, + ], + someotherPath: "testValue", + }, + path: "path1.0.path2", + output: "value", + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + path1: [ + { + path2: { + path3: "value", + }, + }, + ], + someotherPath: "testValue", + }, + path: "path1.0.path2", + output: { + path3: "value", + }, + defaultValue: "will not be returned", + }, + ].forEach((testObj: any) => { + expect( + getValueFromTree(testObj.inputObj, testObj.path, testObj.defaultValue), + ).toEqual(testObj.output); + }); + }); + + it("should test that default value is returned for invalid path when object keys do not have dot", () => { + [ + //Path that has a primitive value as leaf node + { + inputObj: { + path1: { + path2: "value", + }, + someotherPath: "testValue", + }, + path: "path1.path4", + output: "value", + defaultValue: "will be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + path1: { + path2: { + path3: "value", + }, + }, + someotherPath: "testValue", + }, + path: "path4.path2", + output: { + path3: "value", + }, + defaultValue: "will be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + path1: [ + { + path2: "value", + someotherPath: "testValue", + }, + ], + }, + path: "path1.1.path2", + output: "value", + defaultValue: "will be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + path1: [ + { + path2: { + path3: "value", + }, + }, + ], + someotherPath: "testValue", + }, + path: "path1.1.path2", + output: { + path3: "value", + }, + defaultValue: "will be returned", + }, + ].forEach((testObj: any) => { + expect( + getValueFromTree(testObj.inputObj, testObj.path, testObj.defaultValue), + ).toEqual(testObj.defaultValue); + }); + }); + + it("should test that value is correctly plucked from a valid path when object keys have dot", () => { + [ + //Path that has a primitive value as leaf node + { + inputObj: { + "path1.path2.path3": "value", + }, + path: "path1.path2.path3", + output: "value", + defaultValue: "will not be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + "path1.path2": { + path3: "value", + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3", + output: "value", + defaultValue: "will not be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + path1: { + "path2.path3": "value", + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3", + output: "value", + defaultValue: "will not be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + path1: { + path2: { + "path3.path4": "value", + }, + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3.path4", + output: "value", + defaultValue: "will not be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + "path1.path2": { + "path3.path4": "value", + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3.path4", + output: "value", + defaultValue: "will not be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": { + path4: "value", + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3", + output: { + path4: "value", + }, + defaultValue: "will not be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + path1: { + "path2.path3": { + path4: "value", + }, + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3", + output: { + path4: "value", + }, + defaultValue: "will not be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + path1: { + path2: { + "path3.path4": { + path5: "value", + }, + }, + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3.path4", + output: { + path5: "value", + }, + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: "value", + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.0.path3", + output: "value", + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: { + path4: "value", + }, + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.0.path3.path4", + output: "value", + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + "path3.path4": "value", + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.0.path3.path4", + output: "value", + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: [ + { + path4: "value", + }, + ], + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.0.path3.0.path4", + output: "value", + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: { + path4: "value", + }, + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.0.path3", + output: { + path4: "value", + }, + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": [ + { + path4: "value", + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.path3.0", + output: { + path4: "value", + }, + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": [ + { + path4: [ + { + path5: "value", + }, + ], + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.path3.0.path4.0", + output: { + path5: "value", + }, + defaultValue: "will not be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": [ + { + "path4.path5": [ + { + path6: "value", + }, + ], + }, + ], + }, + path: "path1.path2.path3.0.path4.path5.0", + output: { + path6: "value", + }, + defaultValue: "will not be returned", + }, + { + inputObj: { + "path1.path2.path3": [ + { + ".path4.path5": [ + { + path6: "value", + }, + ], + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.path3.0..path4.path5.0", + output: { + path6: "value", + }, + defaultValue: "will not be returned", + }, + ].forEach((testObj: any) => { + expect( + getValueFromTree(testObj.inputObj, testObj.path, testObj.defaultValue), + ).toEqual(testObj.output); + }); + }); + + it("should test that default value is returned for an invalid path when object keys have dot", () => { + [ + //Path that has a primitive value as leaf node + { + inputObj: { + "path1.path2.path3": "value", + someotherPath: "testValue", + }, + path: "path1.path2.path4", + output: "value", + defaultValue: "will be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + "path1.path2": { + path3: "value", + }, + someotherPath: "testValue", + }, + path: "path1.path3.path4", + output: "value", + defaultValue: "will be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + path1: { + "path2.path3": "value", + }, + someotherPath: "testValue", + }, + path: "path1.path2", + output: "value", + defaultValue: "will be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + path1: { + path2: { + "path3.path4": "value", + }, + }, + someotherPath: "testValue", + }, + path: "path1.path2.path3", + output: "value", + defaultValue: "will be returned", + }, + //Path that has a primitive value as leaf node + { + inputObj: { + "path1.path2": { + "path3.path4": "value", + }, + someotherPath: "testValue", + }, + path: "path1.path3.path4", + output: "value", + defaultValue: "will be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": { + path4: "value", + }, + someotherPath: "testValue", + }, + path: "path1.path2", + output: { + path4: "value", + }, + defaultValue: "will be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + path1: { + "path2.path3": { + path4: "value", + }, + }, + someotherPath: "testValue", + }, + path: "path1.path2", + output: { + path4: "value", + }, + defaultValue: "will be returned", + }, + //Path that has a non primitive value as leaf node + { + inputObj: { + path1: { + path2: { + "path3.path4": { + path5: "value", + }, + }, + }, + someotherPath: "testValue", + }, + path: "path2.path3.path4", + output: { + path5: "value", + }, + defaultValue: "will be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: "value", + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.1.path3", + output: "value", + defaultValue: "will be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: { + path4: "value", + }, + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.1.path3.path4", + output: "value", + defaultValue: "will be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + "path3.path4": "value", + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.1.path3.path4", + output: "value", + defaultValue: "will be returned", + }, + //Path that traverse through an array with a primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: [ + { + path4: "value", + }, + ], + }, + ], + someotherPath: "testValue", + }, + path: "path1.path2.2.path3.0.path4", + output: "value", + defaultValue: "will be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2": [ + { + path3: { + path4: "value", + }, + }, + ], + someotherPath: "testValue", + }, + path: "path1.0.path3", + output: { + path4: "value", + }, + defaultValue: "will be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": [ + { + path4: "value", + }, + ], + }, + path: "path1.path2.0", + output: { + path4: "value", + }, + defaultValue: "will be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": [ + { + path4: [ + { + path5: "value", + }, + ], + }, + ], + }, + path: "path1.path2.0.path4.0", + output: { + path5: "value", + }, + defaultValue: "will be returned", + }, + //Path that traverse through an array with a non primitive value as leaf node + { + inputObj: { + "path1.path2.path3": [ + { + "path4.path5": [ + { + path6: "value", + }, + ], + }, + ], + }, + path: "path1.path2.path3.0.path4.0", + output: { + path6: "value", + }, + defaultValue: "will be returned", + }, + ].forEach((testObj: any) => { + expect( + getValueFromTree(testObj.inputObj, testObj.path, testObj.defaultValue), + ).toEqual(testObj.defaultValue); + }); + }); + + it("should check that invalid path strucutre should return defaultValue", () => { + [ + { + inputObj: { + path1: { + path2: { + path3: "value", + }, + }, + }, + path: "path1.path2..path3", + output: { + path6: "value", + }, + defaultValue: "will be returned", + }, + { + inputObj: { + path1: { + path2: [ + { + path3: "value", + }, + ], + }, + }, + path: "path1.path2.0..path3", + output: { + path6: "value", + }, + defaultValue: "will be returned", + }, + ].forEach((testObj: any) => { + expect( + getValueFromTree(testObj.inputObj, testObj.path, testObj.defaultValue), + ).toEqual(testObj.defaultValue); + }); + }); +}); diff --git a/app/client/src/sagas/WidgetOperationUtils.ts b/app/client/src/sagas/WidgetOperationUtils.ts index d2b2b3c95d..0690c16211 100644 --- a/app/client/src/sagas/WidgetOperationUtils.ts +++ b/app/client/src/sagas/WidgetOperationUtils.ts @@ -1381,3 +1381,63 @@ export function purgeOrphanedDynamicPaths(widget: WidgetProps) { } return widget; } + +/* + * Function to extend the lodash's get function to check + * paths which have dots in it's key + * + * Suppose, if the path is `path1.path2.path3.path4`, this function + * checks in following paths in the tree as well, if _.get doesn't return a value + * - path1.path2.path3 -> path4 + * - path1.path2 -> path3.path4 (will recursively traverse with same logic) + * - path1 -> path2.path3.path4 (will recursively traverse with same logic) + */ +export function getValueFromTree( + obj: Record, + path: string, + defaultValue?: unknown, +): unknown { + // Creating a symbol as we need a unique value that will not be present in the input obj + const defaultValueSymbol = Symbol("defaultValue"); + + //Call the original get function with defaultValueSymbol. + const value = _.get(obj, path, defaultValueSymbol); + + /* + * if the value returned by get matches defaultValueSymbol, + * path is invalid. + */ + if (value === defaultValueSymbol && path.includes(".")) { + const pathArray = path.split("."); + const poppedPath: Array = []; + + while (pathArray.length) { + const currentPath = pathArray.join("."); + + if (obj.hasOwnProperty(currentPath)) { + const currentValue = obj[currentPath]; + + if (!poppedPath.length) { + //Valid path + return currentValue; + } else if (typeof currentValue !== "object") { + //Invalid path + return defaultValue; + } else { + //Valid path, need to traverse recursively with same strategy + return getValueFromTree( + currentValue as Record, + poppedPath.join("."), + defaultValue, + ); + } + } else { + // We need the popped paths to traverse recursively + poppedPath.unshift(pathArray.pop() || ""); + } + } + } + + // Need to return the defaultValue, if there is no match for the path in the tree + return value !== defaultValueSymbol ? value : defaultValue; +}