Merge pull request #13456 from Budibase/more-sqs-tests

Assert length on search tests, fixes bug in SQS around `onEmptyReturn`

packages/server/src/api/controllers/row/external.ts

@@ -17,11 +17,9 @@ import {
   Row,
   Table,
   UserCtx,
-  EmptyFilterOption,
 } from "@budibase/types"
 import sdk from "../../../sdk"
 import * as utils from "./utils"
-import { dataFilters } from "@budibase/shared-core"
 import {
   inputProcessing,
   outputProcessing,
@@ -33,17 +31,6 @@ export async function handleRequest<T extends Operation>(
   tableId: string,
   opts?: RunConfig
 ): Promise<ExternalRequestReturnType<T>> {
-  // make sure the filters are cleaned up, no empty strings for equals, fuzzy or string
-  if (opts && opts.filters) {
-    opts.filters = sdk.rows.removeEmptyFilters(opts.filters)
-  }
-  if (
-    !dataFilters.hasFilters(opts?.filters) &&
-    opts?.filters?.onEmptyFilter === EmptyFilterOption.RETURN_NONE
-  ) {
-    return [] as any
-  }
-
   return new ExternalRequest<T>(operation, tableId, opts?.datasource).run(
     opts || {}
   )

packages/server/src/api/routes/tests/search.spec.ts

@@ -6,6 +6,7 @@ import {
   Datasource,
   EmptyFilterOption,
   FieldType,
+  Row,
   SearchFilters,
   Table,
 } from "@budibase/types"
@@ -47,7 +48,7 @@ describe.each([
   })
 
   describe("strings", () => {
-    beforeEach(async () => {
+    beforeAll(async () => {
       table = await config.api.table.save(
         tableForDatasource(datasource, {
           schema: {
@@ -61,6 +62,13 @@ describe.each([
     })
 
     const rows = [{ name: "foo" }, { name: "bar" }]
+    let savedRows: Row[]
+
+    beforeAll(async () => {
+      savedRows = await Promise.all(
+        rows.map(r => config.api.row.save(table._id!, r))
+      )
+    })
 
     interface StringSearchTest {
       query: SearchFilters
@@ -68,6 +76,8 @@ describe.each([
     }
 
     const stringSearchTests: StringSearchTest[] = [
+      // These three test cases are generic and don't really need
+      // to be repeated for all data types, so we just do them here.
       { query: {}, expected: rows },
       {
         query: { onEmptyFilter: EmptyFilterOption.RETURN_ALL },
@@ -77,6 +87,7 @@ describe.each([
         query: { onEmptyFilter: EmptyFilterOption.RETURN_NONE },
         expected: [],
       },
+      // The rest of these tests are specific to strings.
       { query: { string: { name: "foo" } }, expected: [rows[0]] },
       { query: { string: { name: "none" } }, expected: [] },
       { query: { fuzzy: { name: "oo" } }, expected: [rows[0]] },
@@ -88,13 +99,11 @@ describe.each([
     it.each(stringSearchTests)(
       `should be able to run query: $query`,
       async ({ query, expected }) => {
-        const savedRows = await Promise.all(
-          rows.map(r => config.api.row.save(table._id!, r))
-        )
         const { rows: foundRows } = await config.api.row.search(table._id!, {
           tableId: table._id!,
           query,
         })
+        expect(foundRows).toHaveLength(expected.length)
         expect(foundRows).toEqual(
           expect.arrayContaining(
             expected.map(r =>
@@ -107,7 +116,7 @@ describe.each([
   })
 
   describe("number", () => {
-    beforeEach(async () => {
+    beforeAll(async () => {
       table = await config.api.table.save(
         tableForDatasource(datasource, {
           schema: {
@@ -121,6 +130,13 @@ describe.each([
     })
 
     const rows = [{ age: 1 }, { age: 10 }]
+    let savedRows: Row[]
+
+    beforeAll(async () => {
+      savedRows = await Promise.all(
+        rows.map(r => config.api.row.save(table._id!, r))
+      )
+    })
 
     interface NumberSearchTest {
       query: SearchFilters
@@ -128,15 +144,6 @@ describe.each([
     }
 
     const numberSearchTests: NumberSearchTest[] = [
-      { query: {}, expected: rows },
-      {
-        query: { onEmptyFilter: EmptyFilterOption.RETURN_ALL },
-        expected: rows,
-      },
-      {
-        query: { onEmptyFilter: EmptyFilterOption.RETURN_NONE },
-        expected: [],
-      },
       { query: { equal: { age: 1 } }, expected: [rows[0]] },
       { query: { equal: { age: 2 } }, expected: [] },
       { query: { notEqual: { age: 1 } }, expected: [rows[1]] },
@@ -150,13 +157,11 @@ describe.each([
     it.each(numberSearchTests)(
       `should be able to run query: $query`,
       async ({ query, expected }) => {
-        const savedRows = await Promise.all(
-          rows.map(r => config.api.row.save(table._id!, r))
-        )
         const { rows: foundRows } = await config.api.row.search(table._id!, {
           tableId: table._id!,
           query,
         })
+        expect(foundRows).toHaveLength(expected.length)
         expect(foundRows).toEqual(
           expect.arrayContaining(
             expected.map(r =>
@@ -186,6 +191,13 @@ describe.each([
       { dob: new Date("2020-01-01").toISOString() },
       { dob: new Date("2020-01-10").toISOString() },
     ]
+    let savedRows: Row[]
+
+    beforeEach(async () => {
+      savedRows = await Promise.all(
+        rows.map(r => config.api.row.save(table._id!, r))
+      )
+    })
 
     interface DateSearchTest {
       query: SearchFilters
@@ -193,15 +205,6 @@ describe.each([
     }
 
     const dateSearchTests: DateSearchTest[] = [
-      { query: {}, expected: rows },
-      {
-        query: { onEmptyFilter: EmptyFilterOption.RETURN_ALL },
-        expected: rows,
-      },
-      {
-        query: { onEmptyFilter: EmptyFilterOption.RETURN_NONE },
-        expected: [],
-      },
       {
         query: { equal: { dob: new Date("2020-01-01").toISOString() } },
         expected: [rows[0]],
@@ -256,13 +259,11 @@ describe.each([
     it.each(dateSearchTests)(
       `should be able to run query: $query`,
       async ({ query, expected }) => {
-        const savedRows = await Promise.all(
-          rows.map(r => config.api.row.save(table._id!, r))
-        )
         const { rows: foundRows } = await config.api.row.search(table._id!, {
           tableId: table._id!,
           query,
         })
+        expect(foundRows).toHaveLength(expected.length)
         expect(foundRows).toEqual(
           expect.arrayContaining(
             expected.map(r =>

packages/server/src/integrations/base/sql.ts

@@ -334,6 +334,7 @@ class InternalBuilder {
     if (filters.containsAny) {
       contains(filters.containsAny, true)
     }
+
     return query
   }
 

packages/server/src/sdk/app/rows/search.ts

@@ -1,4 +1,5 @@
 import {
+  EmptyFilterOption,
   Row,
   RowSearchParams,
   SearchFilters,
@@ -11,6 +12,7 @@ import { NoEmptyFilterStrings } from "../../../constants"
 import * as sqs from "./search/sqs"
 import env from "../../../environment"
 import { ExportRowsParams, ExportRowsResult } from "./search/types"
+import { dataFilters } from "@budibase/shared-core"
 
 export { isValidFilter } from "../../../integrations/utils"
 
@@ -60,6 +62,16 @@ export async function search(
   options: RowSearchParams
 ): Promise<SearchResponse<Row>> {
   const isExternalTable = isExternalTableID(options.tableId)
+  options.query = removeEmptyFilters(options.query || {})
+  if (
+    !dataFilters.hasFilters(options.query) &&
+    options.query.onEmptyFilter === EmptyFilterOption.RETURN_NONE
+  ) {
+    return {
+      rows: [],
+    }
+  }
+
   if (isExternalTable) {
     return external.search(options)
   } else if (env.SQS_SEARCH_ENABLE) {