From a0fe26cee6d5a8f96e60fd606edc1c4354288277 Mon Sep 17 00:00:00 2001 From: sneha122 Date: Wed, 26 Jul 2023 16:36:58 +0530 Subject: [PATCH] feat: trigger API extended to fetch sheet data (#25681) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR extends the trigger API call, it introduces another trigger method called `TRIGGER_SHEET_DATA`, This method allows us to fetch sheet data which we need to show on datasource review page as shown below: ![Screenshot 2023-07-25 at 2 42 35 PM](https://github.com/appsmithorg/appsmith/assets/30018882/91a0956e-2171-472d-80f5-b31014dd482c) #### PR fixes following issue(s) Fixes #25645 > if no issue exists, please create an issue and ask the maintainers about this first > > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change - New feature (non-breaking change which adds functionality) > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [x] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --------- Co-authored-by: “sneha122” <“sneha@appsmith.com”> --- .../config/GoogleSheetsMethodStrategy.java | 2 + .../com/external/config/MethodConfig.java | 3 + .../external/config/MethodIdentifiers.java | 1 + .../com/external/config/RowsGetMethod.java | 18 ++++- .../external/config/RowsGetMethodTest.java | 75 +++++++++++++++++++ 5 files changed, 98 insertions(+), 1 deletion(-) diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GoogleSheetsMethodStrategy.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GoogleSheetsMethodStrategy.java index 6908f4d006..87cbf3e203 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GoogleSheetsMethodStrategy.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/GoogleSheetsMethodStrategy.java @@ -65,6 +65,8 @@ public class GoogleSheetsMethodStrategy { return new FileInfoMethod(objectMapper); case MethodIdentifiers.TRIGGER_COLUMNS_SELECTOR: return new GetStructureMethod(objectMapper); + case MethodIdentifiers.TRIGGER_SHEET_DATA: + return new RowsGetMethod(objectMapper); default: throw Exceptions.propagate(new AppsmithPluginException( AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR, diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodConfig.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodConfig.java index e3edd66e78..92807f509c 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodConfig.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodConfig.java @@ -25,6 +25,7 @@ import static com.appsmith.external.helpers.PluginUtils.getTrimmedStringDataValu import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormDataAsString; import static com.appsmith.external.helpers.PluginUtils.parseWhereClause; import static com.appsmith.external.helpers.PluginUtils.validDataConfigurationPresentInFormData; +import static com.external.constants.FieldName.QUERY_FORMAT; import static com.external.constants.FieldName.SHEET_NAME; import static com.external.constants.FieldName.SHEET_URL; import static com.external.constants.FieldName.TABLE_HEADER_INDEX; @@ -100,6 +101,8 @@ public class MethodConfig { public MethodConfig(TriggerRequestDTO triggerRequestDTO) { final Map parameters = triggerRequestDTO.getParameters(); switch (parameters.size()) { + case 4: + this.queryFormat = getValueSafelyFromFormDataAsString(parameters, QUERY_FORMAT); case 3: case 2: this.tableHeaderIndex = getValueSafelyFromFormDataAsString(parameters, TABLE_HEADER_INDEX); diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodIdentifiers.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodIdentifiers.java index 81bfd607b6..6291d81bde 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodIdentifiers.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/MethodIdentifiers.java @@ -20,4 +20,5 @@ public class MethodIdentifiers { public static final String TRIGGER_SPREADSHEET_SELECTOR = "SPREADSHEET_SELECTOR"; public static final String TRIGGER_SHEET_SELECTOR = "SHEET_SELECTOR"; public static final String TRIGGER_COLUMNS_SELECTOR = "COLUMNS_SELECTOR"; + public static final String TRIGGER_SHEET_DATA = "SHEET_DATA"; } diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/RowsGetMethod.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/RowsGetMethod.java index 49fdc3e19b..bf007ac06b 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/RowsGetMethod.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/config/RowsGetMethod.java @@ -29,7 +29,7 @@ import java.util.regex.Pattern; * API reference: https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/get */ @Slf4j -public class RowsGetMethod implements ExecutionMethod, TemplateMethod { +public class RowsGetMethod implements ExecutionMethod, TemplateMethod, TriggerMethod { ObjectMapper objectMapper; FilterDataService filterDataService; @@ -193,6 +193,22 @@ public class RowsGetMethod implements ExecutionMethod, TemplateMethod { return preFilteringResponse; } + @Override + public boolean validateTriggerMethodRequest(MethodConfig methodConfig) { + return this.validateExecutionMethodRequest(methodConfig); + } + + @Override + public WebClient.RequestHeadersSpec getTriggerClient(WebClient webClient, MethodConfig methodConfig) { + return this.getExecutionClient(webClient, methodConfig); + } + + @Override + public JsonNode transformTriggerResponse( + JsonNode response, MethodConfig methodConfig, Set userAuthorizedSheetIds) { + return this.transformExecutionResponse(response, methodConfig, userAuthorizedSheetIds); + } + private Set sanitizeHeaders(ArrayNode headers, int valueSize) { final Set headerSet = new LinkedHashSet<>(); int headerSize = headers.size(); diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/RowsGetMethodTest.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/RowsGetMethodTest.java index 716afcd65a..58542eec50 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/RowsGetMethodTest.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/RowsGetMethodTest.java @@ -1,6 +1,7 @@ package com.external.config; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.external.models.TriggerRequestDTO; import com.external.constants.ErrorMessages; import com.external.constants.FieldName; import com.fasterxml.jackson.core.JsonProcessingException; @@ -8,10 +9,12 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; +import java.util.HashMap; import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class RowsGetMethodTest { @@ -168,4 +171,76 @@ public class RowsGetMethodTest { assertEquals(3, result.size()); assertEquals(0, result.get(0).get(FieldName.ROW_INDEX).asInt()); } + + @Test + public void testValidateExecutionMethodRequest_noSpreadsheetId_returnsException() throws JsonProcessingException { + ObjectMapper objectMapper = new ObjectMapper(); + RowsGetMethod rowsGetMethod = new RowsGetMethod(objectMapper); + TriggerRequestDTO triggerRequest = new TriggerRequestDTO(); + Map params = new HashMap(); + + triggerRequest.setParameters(params); + + MethodConfig methodConfig = new MethodConfig(triggerRequest); + final AppsmithPluginException exception = assertThrows( + AppsmithPluginException.class, () -> rowsGetMethod.validateExecutionMethodRequest(methodConfig)); + + assertEquals("Missing required field 'Spreadsheet Url'", exception.getMessage()); + } + + @Test + public void testValidateExecutionMethodRequest_noSheetName_returnsException() throws JsonProcessingException { + ObjectMapper objectMapper = new ObjectMapper(); + RowsGetMethod rowsGetMethod = new RowsGetMethod(objectMapper); + TriggerRequestDTO triggerRequest = new TriggerRequestDTO(); + Map params = new HashMap(); + params.put("sheetUrl", "https://docs.google.com/spreadsheets/d/spreadsheetId/"); + + triggerRequest.setParameters(params); + + MethodConfig methodConfig = new MethodConfig(triggerRequest); + final AppsmithPluginException exception = assertThrows( + AppsmithPluginException.class, () -> rowsGetMethod.validateExecutionMethodRequest(methodConfig)); + + assertEquals("Missing required field 'Spreadsheet Name'", exception.getMessage()); + } + + @Test + public void testValidateExecutionMethodRequest_noTableHeaderIndex_returnsException() + throws JsonProcessingException { + ObjectMapper objectMapper = new ObjectMapper(); + RowsGetMethod rowsGetMethod = new RowsGetMethod(objectMapper); + TriggerRequestDTO triggerRequest = new TriggerRequestDTO(); + Map params = new HashMap(); + params.put("sheetUrl", "https://docs.google.com/spreadsheets/d/spreadsheetId/"); + params.put("queryFormat", "ROWS"); + params.put("sheetName", "sample_sheet_name"); + + triggerRequest.setParameters(params); + + MethodConfig methodConfig = new MethodConfig(triggerRequest); + final AppsmithPluginException exception = assertThrows( + AppsmithPluginException.class, () -> rowsGetMethod.validateExecutionMethodRequest(methodConfig)); + + assertEquals( + "Unexpected value for table header index. Please use a number starting from 1", exception.getMessage()); + } + + @Test + public void testValidateExecutionMethodRequest_allParamsPresent_returnsTrue() throws JsonProcessingException { + ObjectMapper objectMapper = new ObjectMapper(); + RowsGetMethod rowsGetMethod = new RowsGetMethod(objectMapper); + TriggerRequestDTO triggerRequest = new TriggerRequestDTO(); + Map params = new HashMap(); + params.put("sheetUrl", "https://docs.google.com/spreadsheets/d/spreadsheetId/"); + params.put("queryFormat", "ROWS"); + params.put("sheetName", "sample_sheet_name"); + params.put("tableHeaderIndex", 1); + + triggerRequest.setParameters(params); + + MethodConfig methodConfig = new MethodConfig(triggerRequest); + Boolean actualResult = rowsGetMethod.validateExecutionMethodRequest(methodConfig); + assertEquals(true, actualResult); + } }