Merge pull request #14423 from Budibase/fix/conditions-on-views

Fix query conditions on views
Budibase · Aug 21, 2024 · 9f9c93b · 9f9c93b
2 parents f4484f4 + 2322925
commit 9f9c93b
Show file tree

Hide file tree

Showing 3 changed files with 180 additions and 72 deletions.
diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
@@ -339,7 +339,7 @@ class InternalBuilder {
     if (!filters) {
       return query
     }
-    filters = this.parseFilters(filters)
+    filters = this.parseFilters({ ...filters })
     const aliases = this.query.tableAliases
     // if all or specified in filters, then everything is an or
     const allOr = filters.allOr
@@ -468,18 +468,20 @@ class InternalBuilder {
 
     if (filters.$and) {
       const { $and } = filters
-      query = query.where(x => {
-        for (const condition of $and.conditions) {
-          x = this.addFilters(x, condition, opts)
-        }
-      })
+      for (const condition of $and.conditions) {
+        query = query.where(b => {
+          this.addFilters(b, condition, opts)
+        })
+      }
     }
 
     if (filters.$or) {
       const { $or } = filters
-      query = query.where(x => {
+      query = query.where(b => {
         for (const condition of $or.conditions) {
-          x = this.addFilters(x, { ...condition, allOr: true }, opts)
+          b.orWhere(c =>
+            this.addFilters(c, { ...condition, allOr: true }, opts)
+          )
         }
       })
     }

diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts
@@ -38,7 +38,6 @@ export async function searchView(
   let query = dataFilters.buildQuery(view.query || [])
   if (body.query) {
     // Delete extraneous search params that cannot be overridden
-    delete body.query.allOr
     delete body.query.onEmptyFilter
 
     if (!isExternalTableID(view.tableId) && !db.isSqsEnabledForTenant()) {

diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts
@@ -1492,83 +1492,189 @@ describe.each([
         )
       })
 
-      isLucene &&
-        it("in lucene, cannot override a view filter", async () => {
-          await config.api.row.save(table._id!, {
-            one: "foo",
-            two: "bar",
-          })
-          const two = await config.api.row.save(table._id!, {
-            one: "foo2",
-            two: "bar2",
-          })
+      it("can query on top of the view filters", async () => {
+        await config.api.row.save(table._id!, {
+          one: "foo",
+          two: "bar",
+        })
+        await config.api.row.save(table._id!, {
+          one: "foo2",
+          two: "bar2",
+        })
+        const three = await config.api.row.save(table._id!, {
+          one: "foo3",
+          two: "bar3",
+        })
 
-          const view = await config.api.viewV2.create({
-            tableId: table._id!,
-            name: generator.guid(),
-            query: [
-              {
-                operator: BasicOperator.EQUAL,
-                field: "two",
-                value: "bar2",
-              },
-            ],
-            schema: {
-              id: { visible: true },
-              one: { visible: false },
-              two: { visible: true },
+        const view = await config.api.viewV2.create({
+          tableId: table._id!,
+          name: generator.guid(),
+          query: [
+            {
+              operator: BasicOperator.NOT_EQUAL,
+              field: "one",
+              value: "foo2",
             },
-          })
+          ],
+          schema: {
+            id: { visible: true },
+            one: { visible: true },
+            two: { visible: true },
+          },
+        })
 
-          const response = await config.api.viewV2.search(view.id, {
-            query: {
-              equal: {
-                two: "bar",
-              },
+        const response = await config.api.viewV2.search(view.id, {
+          query: {
+            [BasicOperator.EQUAL]: {
+              two: "bar3",
             },
-          })
-          expect(response.rows).toHaveLength(1)
-          expect(response.rows).toEqual([
-            expect.objectContaining({ _id: two._id }),
-          ])
+            [BasicOperator.NOT_EMPTY]: {
+              two: null,
+            },
+          },
         })
+        expect(response.rows).toHaveLength(1)
+        expect(response.rows).toEqual(
+          expect.arrayContaining([expect.objectContaining({ _id: three._id })])
+        )
+      })
 
-      !isLucene &&
-        it("can filter a view without a view filter", async () => {
-          const one = await config.api.row.save(table._id!, {
-            one: "foo",
-            two: "bar",
-          })
-          await config.api.row.save(table._id!, {
-            one: "foo2",
-            two: "bar2",
-          })
+      it("can query on top of the view filters (using or filters)", async () => {
+        const one = await config.api.row.save(table._id!, {
+          one: "foo",
+          two: "bar",
+        })
+        await config.api.row.save(table._id!, {
+          one: "foo2",
+          two: "bar2",
+        })
+        const three = await config.api.row.save(table._id!, {
+          one: "foo3",
+          two: "bar3",
+        })
 
-          const view = await config.api.viewV2.create({
-            tableId: table._id!,
-            name: generator.guid(),
-            schema: {
-              id: { visible: true },
-              one: { visible: false },
-              two: { visible: true },
+        const view = await config.api.viewV2.create({
+          tableId: table._id!,
+          name: generator.guid(),
+          query: [
+            {
+              operator: BasicOperator.NOT_EQUAL,
+              field: "two",
+              value: "bar2",
             },
-          })
+          ],
+          schema: {
+            id: { visible: true },
+            one: { visible: false },
+            two: { visible: true },
+          },
+        })
 
-          const response = await config.api.viewV2.search(view.id, {
-            query: {
-              equal: {
-                two: "bar",
-              },
+        const response = await config.api.viewV2.search(view.id, {
+          query: {
+            allOr: true,
+            [BasicOperator.NOT_EQUAL]: {
+              two: "bar",
             },
-          })
-          expect(response.rows).toHaveLength(1)
-          expect(response.rows).toEqual([
+            [BasicOperator.NOT_EMPTY]: {
+              two: null,
+            },
+          },
+        })
+        expect(response.rows).toHaveLength(2)
+        expect(response.rows).toEqual(
+          expect.arrayContaining([
             expect.objectContaining({ _id: one._id }),
+            expect.objectContaining({ _id: three._id }),
           ])
-        })
+        )
+      })
+
+      isLucene &&
+        it.each([true, false])(
+          "in lucene, cannot override a view filter",
+          async allOr => {
+            await config.api.row.save(table._id!, {
+              one: "foo",
+              two: "bar",
+            })
+            const two = await config.api.row.save(table._id!, {
+              one: "foo2",
+              two: "bar2",
+            })
+
+            const view = await config.api.viewV2.create({
+              tableId: table._id!,
+              name: generator.guid(),
+              query: [
+                {
+                  operator: BasicOperator.EQUAL,
+                  field: "two",
+                  value: "bar2",
+                },
+              ],
+              schema: {
+                id: { visible: true },
+                one: { visible: false },
+                two: { visible: true },
+              },
+            })
+
+            const response = await config.api.viewV2.search(view.id, {
+              query: {
+                allOr,
+                equal: {
+                  two: "bar",
+                },
+              },
+            })
+            expect(response.rows).toHaveLength(1)
+            expect(response.rows).toEqual([
+              expect.objectContaining({ _id: two._id }),
+            ])
+          }
+        )
+
+      !isLucene &&
+        it.each([true, false])(
+          "can filter a view without a view filter",
+          async allOr => {
+            const one = await config.api.row.save(table._id!, {
+              one: "foo",
+              two: "bar",
+            })
+            await config.api.row.save(table._id!, {
+              one: "foo2",
+              two: "bar2",
+            })
+
+            const view = await config.api.viewV2.create({
+              tableId: table._id!,
+              name: generator.guid(),
+              schema: {
+                id: { visible: true },
+                one: { visible: false },
+                two: { visible: true },
+              },
+            })
+
+            const response = await config.api.viewV2.search(view.id, {
+              query: {
+                allOr,
+                equal: {
+                  two: "bar",
+                },
+              },
+            })
+            expect(response.rows).toHaveLength(1)
+            expect(response.rows).toEqual([
+              expect.objectContaining({ _id: one._id }),
+            ])
+          }
+        )
 
       !isLucene &&
-        it("cannot bypass a view filter", async () => {
+        it.each([true, false])("cannot bypass a view filter", async allOr => {
           await config.api.row.save(table._id!, {
             one: "foo",
             two: "bar",
@@ -1597,6 +1703,7 @@ describe.each([
 
           const response = await config.api.viewV2.search(view.id, {
             query: {
+              allOr,
               equal: {
                 two: "bar",
               },