Merge pull request #14128 from Budibase/budi-8445-is-in-filter-broken

Fix bug in oneOf search.

.github/workflows/budibase_ci.yml

@@ -108,7 +108,7 @@ jobs:
       - name: Pull testcontainers images
         run: |
           docker pull testcontainers/ryuk:0.5.1 &
-          docker pull budibase/couchdb:v3.2.1-sql &
+          docker pull budibase/couchdb:v3.2.1-sqs &
           docker pull redis &
 
           wait $(jobs -p)

packages/backend-core/src/users/users.ts

@@ -18,9 +18,10 @@ import {
   CouchFindOptions,
   DatabaseQueryOpts,
   SearchFilters,
-  SearchFilterOperator,
   SearchUsersRequest,
   User,
+  BasicOperator,
+  ArrayOperator,
 } from "@budibase/types"
 import * as context from "../context"
 import { getGlobalDB } from "../context"
@@ -46,9 +47,9 @@ function removeUserPassword(users: User | User[]) {
 
 export function isSupportedUserSearch(query: SearchFilters) {
   const allowed = [
-    { op: SearchFilterOperator.STRING, key: "email" },
-    { op: SearchFilterOperator.EQUAL, key: "_id" },
-    { op: SearchFilterOperator.ONE_OF, key: "_id" },
+    { op: BasicOperator.STRING, key: "email" },
+    { op: BasicOperator.EQUAL, key: "_id" },
+    { op: ArrayOperator.ONE_OF, key: "_id" },
   ]
   for (let [key, operation] of Object.entries(query)) {
     if (typeof operation !== "object") {

packages/frontend-core/src/components/FilterBuilder.svelte

@@ -11,7 +11,7 @@
     Label,
     Multiselect,
   } from "@budibase/bbui"
-  import { FieldType, SearchFilterOperator } from "@budibase/types"
+  import { ArrayOperator, FieldType } from "@budibase/types"
   import { generate } from "shortid"
   import { QueryUtils, Constants } from "@budibase/frontend-core"
   import { getContext } from "svelte"
@@ -268,7 +268,7 @@
                 <slot name="binding" {filter} />
               {:else if [FieldType.STRING, FieldType.LONGFORM, FieldType.NUMBER, FieldType.BIGINT, FieldType.FORMULA].includes(filter.type)}
                 <Input disabled={filter.noValue} bind:value={filter.value} />
-              {:else if filter.type === FieldType.ARRAY || (filter.type === FieldType.OPTIONS && filter.operator === SearchFilterOperator.ONE_OF)}
+              {:else if filter.type === FieldType.ARRAY || (filter.type === FieldType.OPTIONS && filter.operator === ArrayOperator.ONE_OF)}
                 <Multiselect
                   disabled={filter.noValue}
                   options={getFieldOptions(filter.field)}

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

@@ -780,6 +780,32 @@ describe.each([
       it("fails to find nonexistent row", async () => {
         await expectQuery({ oneOf: { name: ["none"] } }).toFindNothing()
       })
+
+      it("can have multiple values for same column", async () => {
+        await expectQuery({
+          oneOf: {
+            name: ["foo", "bar"],
+          },
+        }).toContainExactly([{ name: "foo" }, { name: "bar" }])
+      })
+
+      it("splits comma separated strings", async () => {
+        await expectQuery({
+          oneOf: {
+            // @ts-ignore
+            name: "foo,bar",
+          },
+        }).toContainExactly([{ name: "foo" }, { name: "bar" }])
+      })
+
+      it("trims whitespace", async () => {
+        await expectQuery({
+          oneOf: {
+            // @ts-ignore
+            name: "foo, bar",
+          },
+        }).toContainExactly([{ name: "foo" }, { name: "bar" }])
+      })
     })
 
     describe("fuzzy", () => {
@@ -1002,6 +1028,32 @@ describe.each([
       it("fails to find nonexistent row", async () => {
         await expectQuery({ oneOf: { age: [2] } }).toFindNothing()
       })
+
+      // I couldn't find a way to make this work in Lucene and given that
+      // we're getting rid of Lucene soon I wasn't inclined to spend time on
+      // it.
+      !isLucene &&
+        it("can convert from a string", async () => {
+          await expectQuery({
+            oneOf: {
+              // @ts-ignore
+              age: "1",
+            },
+          }).toContainExactly([{ age: 1 }])
+        })
+
+      // I couldn't find a way to make this work in Lucene and given that
+      // we're getting rid of Lucene soon I wasn't inclined to spend time on
+      // it.
+      !isLucene &&
+        it("can find multiple values for same column", async () => {
+          await expectQuery({
+            oneOf: {
+              // @ts-ignore
+              age: "1,10",
+            },
+          }).toContainExactly([{ age: 1 }, { age: 10 }])
+        })
     })
 
     describe("range", () => {

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

@@ -9,7 +9,6 @@ import {
   QuotaUsageType,
   Row,
   SaveTableRequest,
-  SearchFilterOperator,
   SortOrder,
   SortType,
   StaticQuotaName,
@@ -19,6 +18,7 @@ import {
   ViewUIFieldMetadata,
   ViewV2,
   SearchResponse,
+  BasicOperator,
 } from "@budibase/types"
 import { generator, mocks } from "@budibase/backend-core/tests"
 import { DatabaseName, getDatasource } from "../../../integrations/tests/utils"
@@ -149,7 +149,7 @@ describe.each([
         primaryDisplay: "id",
         query: [
           {
-            operator: SearchFilterOperator.EQUAL,
+            operator: BasicOperator.EQUAL,
             field: "field",
             value: "value",
           },
@@ -561,7 +561,7 @@ describe.each([
         ...view,
         query: [
           {
-            operator: SearchFilterOperator.EQUAL,
+            operator: BasicOperator.EQUAL,
             field: "newField",
             value: "thatValue",
           },
@@ -589,7 +589,7 @@ describe.each([
         primaryDisplay: "Price",
         query: [
           {
-            operator: SearchFilterOperator.EQUAL,
+            operator: BasicOperator.EQUAL,
             field: generator.word(),
             value: generator.word(),
           },
@@ -673,7 +673,7 @@ describe.each([
           tableId: generator.guid(),
           query: [
             {
-              operator: SearchFilterOperator.EQUAL,
+              operator: BasicOperator.EQUAL,
               field: "newField",
               value: "thatValue",
             },
@@ -1194,7 +1194,7 @@ describe.each([
           name: generator.guid(),
           query: [
             {
-              operator: SearchFilterOperator.EQUAL,
+              operator: BasicOperator.EQUAL,
               field: "two",
               value: "bar2",
             },

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

@@ -2,7 +2,6 @@ import {
   EmptyFilterOption,
   Row,
   RowSearchParams,
-  SearchFilterOperator,
   SearchFilters,
   SearchResponse,
   SortOrder,
@@ -66,37 +65,12 @@ export function removeEmptyFilters(filters: SearchFilters) {
   return filters
 }
 
-// The frontend can send single values for array fields sometimes, so to handle
-// this we convert them to arrays at the controller level so that nothing below
-// this has to worry about the non-array values.
-function fixupFilterArrays(filters: SearchFilters) {
-  const arrayFields = [
-    SearchFilterOperator.ONE_OF,
-    SearchFilterOperator.CONTAINS,
-    SearchFilterOperator.NOT_CONTAINS,
-    SearchFilterOperator.CONTAINS_ANY,
-  ]
-  for (const searchField of arrayFields) {
-    const field = filters[searchField]
-    if (field == null) {
-      continue
-    }
-
-    for (const key of Object.keys(field)) {
-      if (!Array.isArray(field[key])) {
-        field[key] = [field[key]]
-      }
-    }
-  }
-  return filters
-}
-
 export async function search(
   options: RowSearchParams
 ): Promise<SearchResponse<Row>> {
   const isExternalTable = isExternalTableID(options.tableId)
   options.query = removeEmptyFilters(options.query || {})
-  options.query = fixupFilterArrays(options.query)
+  options.query = dataFilters.fixupFilterArrays(options.query)
   if (
     !dataFilters.hasFilters(options.query) &&
     options.query.onEmptyFilter === EmptyFilterOption.RETURN_NONE

packages/shared-core/src/filters.ts

@@ -6,6 +6,7 @@ import {
   SearchFilter,
   SearchFilters,
   SearchQueryFields,
+  ArrayOperator,
   SearchFilterOperator,
   SortType,
   FieldConstraints,
@@ -14,11 +15,13 @@ import {
   EmptyFilterOption,
   SearchResponse,
   Table,
+  BasicOperator,
+  RangeOperator,
 } from "@budibase/types"
 import dayjs from "dayjs"
 import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants"
 import { deepGet, schema } from "./helpers"
-import _ from "lodash"
+import { isPlainObject, isEmpty } from "lodash"
 
 const HBS_REGEX = /{{([^{].*?)}}/g
 
@@ -323,6 +326,32 @@ export const buildQuery = (filter: SearchFilter[]) => {
   return query
 }
 
+// The frontend can send single values for array fields sometimes, so to handle
+// this we convert them to arrays at the controller level so that nothing below
+// this has to worry about the non-array values.
+export function fixupFilterArrays(filters: SearchFilters) {
+  for (const searchField of Object.values(ArrayOperator)) {
+    const field = filters[searchField]
+    if (field == null || !isPlainObject(field)) {
+      continue
+    }
+
+    for (const key of Object.keys(field)) {
+      if (Array.isArray(field[key])) {
+        continue
+      }
+
+      const value = field[key] as any
+      if (typeof value === "string") {
+        field[key] = value.split(",").map((x: string) => x.trim())
+      } else {
+        field[key] = [value]
+      }
+    }
+  }
+  return filters
+}
+
 export const search = (
   docs: Record<string, any>[],
   query: RowSearchParams
@@ -356,6 +385,7 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
   }
 
   query = cleanupQuery(query)
+  query = fixupFilterArrays(query)
 
   if (
     !hasFilters(query) &&
@@ -382,7 +412,7 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
     }
 
   const stringMatch = match(
-    SearchFilterOperator.STRING,
+    BasicOperator.STRING,
     (docValue: any, testValue: any) => {
       if (!(typeof docValue === "string")) {
         return false
@@ -395,7 +425,7 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
   )
 
   const fuzzyMatch = match(
-    SearchFilterOperator.FUZZY,
+    BasicOperator.FUZZY,
     (docValue: any, testValue: any) => {
       if (!(typeof docValue === "string")) {
         return false
@@ -408,17 +438,17 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
   )
 
   const rangeMatch = match(
-    SearchFilterOperator.RANGE,
+    RangeOperator.RANGE,
     (docValue: any, testValue: any) => {
       if (docValue == null || docValue === "") {
         return false
       }
 
-      if (_.isObject(testValue.low) && _.isEmpty(testValue.low)) {
+      if (isPlainObject(testValue.low) && isEmpty(testValue.low)) {
         testValue.low = undefined
       }
 
-      if (_.isObject(testValue.high) && _.isEmpty(testValue.high)) {
+      if (isPlainObject(testValue.high) && isEmpty(testValue.high)) {
         testValue.high = undefined
       }
 
@@ -497,11 +527,8 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
     (...args: T): boolean =>
       !f(...args)
 
-  const equalMatch = match(SearchFilterOperator.EQUAL, _valueMatches)
-  const notEqualMatch = match(
-    SearchFilterOperator.NOT_EQUAL,
-    not(_valueMatches)
-  )
+  const equalMatch = match(BasicOperator.EQUAL, _valueMatches)
+  const notEqualMatch = match(BasicOperator.NOT_EQUAL, not(_valueMatches))
 
   const _empty = (docValue: any) => {
     if (typeof docValue === "string") {
@@ -516,26 +543,24 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
     return docValue == null
   }
 
-  const emptyMatch = match(SearchFilterOperator.EMPTY, _empty)
-  const notEmptyMatch = match(SearchFilterOperator.NOT_EMPTY, not(_empty))
+  const emptyMatch = match(BasicOperator.EMPTY, _empty)
+  const notEmptyMatch = match(BasicOperator.NOT_EMPTY, not(_empty))
 
-  const oneOf = match(
-    SearchFilterOperator.ONE_OF,
-    (docValue: any, testValue: any) => {
-      if (typeof testValue === "string") {
-        testValue = testValue.split(",")
-        if (typeof docValue === "number") {
-          testValue = testValue.map((item: string) => parseFloat(item))
-        }
-      }
-
-      if (!Array.isArray(testValue)) {
-        return false
-      }
-
-      return testValue.some(item => _valueMatches(docValue, item))
+  const oneOf = match(ArrayOperator.ONE_OF, (docValue: any, testValue: any) => {
+    if (typeof testValue === "string") {
+      testValue = testValue.split(",")
     }
-  )
+
+    if (typeof docValue === "number") {
+      testValue = testValue.map((item: string) => parseFloat(item))
+    }
+
+    if (!Array.isArray(testValue)) {
+      return false
+    }
+
+    return testValue.some(item => _valueMatches(docValue, item))
+  })
 
   const _contains =
     (f: "some" | "every") => (docValue: any, testValue: any) => {
@@ -562,7 +587,7 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
     }
 
   const contains = match(
-    SearchFilterOperator.CONTAINS,
+    ArrayOperator.CONTAINS,
     (docValue: any, testValue: any) => {
       if (Array.isArray(testValue) && testValue.length === 0) {
         return true
@@ -571,7 +596,7 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
     }
   )
   const notContains = match(
-    SearchFilterOperator.NOT_CONTAINS,
+    ArrayOperator.NOT_CONTAINS,
     (docValue: any, testValue: any) => {
       // Not sure if this is logically correct, but at the time this code was
       // written the search endpoint behaved this way and we wanted to make this
@@ -582,10 +607,7 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
       return not(_contains("every"))(docValue, testValue)
     }
   )
-  const containsAny = match(
-    SearchFilterOperator.CONTAINS_ANY,
-    _contains("some")
-  )
+  const containsAny = match(ArrayOperator.CONTAINS_ANY, _contains("some"))
 
   const docMatch = (doc: Record<string, any>) => {
     const filterFunctions = {

packages/types/src/sdk/search.ts

@@ -3,20 +3,28 @@ import { Row, Table, DocumentType } from "../documents"
 import { SortOrder, SortType } from "../api"
 import { Knex } from "knex"
 
-export enum SearchFilterOperator {
-  STRING = "string",
-  FUZZY = "fuzzy",
-  RANGE = "range",
+export enum BasicOperator {
   EQUAL = "equal",
   NOT_EQUAL = "notEqual",
   EMPTY = "empty",
   NOT_EMPTY = "notEmpty",
-  ONE_OF = "oneOf",
+  FUZZY = "fuzzy",
+  STRING = "string",
+}
+
+export enum ArrayOperator {
   CONTAINS = "contains",
   NOT_CONTAINS = "notContains",
   CONTAINS_ANY = "containsAny",
+  ONE_OF = "oneOf",
 }
 
+export enum RangeOperator {
+  RANGE = "range",
+}
+
+export type SearchFilterOperator = BasicOperator | ArrayOperator | RangeOperator
+
 export enum InternalSearchFilterOperator {
   COMPLEX_ID_OPERATOR = "_complexIdOperator",
 }
@@ -52,17 +60,17 @@ export interface SearchFilters {
   // allows just fuzzy to be or - all the fuzzy/like parameters
   fuzzyOr?: boolean
   onEmptyFilter?: EmptyFilterOption
-  [SearchFilterOperator.STRING]?: BasicFilter<string>
-  [SearchFilterOperator.FUZZY]?: BasicFilter<string>
-  [SearchFilterOperator.RANGE]?: RangeFilter
-  [SearchFilterOperator.EQUAL]?: BasicFilter
-  [SearchFilterOperator.NOT_EQUAL]?: BasicFilter
-  [SearchFilterOperator.EMPTY]?: BasicFilter
-  [SearchFilterOperator.NOT_EMPTY]?: BasicFilter
-  [SearchFilterOperator.ONE_OF]?: ArrayFilter
-  [SearchFilterOperator.CONTAINS]?: ArrayFilter
-  [SearchFilterOperator.NOT_CONTAINS]?: ArrayFilter
-  [SearchFilterOperator.CONTAINS_ANY]?: ArrayFilter
+  [BasicOperator.STRING]?: BasicFilter<string>
+  [BasicOperator.FUZZY]?: BasicFilter<string>
+  [RangeOperator.RANGE]?: RangeFilter
+  [BasicOperator.EQUAL]?: BasicFilter
+  [BasicOperator.NOT_EQUAL]?: BasicFilter
+  [BasicOperator.EMPTY]?: BasicFilter
+  [BasicOperator.NOT_EMPTY]?: BasicFilter
+  [ArrayOperator.ONE_OF]?: ArrayFilter
+  [ArrayOperator.CONTAINS]?: ArrayFilter
+  [ArrayOperator.NOT_CONTAINS]?: ArrayFilter
+  [ArrayOperator.CONTAINS_ANY]?: ArrayFilter
   // specific to SQS/SQLite search on internal tables this can be used
   // to make sure the documents returned are always filtered down to a
   // specific document type (such as just rows)