Merge pull request #13614 from Budibase/search-tests-boolean

Tests for searching boolean fields, including fixes for various bugs.

packages/backend-core/src/db/lucene.ts

@@ -12,6 +12,10 @@ import { dataFilters } from "@budibase/shared-core"
 
 export const removeKeyNumbering = dataFilters.removeKeyNumbering
 
+function isEmpty(value: any) {
+  return value == null || value === ""
+}
+
 /**
  * Class to build lucene query URLs.
  * Optionally takes a base lucene query object.
@@ -282,15 +286,14 @@ export class QueryBuilder<T> {
     }
 
     const equal = (key: string, value: any) => {
-      // 0 evaluates to false, which means we would return all rows if we don't check it
+      if (isEmpty(value)) {
-      if (!value && value !== 0) {
         return null
       }
       return `${key}:${builder.preprocess(value, allPreProcessingOpts)}`
     }
 
     const contains = (key: string, value: any, mode = "AND") => {
-      if (!value || (Array.isArray(value) && value.length === 0)) {
+      if (isEmpty(value)) {
         return null
       }
       if (!Array.isArray(value)) {
@@ -306,7 +309,7 @@ export class QueryBuilder<T> {
     }
 
     const fuzzy = (key: string, value: any) => {
-      if (!value) {
+      if (isEmpty(value)) {
         return null
       }
       value = builder.preprocess(value, {
@@ -328,7 +331,7 @@ export class QueryBuilder<T> {
     }
 
     const oneOf = (key: string, value: any) => {
-      if (!value) {
+      if (isEmpty(value)) {
         return `*:*`
       }
       if (!Array.isArray(value)) {
@@ -386,7 +389,7 @@ export class QueryBuilder<T> {
     // Construct the actual lucene search query string from JSON structure
     if (this.#query.string) {
       build(this.#query.string, (key: string, value: any) => {
-        if (!value) {
+        if (isEmpty(value)) {
           return null
         }
         value = builder.preprocess(value, {
@@ -399,7 +402,7 @@ export class QueryBuilder<T> {
     }
     if (this.#query.range) {
       build(this.#query.range, (key: string, value: any) => {
-        if (!value) {
+        if (isEmpty(value)) {
           return null
         }
         if (value.low == null || value.low === "") {
@@ -421,7 +424,7 @@ export class QueryBuilder<T> {
     }
     if (this.#query.notEqual) {
       build(this.#query.notEqual, (key: string, value: any) => {
-        if (!value) {
+        if (isEmpty(value)) {
           return null
         }
         if (typeof value === "boolean") {

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

@@ -117,6 +117,19 @@ export async function validate(
   })
 }
 
+function fixBooleanFields({ row, table }: { row: Row; table: Table }) {
+  for (let col of Object.values(table.schema)) {
+    if (col.type === FieldType.BOOLEAN) {
+      if (row[col.name] === 1) {
+        row[col.name] = true
+      } else if (row[col.name] === 0) {
+        row[col.name] = false
+      }
+    }
+  }
+  return row
+}
+
 export async function sqlOutputProcessing(
   rows: DatasourcePlusQueryResponse,
   table: Table,
@@ -161,7 +174,9 @@ export async function sqlOutputProcessing(
     if (thisRow._id == null) {
       throw new Error("Unable to generate row ID for SQL rows")
     }
-    finalRows[thisRow._id] = thisRow
+
+    finalRows[thisRow._id] = fixBooleanFields({ row: thisRow, table })
+
     // do this at end once its been added to the final rows
     finalRows = await updateRelationshipColumns(
       table,

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

@@ -67,6 +67,22 @@ describe.each([
   class SearchAssertion {
     constructor(private readonly query: RowSearchParams) {}
 
+    private findRow(expectedRow: any, foundRows: any[]) {
+      const row = foundRows.find(foundRow => _.isMatch(foundRow, expectedRow))
+      if (!row) {
+        const fields = Object.keys(expectedRow)
+        // To make the error message more readable, we only include the fields
+        // that are present in the expected row.
+        const searchedObjects = foundRows.map(row => _.pick(row, fields))
+        throw new Error(
+          `Failed to find row: ${JSON.stringify(
+            expectedRow
+          )} in ${JSON.stringify(searchedObjects)}`
+        )
+      }
+      return row
+    }
+
     // Asserts that the query returns rows matching exactly the set of rows
     // passed in. The order of the rows matters. Rows returned in an order
     // different to the one passed in will cause the assertion to fail.  Extra
@@ -82,9 +98,7 @@ describe.each([
       // eslint-disable-next-line jest/no-standalone-expect
       expect(foundRows).toEqual(
         expectedRows.map((expectedRow: any) =>
-          expect.objectContaining(
+          expect.objectContaining(this.findRow(expectedRow, foundRows))
-            foundRows.find(foundRow => _.isMatch(foundRow, expectedRow))
-          )
         )
       )
     }
@@ -104,9 +118,7 @@ describe.each([
       expect(foundRows).toEqual(
         expect.arrayContaining(
           expectedRows.map((expectedRow: any) =>
-            expect.objectContaining(
+            expect.objectContaining(this.findRow(expectedRow, foundRows))
-              foundRows.find(foundRow => _.isMatch(foundRow, expectedRow))
-            )
           )
         )
       )
@@ -125,9 +137,7 @@ describe.each([
       expect(foundRows).toEqual(
         expect.arrayContaining(
           expectedRows.map((expectedRow: any) =>
-            expect.objectContaining(
+            expect.objectContaining(this.findRow(expectedRow, foundRows))
-              foundRows.find(foundRow => _.isMatch(foundRow, expectedRow))
-            )
           )
         )
       )
@@ -156,6 +166,67 @@ describe.each([
     return expectSearch({ query })
   }
 
+  describe("boolean", () => {
+    beforeAll(async () => {
+      await createTable({
+        isTrue: { name: "isTrue", type: FieldType.BOOLEAN },
+      })
+      await createRows([{ isTrue: true }, { isTrue: false }])
+    })
+
+    describe("equal", () => {
+      it("successfully finds true row", () =>
+        expectQuery({ equal: { isTrue: true } }).toMatchExactly([
+          { isTrue: true },
+        ]))
+
+      it("successfully finds false row", () =>
+        expectQuery({ equal: { isTrue: false } }).toMatchExactly([
+          { isTrue: false },
+        ]))
+    })
+
+    describe("notEqual", () => {
+      it("successfully finds false row", () =>
+        expectQuery({ notEqual: { isTrue: true } }).toContainExactly([
+          { isTrue: false },
+        ]))
+
+      it("successfully finds true row", () =>
+        expectQuery({ notEqual: { isTrue: false } }).toContainExactly([
+          { isTrue: true },
+        ]))
+    })
+
+    describe("oneOf", () => {
+      it("successfully finds true row", () =>
+        expectQuery({ oneOf: { isTrue: [true] } }).toContainExactly([
+          { isTrue: true },
+        ]))
+
+      it("successfully finds false row", () =>
+        expectQuery({ oneOf: { isTrue: [false] } }).toContainExactly([
+          { isTrue: false },
+        ]))
+    })
+
+    describe("sort", () => {
+      it("sorts ascending", () =>
+        expectSearch({
+          query: {},
+          sort: "isTrue",
+          sortOrder: SortOrder.ASCENDING,
+        }).toMatchExactly([{ isTrue: false }, { isTrue: true }]))
+
+      it("sorts descending", () =>
+        expectSearch({
+          query: {},
+          sort: "isTrue",
+          sortOrder: SortOrder.DESCENDING,
+        }).toMatchExactly([{ isTrue: true }, { isTrue: false }]))
+    })
+  })
+
   describe.each([FieldType.STRING, FieldType.LONGFORM])("%s", () => {
     beforeAll(async () => {
       await createTable({

packages/server/src/automations/tests/queryRows.spec.ts

@@ -1,14 +1,4 @@
-// lucene searching not supported in test due to use of PouchDB
+import { Table } from "@budibase/types"
-let rows: Row[] = []
-jest.mock("../../sdk/app/rows/search/internalSearch", () => ({
-  fullSearch: jest.fn(() => {
-    return {
-      rows,
-    }
-  }),
-  paginatedSearch: jest.fn(),
-}))
-import { Row, Table } from "@budibase/types"
 import * as setup from "./utilities"
 
 const NAME = "Test"
@@ -25,8 +15,8 @@ describe("Test a query step automation", () => {
       description: "original description",
       tableId: table._id,
     }
-    rows.push(await config.createRow(row))
+    await config.createRow(row)
-    rows.push(await config.createRow(row))
+    await config.createRow(row)
   })
 
   afterAll(setup.afterAll)

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

@@ -150,6 +150,22 @@ function getTableName(table?: Table): string | undefined {
   }
 }
 
+function convertBooleans(query: SqlQuery | SqlQuery[]): SqlQuery | SqlQuery[] {
+  if (Array.isArray(query)) {
+    return query.map((q: SqlQuery) => convertBooleans(q) as SqlQuery)
+  } else {
+    if (query.bindings) {
+      query.bindings = query.bindings.map(binding => {
+        if (typeof binding === "boolean") {
+          return binding ? 1 : 0
+        }
+        return binding
+      })
+    }
+  }
+  return query
+}
+
 class InternalBuilder {
   private readonly client: string
 
@@ -654,7 +670,11 @@ class SqlQueryBuilder extends SqlTableQueryBuilder {
     if (opts?.disableBindings) {
       return { sql: query.toString() }
     } else {
-      return getNativeSql(query)
+      let native = getNativeSql(query)
+      if (sqlClient === SqlClient.SQL_LITE) {
+        native = convertBooleans(native)
+      }
+      return native
     }
   }
 

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

@@ -164,8 +164,8 @@ export async function search(
         throw new Error("SQS cannot currently handle multiple queries")
       }
 
-      let sql = query.sql,
+      let sql = query.sql
-        bindings = query.bindings
+      let bindings = query.bindings
 
       // quick hack for docIds
       sql = sql.replace(/`doc1`.`rowId`/g, "`doc1.rowId`")