fix: gsheet cell range filter format issue fixed (#26827)

## Description

This PR fixes the issue with filter format in gsheet query creation:

The issue occurs when say the spreadsheet has data in the following
format:

<img width="258" alt="Screenshot 2023-08-31 at 3 09 03 PM"
src="https://github.com/appsmithorg/appsmith/assets/30018882/88f8fdae-4223-468a-b05d-04fd8e9841dc">

Now if we write a query with where clause to to fetch all records with
Id < 3, it returns first two records as expected. Now if we change the
filter format to cell range and define cell range as b2:b4, It should
return `Name: test1 test2 test3`, but instead it throws error, because
it first filters out data based on cell range provided, and post that it
checks if the where conditions are defined in actionConfig and applies
that on the result set to filter data. In above example result set would
be `Name: test1 test2 test3` and it tries to apply the where clause
condition of `Id < 2`

In order to solve the issue, we need to apply where conditions only when
filter format is `Where Clause`.

#### PR fixes following issue(s)
Fixes #25447
> 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
- Bug fix (non-breaking change which fixes an issue)
>
>
>
## 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
- [ ] JUnit
- [ ] Jest
- [x] 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
- [ ] 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:
- [x] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [x] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [x] Test plan has been peer reviewed by project stakeholders and other
QA members
- [x] 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”>
This commit is contained in:
sneha122 2023-09-01 18:50:21 +05:30 committed by GitHub
parent 939cd2a217
commit 22cd648c7d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 55 additions and 57 deletions

View File

@ -173,21 +173,20 @@ describe("GSheet-Functional Tests With All Access", function () {
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "87bbb472ef9d90dcef140a551665c929");
// Currently commenting this until https://github.com/appsmithorg/appsmith/issues/25447 is fixed.
// Filter by cell range and verify
// dataSources.ValidateNSelectDropdown(
// "Filter Format",
// "Where Clause",
// "Cell range",
// );
// agHelper.EnterValue("A2:A5", {
// propFieldName: "",
// directInput: false,
// inputFieldName: "Cell range",
// });
// dataSources.RunQuery();
// dataSources.RunQueryNVerifyResponseViews(8);
// dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
dataSources.ValidateNSelectDropdown(
"Filter Format",
"Where Clause",
"Cell range",
);
agHelper.EnterValue("A2:A5", {
propFieldName: "",
directInput: false,
inputFieldName: "Cell range",
});
dataSources.RunQuery();
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
});
it("5. Update a record which is not present and verify the error", () => {

View File

@ -170,21 +170,20 @@ describe("GSheet-Functional Tests With Read/Write Access", function () {
dataSources.RunQuery();
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "87bbb472ef9d90dcef140a551665c929");
// Currently commenting this until https://github.com/appsmithorg/appsmith/issues/25447 is fixed.
// Filter by cell range and verify
// dataSources.ValidateNSelectDropdown(
// "Filter Format",
// "Where Clause",
// "Cell range",
// );
// agHelper.EnterValue("A2:A5", {
// propFieldName: "",
// directInput: false,
// inputFieldName: "Cell range",
// });
// dataSources.RunQuery();
// dataSources.RunQueryNVerifyResponseViews(8);
// dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
dataSources.ValidateNSelectDropdown(
"Filter Format",
"Where Clause",
"Cell range",
);
agHelper.EnterValue("A2:A5", {
propFieldName: "",
directInput: false,
inputFieldName: "Cell range",
});
dataSources.RunQuery();
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
});
it("5. Update a record which is not present and verify the error", () => {

View File

@ -178,21 +178,20 @@ describe("GSheet-Functional Tests With Read Access", function () {
dataSources.RunQuery();
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "87bbb472ef9d90dcef140a551665c929");
// Currently commenting this until https://github.com/appsmithorg/appsmith/issues/25447 is fixed.
// Filter by cell range and verify
// dataSources.ValidateNSelectDropdown(
// "Filter Format",
// "Where Clause",
// "Cell range",
// );
// agHelper.EnterValue("A2:A5", {
// propFieldName: "",
// directInput: false,
// inputFieldName: "Cell range",
// });
// dataSources.RunQuery();
// dataSources.RunQueryNVerifyResponseViews(8);
// dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
dataSources.ValidateNSelectDropdown(
"Filter Format",
"Where Clause",
"Cell range",
);
agHelper.EnterValue("A2:A5", {
propFieldName: "",
directInput: false,
inputFieldName: "Cell range",
});
dataSources.RunQuery();
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
});
it("5. Convert field to JS and verify", () => {

View File

@ -161,21 +161,20 @@ describe("GSheet-Functional Tests With Selected Access", function () {
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "87bbb472ef9d90dcef140a551665c929");
// Currently commenting this until https://github.com/appsmithorg/appsmith/issues/25447 is fixed.
// Filter by cell range and verify
// dataSources.ValidateNSelectDropdown(
// "Filter Format",
// "Where Clause",
// "Cell range",
// );
// agHelper.EnterValue("A2:A5", {
// propFieldName: "",
// directInput: false,
// inputFieldName: "Cell range",
// });
// dataSources.RunQuery();
// dataSources.RunQueryNVerifyResponseViews(8);
// dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
dataSources.ValidateNSelectDropdown(
"Filter Format",
"Where Clause",
"Cell range",
);
agHelper.EnterValue("A2:A5", {
propFieldName: "",
directInput: false,
inputFieldName: "Cell range",
});
dataSources.RunQuery();
dataSources.RunQueryNVerifyResponseViews(8);
dataSources.AssertQueryTableResponse(0, "eac7efa5dbd3d667f26eb3d3ab504464");
});
it("5. Update a record which is not present and verify the error", () => {

View File

@ -179,7 +179,9 @@ public class RowsGetMethod implements ExecutionMethod, TemplateMethod, TriggerMe
ArrayNode preFilteringResponse = this.objectMapper.valueToTree(collectedCells);
if (isWhereConditionConfigured(methodConfig)) {
// where condition needs to applied only when the filter format is where clause
// For filter format of cell range, we do not need to apply where clause
if (isWhereConditionConfigured(methodConfig) && "ROWS".equalsIgnoreCase(methodConfig.getQueryFormat())) {
return filterDataService.filterDataNew(
preFilteringResponse,
new UQIDataFilterParams(