Merge pull request #28064 from appsmithorg/hotfix/v1.9.40

chore: Hotfix/v1.9.40

app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts

@@ -90,4 +90,52 @@ describe("JSObject testing", () => {
         expect($label).contains("[]");
       });
   });
+
+  it("6. Bug 27978 Check assignment should not get overridden by evaluation", () => {
+    _.entityExplorer.DragNDropWidget(_.draggableWidgets.TEXT, 400, 400);
+    _.propPane.TypeTextIntoField(
+      "Text",
+      `{{JSObject1.data.length ? 'id-' + JSObject1.data[0].id : 'Not Set' }}`,
+      true,
+      false,
+    );
+    _.entityExplorer.NavigateToSwitcher("Explorer");
+    _.apiPage.CreateAndFillApi(
+      _.dataManager.dsValues[_.dataManager.defaultEnviorment].mockApiUrl,
+    );
+    const JS_OBJECT_BODY = `export default {
+      data: [],
+      async getData() {
+        await Api1.run()
+        return Api1.data
+      },
+      async myFun1() {
+        this.data = await this.getData();
+        console.log(this.data);
+      },
+      async myFun2() {
+        const data = await this.getData();
+        data.push({ name: "test123" })
+        this.data = data;
+        console.log(this.data);
+      },
+    }`;
+    _.jsEditor.CreateJSObject(JS_OBJECT_BODY, {
+      paste: true,
+      completeReplace: true,
+      toRun: false,
+      shouldCreateNewJSObj: true,
+    });
+    _.jsEditor.SelectFunctionDropdown("myFun1");
+    _.jsEditor.RunJSObj();
+    _.entityExplorer.SelectEntityByName("Text2", "Widgets");
+    _.agHelper.AssertContains("id-1");
+    cy.reload();
+    _.agHelper.AssertContains("Not Set");
+    _.entityExplorer.SelectEntityByName("JSObject1", "Queries/JS");
+    _.jsEditor.SelectFunctionDropdown("myFun2");
+    _.jsEditor.RunJSObj();
+    _.entityExplorer.SelectEntityByName("Text2", "Widgets");
+    _.agHelper.AssertContains("id-1");
+  });
 });

app/client/cypress_ci_custom.config.ts

@@ -58,6 +58,8 @@ export default defineConfig({
       "cypress/e2e/EE/Enterprise/MultipleEnv/ME_airtable_spec.ts",
       "cypress/e2e/Regression/ServerSide/Datasources/ElasticSearch_Basic_Spec.ts",
       "cypress/e2e/Regression/ServerSide/Datasources/Oracle_Spec.ts",
+      "cypress/e2e/Regression/ClientSide/Widgets/Others/MapWidget_Spec.ts",
+      "cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts",
     ],
   },
 });

app/client/src/ce/AppRouter.tsx

@@ -45,7 +45,10 @@ import * as Sentry from "@sentry/react";
 import { getSafeCrash, getSafeCrashCode } from "selectors/errorSelectors";
 import UserProfile from "pages/UserProfile";
 import { getCurrentUser } from "actions/authActions";
-import { getCurrentUserLoading } from "selectors/usersSelectors";
+import {
+  getCurrentUserLoading,
+  getFeatureFlagsFetching,
+} from "selectors/usersSelectors";
 import Setup from "pages/setup";
 import SettingsLoader from "pages/AdminSettings/loader";
 import SignupSuccess from "pages/setup/SignupSuccess";
