Merge branch 'appsmithorg:release' into release

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

@@ -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;
     });
   });

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

@@ -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);
+  });
+});

app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/constants/ErrorMessages.java

@@ -39,6 +39,9 @@ public class ErrorMessages extends BasePluginErrorMessages {
 
     public static final String MISSING_ROW_INDEX_ERROR_MSG = "Missing required field 'Row index'";
 
+    public static final String MISSING_SPREADSHEET_URL_SELECTED_SHEETS_ERROR_MSG =
+            "Missing required field 'Spreadsheet Url'. Please check if your datasource is authorized to use this spreadsheet.";
+
     public static final String UNABLE_TO_CREATE_URI_ERROR_MSG = "Unable to create URI";
 
     public static final String MISSING_VALID_RESPONSE_ERROR_MSG = "Missing a valid response object.";

app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java

@@ -51,7 +51,7 @@
 import static com.appsmith.external.helpers.PluginUtils.getDataValueSafelyFromFormData;
 import static com.appsmith.external.helpers.PluginUtils.setDataValueSafelyInFormData;
 import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
-import static com.external.utils.SheetsUtil.getUserAuthorizedSheetIds;
+import static com.external.utils.SheetsUtil.validateAndGetUserAuthorizedSheetIds;
 import static java.lang.Boolean.TRUE;
 
 @Slf4j
@@ -175,7 +175,8 @@ public Mono<ActionExecutionResult> executeCommon(
 
             // This will get list of authorised sheet ids from datasource config, and transform execution response to
             // contain only authorised files
-            final Set<String> userAuthorizedSheetIds = getUserAuthorizedSheetIds(datasourceConfiguration);
+            final Set<String> userAuthorizedSheetIds =
+                    validateAndGetUserAuthorizedSheetIds(datasourceConfiguration, methodConfig);
 
             // Triggering the actual REST API call
             return executionMethod
@@ -338,7 +339,8 @@ public Mono<TriggerResultDTO> trigger(
 
             // This will get list of authorised sheet ids from datasource config, and transform trigger response to
             // contain only authorised files
-            Set<String> userAuthorizedSheetIds = getUserAuthorizedSheetIds(datasourceConfiguration);
+            Set<String> userAuthorizedSheetIds =
+                    validateAndGetUserAuthorizedSheetIds(datasourceConfiguration, methodConfig);
 
             return triggerMethod
                     .getTriggerClient(client, methodConfig)

app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/utils/SheetsUtil.java

@@ -1,7 +1,11 @@
 package com.external.utils;
 
+import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
+import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
 import com.appsmith.external.models.DatasourceConfiguration;
 import com.appsmith.external.models.OAuth2;
+import com.external.config.MethodConfig;
+import com.external.constants.ErrorMessages;
 import com.external.enums.GoogleSheetMethodEnum;
 import com.fasterxml.jackson.databind.JsonNode;
 
@@ -17,8 +21,10 @@ public class SheetsUtil {
     private static final String FILE_SPECIFIC_DRIVE_SCOPE = "https://www.googleapis.com/auth/drive.file";
     private static final int USER_AUTHORIZED_SHEET_IDS_INDEX = 1;
 
-    public static Set<String> getUserAuthorizedSheetIds(DatasourceConfiguration datasourceConfiguration) {
+    public static Set<String> validateAndGetUserAuthorizedSheetIds(
+            DatasourceConfiguration datasourceConfiguration, MethodConfig methodConfig) {
         OAuth2 oAuth2 = (OAuth2) datasourceConfiguration.getAuthentication();
+        Set<String> userAuthorisedSheetIds = null;
         if (!isEmpty(datasourceConfiguration.getProperties())
                 && datasourceConfiguration.getProperties().size() > 1
                 && datasourceConfiguration.getProperties().get(USER_AUTHORIZED_SHEET_IDS_INDEX) != null
@@ -33,9 +39,23 @@ public static Set<String> getUserAuthorizedSheetIds(DatasourceConfiguration data
                     .getProperties()
                     .get(USER_AUTHORIZED_SHEET_IDS_INDEX)
                     .getValue();
-            return new HashSet<String>(temp);
+            userAuthorisedSheetIds = new HashSet<String>(temp);
+
+            // This is added specifically for selected gsheets, so that whenever authorisation changes from one sheet to
+            // another
+            // We throw an error, this is done because when we use drive.file scope which is for selected sheets through
+            // file picker
+            // The access token for this scope grants access to all selected sheets across datasources
+            // we want to constraint the access for datasource to the sheet which was selected during ds authorisation
+            if (methodConfig != null
+                    && methodConfig.getSpreadsheetId() != null
+                    && !userAuthorisedSheetIds.contains(methodConfig.getSpreadsheetId())) {
+                throw new AppsmithPluginException(
+                        AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR,
+                        ErrorMessages.MISSING_SPREADSHEET_URL_SELECTED_SHEETS_ERROR_MSG);
+            }
         }
-        return null;
+        return userAuthorisedSheetIds;
     }
 
     public static Map<String, String> getSpreadsheetData(