Respond to PR feedback.

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 { isPlainObject, isEmpty, isArray } from "lodash"
+import { isPlainObject, isEmpty } from "lodash"
 
 const HBS_REGEX = /{{([^{].*?)}}/g
 
@@ -327,13 +330,7 @@ export const buildQuery = (filter: SearchFilter[]) => {
 // 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) {
-  const arrayFields = [
+  for (const searchField of Object.values(ArrayOperator)) {
-    SearchFilterOperator.ONE_OF,
-    SearchFilterOperator.CONTAINS,
-    SearchFilterOperator.NOT_CONTAINS,
-    SearchFilterOperator.CONTAINS_ANY,
-  ]
-  for (const searchField of arrayFields) {
     const field = filters[searchField]
     if (field == null || !isPlainObject(field)) {
       continue
@@ -341,10 +338,12 @@ export function fixupFilterArrays(filters: SearchFilters) {
 
     for (const key of Object.keys(field)) {
       if (!Array.isArray(field[key])) {
-        if (typeof field[key] !== "string") {
+        if (typeof field[key] === "string") {
-          field[key] = [field[key]]
+          field[key] = (field[key] as string)
+            .split(",")
+            .map((x: string) => x.trim())
         } else {
-          field[key] = field[key].split(",").map((x: string) => x.trim())
+          field[key] = [field[key]]
         }
       }
     }
@@ -412,7 +411,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
@@ -425,7 +424,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
@@ -438,7 +437,7 @@ 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
@@ -527,11 +526,8 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
     (...args: T): boolean =>
       !f(...args)
 
-  const equalMatch = match(SearchFilterOperator.EQUAL, _valueMatches)
+  const equalMatch = match(BasicOperator.EQUAL, _valueMatches)
-  const notEqualMatch = match(
+  const notEqualMatch = match(BasicOperator.NOT_EQUAL, not(_valueMatches))
-    SearchFilterOperator.NOT_EQUAL,
-    not(_valueMatches)
-  )
 
   const _empty = (docValue: any) => {
     if (typeof docValue === "string") {
@@ -546,27 +542,24 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
     return docValue == null
   }
 
-  const emptyMatch = match(SearchFilterOperator.EMPTY, _empty)
+  const emptyMatch = match(BasicOperator.EMPTY, _empty)
-  const notEmptyMatch = match(SearchFilterOperator.NOT_EMPTY, not(_empty))
+  const notEmptyMatch = match(BasicOperator.NOT_EMPTY, not(_empty))
 
-  const oneOf = match(
+  const oneOf = match(ArrayOperator.ONE_OF, (docValue: any, testValue: any) => {
-    SearchFilterOperator.ONE_OF,
+    if (typeof testValue === "string") {
-    (docValue: any, testValue: any) => {
+      testValue = testValue.split(",")
-      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))
     }
-  )
+
+    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) => {
@@ -593,7 +586,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
@@ -602,7 +595,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
@@ -613,10 +606,7 @@ export const runQuery = (docs: Record<string, any>[], query: SearchFilters) => {
       return not(_contains("every"))(docValue, testValue)
     }
   )
-  const containsAny = match(
+  const containsAny = match(ArrayOperator.CONTAINS_ANY, _contains("some"))
-    SearchFilterOperator.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 {
+export enum BasicOperator {
-  STRING = "string",
-  FUZZY = "fuzzy",
-  RANGE = "range",
   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>
+  [BasicOperator.STRING]?: BasicFilter<string>
-  [SearchFilterOperator.FUZZY]?: BasicFilter<string>
+  [BasicOperator.FUZZY]?: BasicFilter<string>
-  [SearchFilterOperator.RANGE]?: RangeFilter
+  [RangeOperator.RANGE]?: RangeFilter
-  [SearchFilterOperator.EQUAL]?: BasicFilter
+  [BasicOperator.EQUAL]?: BasicFilter
-  [SearchFilterOperator.NOT_EQUAL]?: BasicFilter
+  [BasicOperator.NOT_EQUAL]?: BasicFilter
-  [SearchFilterOperator.EMPTY]?: BasicFilter
+  [BasicOperator.EMPTY]?: BasicFilter
-  [SearchFilterOperator.NOT_EMPTY]?: BasicFilter
+  [BasicOperator.NOT_EMPTY]?: BasicFilter
-  [SearchFilterOperator.ONE_OF]?: ArrayFilter
+  [ArrayOperator.ONE_OF]?: ArrayFilter
-  [SearchFilterOperator.CONTAINS]?: ArrayFilter
+  [ArrayOperator.CONTAINS]?: ArrayFilter
-  [SearchFilterOperator.NOT_CONTAINS]?: ArrayFilter
+  [ArrayOperator.NOT_CONTAINS]?: ArrayFilter
-  [SearchFilterOperator.CONTAINS_ANY]?: 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)