@@ -153,6 +156,7 @@ function AppRouter(props: {
   } = props;
   const tenantIsLoading = useSelector(isTenantLoading);
   const currentUserIsLoading = useSelector(getCurrentUserLoading);
+  const featuresIsLoading = useSelector(getFeatureFlagsFetching);
 
   useEffect(() => {
     getCurrentUser();
@@ -166,7 +170,11 @@ function AppRouter(props: {
 
   // hide the top loader once the tenant is loaded
   useEffect(() => {
-    if (tenantIsLoading === false && currentUserIsLoading === false) {
+    if (
+      tenantIsLoading === false &&
+      currentUserIsLoading === false &&
+      featuresIsLoading === false
+    ) {
       const loader = document.getElementById("loader") as HTMLDivElement;
 
       if (loader) {
@@ -177,9 +185,9 @@ function AppRouter(props: {
         });
       }
     }
-  }, [tenantIsLoading, currentUserIsLoading]);
+  }, [tenantIsLoading, currentUserIsLoading, featuresIsLoading]);
 
-  if (tenantIsLoading || currentUserIsLoading) return null;
+  if (tenantIsLoading || currentUserIsLoading || featuresIsLoading) return null;
 
   return (
     <Router history={history}>

app/client/src/ce/actions/evaluationActions.ts

@@ -66,8 +66,6 @@ export const EVALUATE_REDUX_ACTIONS = [
   ReduxActionTypes.MOVE_ACTION_SUCCESS,
   ReduxActionTypes.RUN_ACTION_SUCCESS,
   ReduxActionErrorTypes.RUN_ACTION_ERROR,
-  ReduxActionTypes.EXECUTE_PLUGIN_ACTION_SUCCESS,
-  ReduxActionErrorTypes.EXECUTE_PLUGIN_ACTION_ERROR,
   ReduxActionTypes.CLEAR_ACTION_RESPONSE,
   // JS Actions
   ReduxActionTypes.CREATE_JS_ACTION_SUCCESS,

app/client/src/ce/sagas/tenantSagas.tsx

@@ -22,12 +22,22 @@ export function* fetchCurrentTenantConfigSaga() {
     );
     const isValidResponse: boolean = yield validateResponse(response);
     if (isValidResponse) {
-      const data: any = response.data;
+      const payload: any = response.data;
       yield put({
         type: ReduxActionTypes.FETCH_CURRENT_TENANT_CONFIG_SUCCESS,
-        payload: data,
+        payload: {
+          ...payload,
+          tenantConfiguration: {
+            ...CE_defaultBrandingConfig,
+            ...payload.tenantConfiguration,
+            brandColors: {
+              ...CE_defaultBrandingConfig.brandColors,
+              ...payload.tenantConfiguration.brandColors,
+            },
+          },
+        },
       });
-      AnalyticsUtil.initInstanceId(data.instanceId);
+      AnalyticsUtil.initInstanceId(payload.instanceId);
     }
   } catch (error) {
     yield put({

app/client/src/reducers/uiReducers/usersReducer.ts

@@ -24,6 +24,7 @@ const initialState: UsersReduxState = {
   featureFlag: {
     data: DEFAULT_FEATURE_FLAG_VALUE,
     isFetched: false,
+    isFetching: true,
   },
   productAlert: {
     config: {
@@ -173,6 +174,14 @@ const usersReducer = createReducer(initialState, {
       photoId: action.payload.photoId,
     },
   }),
+  [ReduxActionTypes.FETCH_FEATURE_FLAGS_INIT]: (state: UsersReduxState) => ({
+    ...state,
+    featureFlag: {
+      ...state.featureFlag,
+      isFetched: false,
+      isFetching: true,
+    },
+  }),
   [ReduxActionTypes.FETCH_FEATURE_FLAGS_SUCCESS]: (
     state: UsersReduxState,
     action: ReduxAction<FeatureFlags>,
@@ -181,6 +190,7 @@ const usersReducer = createReducer(initialState, {
     featureFlag: {
       data: action.payload,
       isFetched: true,
+      isFetching: false,
     },
   }),
   [ReduxActionErrorTypes.FETCH_FEATURE_FLAGS_ERROR]: (
@@ -190,6 +200,7 @@ const usersReducer = createReducer(initialState, {
     featureFlag: {
       data: {},
       isFetched: true,
+      isFetching: false,
     },
   }),
   [ReduxActionTypes.FETCH_PRODUCT_ALERT_SUCCESS]: (
@@ -252,6 +263,7 @@ export interface UsersReduxState {
   featureFlag: {
     isFetched: boolean;
     data: FeatureFlags;
+    isFetching: boolean;
   };
   productAlert: ProductAlertState;
 }

app/client/src/selectors/usersSelectors.tsx

@@ -5,16 +5,24 @@ import { ANONYMOUS_USERNAME } from "constants/userConstants";
 
 export const getCurrentUser = (state: AppState): User | undefined =>
   state.ui?.users?.currentUser;
+
 export const getCurrentUserLoading = (state: AppState): boolean =>
   state.ui.users.loadingStates.fetchingUser;
+
 export const getUserAuthError = (state: AppState): string =>
   state.ui.users.error;
+
 export const getUsers = (state: AppState): User[] => state.ui.users.users;
+
 export const getProppanePreference = (
   state: AppState,
 ): PropertyPanePositionConfig | undefined => state.ui.users.propPanePreferences;
+
 export const getFeatureFlagsFetched = (state: AppState) =>
   state.ui.users.featureFlag.isFetched;
 
+export const getFeatureFlagsFetching = (state: AppState) =>
+  state.ui.users.featureFlag.isFetching;
+
 export const getIsUserLoggedIn = (state: AppState): boolean =>
   state.ui.users.currentUser?.email !== ANONYMOUS_USERNAME;

app/client/src/widgets/WidgetUtils.test.ts

@@ -26,6 +26,7 @@ import {
   isAutoHeightEnabledForWidgetWithLimits,
   getWidgetMaxAutoHeight,
   getWidgetMinAutoHeight,
+  isCompactMode,
 } from "./WidgetUtils";
 import {
   getCustomTextColor,
@@ -696,4 +697,10 @@ describe("Should Update Widget Height Automatically?", () => {
     const result = shouldUpdateWidgetHeightAutomatically(input, props);
     expect(result).toStrictEqual(expected);
   });
+  it("should return correct value for isCompactMode", () => {
+    const compactHeight = 40;
+    const unCompactHeight = 41;
+    expect(isCompactMode(compactHeight)).toBeTruthy();
+    expect(isCompactMode(unCompactHeight)).toBeFalsy();
+  });
 });

app/client/src/widgets/WidgetUtils.ts

@@ -921,7 +921,7 @@ const findReactInstanceProps = (domElement: any) => {
 
 export function isCompactMode(componentHeight: number) {
   return (
-    componentHeight <
+    componentHeight <=
     COMPACT_MODE_MIN_ROWS * GridDefaults.DEFAULT_GRID_ROW_HEIGHT
   );
 }

app/client/src/workers/Evaluation/JSObject/Collection.ts

@@ -1,6 +1,20 @@
 import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils";
 import { klona } from "klona/full";
 import { get, set } from "lodash";
+import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter";
+import ExecutionMetaData from "../fns/utils/ExecutionMetaData";
+import type { JSActionEntity } from "@appsmith/entities/DataTree/types";
+
+export enum PatchType {
+  "SET" = "SET",
+  "GET" = "GET",
+}
+
+export interface Patch {
+  path: string;
+  method: PatchType;
+  value?: unknown;
+}
 
 export type VariableState = Record<string, Record<string, any>>;
 
@@ -61,6 +75,7 @@ export default class JSObjectCollection {
     const newVarState = { ...this.variableState[entityName] };
     newVarState[propertyPath] = variableValue;
     this.variableState[entityName] = newVarState;
+    JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName);
   }
 
   static getVariableState(
@@ -77,6 +92,53 @@ export default class JSObjectCollection {
       delete jsObject[propertyPath];
   }
 
+  /**Map<JSObjectName, Map<variableName, variableValue> */
+  static cachedJSVariablesByEntityName: Record<string, JSActionEntity> = {};
+
+  /**Computes  Map<JSObjectName, Map<variableName, variableValue> with getters & setters to track JS mutations
+   * We cache and reuse this map. We recreate only when the JSObject's content changes or when any of the variables
+   * gets evaluated
+   */
+  static getVariablesForEvaluationContext(entityName: string) {
+    if (JSObjectCollection.cachedJSVariablesByEntityName[entityName])
+      return JSObjectCollection.cachedJSVariablesByEntityName[entityName];
+    const varState = JSObjectCollection.getVariableState(entityName);
+    const variables = Object.entries(varState);
+    const newJSObject = {} as JSActionEntity;
+
+    for (const [varName, varValue] of variables) {
+      let variable = varValue;
+      Object.defineProperty(newJSObject, varName, {
+        enumerable: true,
+        configurable: true,
+        get() {
+          TriggerEmitter.emit(BatchKey.process_js_variable_updates, {
+            path: `${entityName}.${varName}`,
+            method: PatchType.GET,
+          });
+          return variable;
+        },
+        set(value) {
+          TriggerEmitter.emit(BatchKey.process_js_variable_updates, {
+            path: `${entityName}.${varName}`,
+            method: PatchType.SET,
+            value,
+          });
+          variable = value;
+        },
+      });
+    }
+    ExecutionMetaData.setExecutionMetaData({
+      enableJSVarUpdateTracking: true,
+    });
+    JSObjectCollection.cachedJSVariablesByEntityName[entityName] = newJSObject;
+    return JSObjectCollection.cachedJSVariablesByEntityName[entityName];
+  }
+
+  static clearCachedVariablesForEvaluationContext(entityName: string) {
+    delete JSObjectCollection.cachedJSVariablesByEntityName[entityName];
+  }
+
   static clear() {
     this.variableState = {};
     this.unEvalState = {};

app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts

@@ -1,52 +0,0 @@
-import { PatchType } from "./JSVariableUpdates";
-import ExecutionMetaData from "../fns/utils/ExecutionMetaData";
-import type { JSActionEntity } from "@appsmith/entities/DataTree/types";
-import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter";
-
-class JSFactory {
-  static create(
-    jsObjectName: string,
-    varState: Record<string, unknown> = {},
-  ): JSActionEntity {
-    const newJSObject: any = {};
-
-    const variables = Object.entries(varState);
-
-    for (const [varName, varValue] of variables) {
-      let variable = varValue;
-      Object.defineProperty(newJSObject, varName, {
-        enumerable: true,
-        configurable: true,
-        get() {
-          TriggerEmitter.emit(BatchKey.process_js_variable_updates, {
-            path: `${jsObjectName}.${varName}`,
-            method: PatchType.GET,
-          });
-          return variable;
-        },
-        set(value) {
-          TriggerEmitter.emit(BatchKey.process_js_variable_updates, {
-            path: `${jsObjectName}.${varName}`,
-            method: PatchType.SET,
-            value,
-          });
-          variable = value;
-        },
-      });
-
-      ExecutionMetaData.setExecutionMetaData({
-        enableJSVarUpdateTracking: false,
-      });
-
-      newJSObject[varName] = varValue;
-
-      ExecutionMetaData.setExecutionMetaData({
-        enableJSVarUpdateTracking: true,
-      });
-    }
-
-    return newJSObject;
-  }
-}
-
-export default JSFactory;

app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts

@@ -5,17 +5,8 @@ import { evalTreeWithChanges } from "../evalTreeWithChanges";
 import { get } from "lodash";
 import { isJSObjectVariable } from "./utils";
 import isDeepEqualES6 from "fast-deep-equal/es6";
-
+import type { Patch } from "./Collection";
-export enum PatchType {
+import { PatchType } from "./Collection";
-  "SET" = "SET",
-  "GET" = "GET",
-}
-
-export type Patch = {
-  path: string;
-  method: PatchType;
-  value?: unknown;
-};
 
 export type UpdatedPathsMap = Record<string, Patch>;
 

app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts

@@ -1,9 +1,9 @@
-import JSFactory from "../JSVariableFactory";
 import ExecutionMetaData from "workers/Evaluation/fns/utils/ExecutionMetaData";
 import type { JSActionEntity } from "@appsmith/entities/DataTree/types";
 import TriggerEmitter, {
   jsVariableUpdatesHandlerWrapper,
 } from "workers/Evaluation/fns/utils/TriggerEmitter";
+import JSObjectCollection from "../Collection";
 
 const applyJSVariableUpdatesToEvalTreeMock = jest.fn();
 jest.mock("../JSVariableUpdates.ts", () => ({
@@ -36,7 +36,12 @@ describe("JSVariableFactory", () => {
       weakSet: new WeakSet(),
     } as unknown as JSActionEntity;
 
-    const proxiedJSObject = JSFactory.create("JSObject1", jsObject);
+    Object.entries(jsObject).forEach(([k, v]) =>
+      JSObjectCollection.setVariableValue(v, `JSObject1.${k}`),
+    );
+
+    const proxiedJSObject =
+      JSObjectCollection.getVariablesForEvaluationContext("JSObject1");
 
     ExecutionMetaData.setExecutionMetaData({
       enableJSVarUpdateTracking: true,
@@ -96,7 +101,12 @@ describe("JSVariableFactory", () => {
       weakSet: new WeakSet(),
     } as unknown as JSActionEntity;
 
-    const proxiedJSObject = JSFactory.create("JSObject1", jsObject);
+    Object.entries(jsObject).forEach(([k, v]) =>
+      JSObjectCollection.setVariableValue(v, `JSObject1.${k}`),
+    );
+
+    const proxiedJSObject =
+      JSObjectCollection.getVariablesForEvaluationContext("JSObject1");
 
     ExecutionMetaData.setExecutionMetaData({
       enableJSVarUpdateTracking: false,
@@ -125,7 +135,12 @@ describe("JSVariableFactory", () => {
       weakSet: new WeakSet(),
     } as unknown as JSActionEntity;
 
-    const proxiedJSObject = JSFactory.create("JSObject1", jsObject);
+    Object.entries(jsObject).forEach(([k, v]) =>
+      JSObjectCollection.setVariableValue(v, `JSObject1.${k}`),
+    );
+
+    const proxiedJSObject =
+      JSObjectCollection.getVariablesForEvaluationContext("JSObject1");
 
     ExecutionMetaData.setExecutionMetaData({
       enableJSVarUpdateTracking: true,

app/client/src/workers/Evaluation/JSObject/index.ts

@@ -95,6 +95,7 @@ export function saveResolvedFunctionsAndJSUpdates(
     try {
       JSObjectCollection.deleteResolvedFunction(entityName);
       JSObjectCollection.deleteUnEvalState(entityName);
+      JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName);
 
       const parseStartTime = performance.now();
       const { parsedObject, success } = parseJSObject(entity.body);

app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts

@@ -1,10 +1,7 @@
 import { EventEmitter } from "events";
 import { MAIN_THREAD_ACTION } from "@appsmith/workers/Evaluation/evalWorkerActions";
 import { WorkerMessenger } from "workers/Evaluation/fns/utils/Messenger";
-import type {
+import type { UpdatedPathsMap } from "workers/Evaluation/JSObject/JSVariableUpdates";
-  Patch,
-  UpdatedPathsMap,
-} from "workers/Evaluation/JSObject/JSVariableUpdates";
 import { applyJSVariableUpdatesToEvalTree } from "workers/Evaluation/JSObject/JSVariableUpdates";
 import ExecutionMetaData from "./ExecutionMetaData";
 import { get } from "lodash";
@@ -18,6 +15,7 @@ import type {
 import type { UpdateActionProps } from "workers/Evaluation/handlers/updateActionData";
 import { handleActionsDataUpdate } from "workers/Evaluation/handlers/updateActionData";
 import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils";
+import type { Patch } from "workers/Evaluation/JSObject/Collection";
 
 const _internalSetTimeout = self.setTimeout;
 const _internalClearTimeout = self.clearTimeout;

app/client/src/workers/Evaluation/getEntityForContext.ts

@@ -4,7 +4,6 @@ import type {
 } from "@appsmith/entities/DataTree/types";
 import { ENTITY_TYPE_VALUE } from "entities/DataTree/dataTreeFactory";
 import JSObjectCollection from "./JSObject/Collection";
-import JSFactory from "./JSObject/JSVariableFactory";
 import { jsObjectFunctionFactory } from "./fns/utils/jsObjectFnFactory";
 import { isObject } from "lodash";
 
@@ -55,7 +54,8 @@ export function getEntityForEvalContext(
           return Object.assign({}, jsObject, fns);
         }
 
-        jsObjectForEval = JSFactory.create(entityName, jsObjectForEval);
+        jsObjectForEval =
+          JSObjectCollection.getVariablesForEvaluationContext(entityName);
         return Object.assign(jsObjectForEval, fns);
       }
     }