Merge pull request #14818 from Budibase/queryui-default

Make `queryUI` on `ViewV2` something you can write on its own, and `query` gets filled in for you

packages/builder/src/components/backend/DataTable/modals/ExportModal.test.js

@@ -79,6 +79,8 @@ describe("Export Modal", () => {
       props: propsCfg,
     })
 
+    expect(propsCfg.filters[0].field).toBe("1:Cost")
+
     expect(screen.getByTestId("filters-applied")).toBeVisible()
     expect(screen.getByTestId("filters-applied").textContent).toBe(
       "Filters applied"

packages/builder/src/stores/builder/viewsV2.js

@@ -4,7 +4,7 @@ import { API } from "api"
 import { dataFilters } from "@budibase/shared-core"
 
 function convertToSearchFilters(view) {
-  // convert from SearchFilterGroup type
+  // convert from UISearchFilter type
   if (view?.query) {
     return {
       ...view,
@@ -15,7 +15,7 @@ function convertToSearchFilters(view) {
   return view
 }
 
-function convertToSearchFilterGroup(view) {
+function convertToUISearchFilter(view) {
   if (view?.queryUI) {
     return {
       ...view,
@@ -36,7 +36,7 @@ export function createViewsV2Store() {
       const views = Object.values(table?.views || {}).filter(view => {
         return view.version === 2
       })
-      list = list.concat(views.map(view => convertToSearchFilterGroup(view)))
+      list = list.concat(views.map(view => convertToUISearchFilter(view)))
     })
     return {
       ...$store,
@@ -77,7 +77,7 @@ export function createViewsV2Store() {
     if (!viewId) {
       return
     }
-    view = convertToSearchFilterGroup(view)
+    view = convertToUISearchFilter(view)
     const existingView = get(derivedStore).list.find(view => view.id === viewId)
     const tableIndex = get(tables).list.findIndex(table => {
       return table._id === view?.tableId || table._id === existingView?.tableId

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

@@ -10,7 +10,7 @@
   } from "@budibase/bbui"
   import {
     FieldType,
-    FilterGroupLogicalOperator,
+    UILogicalOperator,
     EmptyFilterOption,
   } from "@budibase/types"
   import { QueryUtils, Constants } from "@budibase/frontend-core"
@@ -220,7 +220,7 @@
     } else if (addGroup) {
       if (!editable?.groups?.length) {
         editable = {
-          logicalOperator: FilterGroupLogicalOperator.ALL,
+          logicalOperator: UILogicalOperator.ALL,
           onEmptyFilter: EmptyFilterOption.RETURN_NONE,
           groups: [],
         }

packages/frontend-core/src/components/grid/stores/filter.js

@@ -1,5 +1,5 @@
 import { get, derived } from "svelte/store"
-import { FieldType, FilterGroupLogicalOperator } from "@budibase/types"
+import { FieldType, UILogicalOperator } from "@budibase/types"
 import { memo } from "../../../utils/memo"
 
 export const createStores = context => {
@@ -25,10 +25,10 @@ export const deriveStores = context => {
         return $filter
       }
       let allFilters = {
-        logicalOperator: FilterGroupLogicalOperator.ALL,
+        logicalOperator: UILogicalOperator.ALL,
         groups: [
           {
-            logicalOperator: FilterGroupLogicalOperator.ALL,
+            logicalOperator: UILogicalOperator.ALL,
             filters: $inlineFilters,
           },
         ],

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

@@ -1688,7 +1688,7 @@ describe.each([
         })
       })
 
-    describe.each([FieldType.ARRAY, FieldType.OPTIONS])("%s", () => {
+    describe("arrays", () => {
       beforeAll(async () => {
         tableOrViewId = await createTableOrView({
           numbers: {

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

@@ -6,6 +6,7 @@ import {
   Row,
   RowSearchParams,
   SearchFilterKey,
+  SearchFilters,
   SearchResponse,
   SortOrder,
   Table,
@@ -90,17 +91,18 @@ export async function search(
     options = searchInputMapping(table, options)
 
     if (options.viewId) {
-      // Delete extraneous search params that cannot be overridden
-      delete options.query.onEmptyFilter
-
       const view = source as ViewV2
+
       // Enrich saved query with ephemeral query params.
       // We prevent searching on any fields that are saved as part of the query, as
       // that could let users find rows they should not be allowed to access.
-      let viewQuery = await enrichSearchContext(view.query || {}, context)
-      viewQuery = dataFilters.buildQueryLegacy(viewQuery) || {}
+      let viewQuery = (await enrichSearchContext(view.query || {}, context)) as
+        | SearchFilters
+        | LegacyFilter[]
+      if (Array.isArray(viewQuery)) {
+        viewQuery = dataFilters.buildQuery(viewQuery)
+      }
       viewQuery = checkFilters(table, viewQuery)
-      delete viewQuery?.onEmptyFilter
 
       const sqsEnabled = await features.flags.isEnabled(FeatureFlag.SQS)
       const supportsLogicalOperators =
@@ -113,13 +115,12 @@ export async function search(
           ? view.query
           : []
 
-        delete options.query.onEmptyFilter
+        const { filters } = dataFilters.splitFiltersArray(queryFilters)
 
         // Extract existing fields
-        const existingFields =
-          queryFilters
-            ?.filter(filter => filter.field)
-            .map(filter => db.removeKeyNumbering(filter.field)) || []
+        const existingFields = filters.map(filter =>
+          db.removeKeyNumbering(filter.field)
+        )
 
         // Carry over filters for unused fields
         Object.keys(options.query).forEach(key => {

packages/server/src/sdk/app/tables/getters.ts

@@ -16,6 +16,8 @@ import {
 } from "@budibase/types"
 import datasources from "../datasources"
 import sdk from "../../../sdk"
+import { ensureQueryUISet } from "../views/utils"
+import { isV2 } from "../views"
 
 export async function processTable(table: Table): Promise<Table> {
   if (!table) {
@@ -23,6 +25,14 @@ export async function processTable(table: Table): Promise<Table> {
   }
 
   table = { ...table }
+  if (table.views) {
+    for (const [key, view] of Object.entries(table.views)) {
+      if (!isV2(view)) {
+        continue
+      }
+      table.views[key] = ensureQueryUISet(view)
+    }
+  }
   if (table._id && isExternalTableID(table._id)) {
     // Old created external tables via Budibase might have a missing field name breaking some UI such as filters
     if (table.schema["id"] && !table.schema["id"].name) {

packages/server/src/sdk/app/views/external.ts

@@ -5,6 +5,7 @@ import sdk from "../../../sdk"
 import * as utils from "../../../db/utils"
 import { enrichSchema, isV2 } from "."
 import { breakExternalTableId } from "../../../integrations/utils"
+import { ensureQuerySet, ensureQueryUISet } from "./utils"
 
 export async function get(viewId: string): Promise<ViewV2> {
   const { tableId } = utils.extractViewInfoFromID(viewId)
@@ -18,7 +19,7 @@ export async function get(viewId: string): Promise<ViewV2> {
   if (!found) {
     throw new Error("No view found")
   }
-  return found
+  return ensureQueryUISet(found)
 }
 
 export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
@@ -33,19 +34,22 @@ export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
   if (!found) {
     throw new Error("No view found")
   }
-  return await enrichSchema(found, table.schema)
+  return await enrichSchema(ensureQueryUISet(found), table.schema)
 }
 
 export async function create(
   tableId: string,
   viewRequest: Omit<ViewV2, "id" | "version">
 ): Promise<ViewV2> {
-  const view: ViewV2 = {
+  let view: ViewV2 = {
     ...viewRequest,
     id: utils.generateViewID(tableId),
     version: 2,
   }
 
+  view = ensureQuerySet(view)
+  view = ensureQueryUISet(view)
+
   const db = context.getAppDB()
 
   const { datasourceId, tableName } = breakExternalTableId(tableId)
@@ -56,7 +60,10 @@ export async function create(
   return view
 }
 
-export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
+export async function update(
+  tableId: string,
+  view: Readonly<ViewV2>
+): Promise<ViewV2> {
   const db = context.getAppDB()
 
   const { datasourceId, tableName } = breakExternalTableId(tableId)
@@ -74,6 +81,9 @@ export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
     throw new HTTPError(`Cannot update view type after creation`, 400)
   }
 
+  view = ensureQuerySet(view)
+  view = ensureQueryUISet(view)
+
   delete views[existingView.name]
   views[view.name] = view
   await db.put(ds)

packages/server/src/sdk/app/views/internal.ts

@@ -4,6 +4,7 @@ import { context, HTTPError } from "@budibase/backend-core"
 import sdk from "../../../sdk"
 import * as utils from "../../../db/utils"
 import { enrichSchema, isV2 } from "."
+import { ensureQuerySet, ensureQueryUISet } from "./utils"
 
 export async function get(viewId: string): Promise<ViewV2> {
   const { tableId } = utils.extractViewInfoFromID(viewId)
@@ -13,7 +14,7 @@ export async function get(viewId: string): Promise<ViewV2> {
   if (!found) {
     throw new Error("No view found")
   }
-  return found
+  return ensureQueryUISet(found)
 }
 
 export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
@@ -24,19 +25,22 @@ export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
   if (!found) {
     throw new Error("No view found")
   }
-  return await enrichSchema(found, table.schema)
+  return await enrichSchema(ensureQueryUISet(found), table.schema)
 }
 
 export async function create(
   tableId: string,
   viewRequest: Omit<ViewV2, "id" | "version">
 ): Promise<ViewV2> {
-  const view: ViewV2 = {
+  let view: ViewV2 = {
     ...viewRequest,
     id: utils.generateViewID(tableId),
     version: 2,
   }
 
+  view = ensureQuerySet(view)
+  view = ensureQueryUISet(view)
+
   const db = context.getAppDB()
 
   const table = await sdk.tables.getTable(tableId)
@@ -47,7 +51,10 @@ export async function create(
   return view
 }
 
-export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
+export async function update(
+  tableId: string,
+  view: Readonly<ViewV2>
+): Promise<ViewV2> {
   const db = context.getAppDB()
   const table = await sdk.tables.getTable(tableId)
   table.views ??= {}
@@ -63,6 +70,9 @@ export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
     throw new HTTPError(`Cannot update view type after creation`, 400)
   }
 
+  view = ensureQuerySet(view)
+  view = ensureQueryUISet(view)
+
   delete table.views[existingView.name]
   table.views[view.name] = view
   await db.put(table)

packages/server/src/sdk/app/views/utils.ts

@@ -0,0 +1,43 @@
+import { ViewV2 } from "@budibase/types"
+import { utils, dataFilters } from "@budibase/shared-core"
+import { cloneDeep, isPlainObject } from "lodash"
+import { HTTPError } from "@budibase/backend-core"
+
+function isEmptyObject(obj: any) {
+  return obj && isPlainObject(obj) && Object.keys(obj).length === 0
+}
+
+export function ensureQueryUISet(viewArg: Readonly<ViewV2>): ViewV2 {
+  const view = cloneDeep<ViewV2>(viewArg)
+  if (!view.queryUI && view.query && !isEmptyObject(view.query)) {
+    if (!Array.isArray(view.query)) {
+      // In practice this should not happen. `view.query`, at the time this code
+      // goes into the codebase, only contains LegacyFilter[] in production.
+      // We're changing it in the change that this comment is part of to also
+      // include SearchFilters objects. These are created when we receive an
+      // update to a ViewV2 that contains a queryUI and not a query field. We
+      // can convert UISearchFilter (the type of queryUI) to SearchFilters,
+      // but not LegacyFilter[], they are incompatible due to UISearchFilter
+      // and SearchFilters being recursive types.
+      //
+      // So despite the type saying that `view.query` is a LegacyFilter[] |
+      // SearchFilters, it will never be a SearchFilters when a `view.queryUI`
+      // is specified, making it "safe" to throw an error here.
+      throw new HTTPError("view is missing queryUI field", 400)
+    }
+
+    view.queryUI = utils.processSearchFilters(view.query)
+  }
+  return view
+}
+
+export function ensureQuerySet(viewArg: Readonly<ViewV2>): ViewV2 {
+  const view = cloneDeep<ViewV2>(viewArg)
+  // We consider queryUI to be the source of truth, so we don't check for the
+  // presence of query here. We will overwrite it regardless of whether it is
+  // present or not.
+  if (view.queryUI && !isEmptyObject(view.queryUI)) {
+    view.query = dataFilters.buildQuery(view.queryUI)
+  }
+  return view
+}

packages/shared-core/src/filters.ts

@@ -19,8 +19,12 @@ import {
   RangeOperator,
   LogicalOperator,
   isLogicalSearchOperator,
-  SearchFilterGroup,
-  FilterGroupLogicalOperator,
+  UISearchFilter,
+  UILogicalOperator,
+  isBasicSearchOperator,
+  isArraySearchOperator,
+  isRangeSearchOperator,
+  SearchFilter,
 } from "@budibase/types"
 import dayjs from "dayjs"
 import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants"
@@ -310,308 +314,195 @@ export class ColumnSplitter {
  * @param filter the builder filter structure
  */
 
-const buildCondition = (expression: LegacyFilter) => {
-  // Filter body
-  let query: SearchFilters = {
-    string: {},
-    fuzzy: {},
-    range: {},
-    equal: {},
-    notEqual: {},
-    empty: {},
-    notEmpty: {},
-    contains: {},
-    notContains: {},
-    oneOf: {},
-    containsAny: {},
-  }
-  let { operator, field, type, value, externalType, onEmptyFilter } = expression
-
-  if (!operator || !field) {
+function buildCondition(filter: undefined): undefined
+function buildCondition(filter: SearchFilter): SearchFilters
+function buildCondition(filter?: SearchFilter): SearchFilters | undefined {
+  if (!filter) {
     return
   }
 
-  const queryOperator = operator as SearchFilterOperator
-  const isHbs =
-    typeof value === "string" && (value.match(HBS_REGEX) || []).length > 0
-  // Parse all values into correct types
-  if (operator === "allOr") {
-    query.allOr = true
-    return
-  }
-  if (onEmptyFilter) {
-    query.onEmptyFilter = onEmptyFilter
-    return
-  }
+  const query: SearchFilters = {}
+  const { operator, field, type, externalType } = filter
+  let { value } = filter
 
   // Default the value for noValue fields to ensure they are correctly added
   // to the final query
-  if (queryOperator === "empty" || queryOperator === "notEmpty") {
+  if (operator === "empty" || operator === "notEmpty") {
     value = null
   }
 
-  if (
-    type === "datetime" &&
-    !isHbs &&
-    queryOperator !== "empty" &&
-    queryOperator !== "notEmpty"
+  const isHbs =
+    typeof value === "string" && (value.match(HBS_REGEX) || []).length > 0
+
+  // Parsing value depending on what the type is.
+  switch (type) {
+    case FieldType.DATETIME:
+      if (!isHbs && operator !== "empty" && operator !== "notEmpty") {
+        if (!value) {
+          return
+        }
+        value = new Date(value).toISOString()
+      }
+      break
+    case FieldType.NUMBER:
+      if (typeof value === "string" && !isHbs) {
+        if (operator === "oneOf") {
+          value = value.split(",").map(parseFloat)
+        } else {
+          value = parseFloat(value)
+        }
+      }
+      break
+    case FieldType.BOOLEAN:
+      value = `${value}`.toLowerCase() === "true"
+      break
+    case FieldType.ARRAY:
+      if (
+        ["contains", "notContains", "containsAny"].includes(
+          operator.toLocaleString()
+        ) &&
+        typeof value === "string"
+      ) {
+        value = value.split(",")
+      }
+      break
+  }
+
+  if (isRangeSearchOperator(operator)) {
+    const key = externalType as keyof typeof SqlNumberTypeRangeMap
+    const limits = SqlNumberTypeRangeMap[key] || {
+      min: Number.MIN_SAFE_INTEGER,
+      max: Number.MAX_SAFE_INTEGER,
+    }
+
+    query[operator] ??= {}
+    query[operator][field] = {
+      low: type === "number" ? limits.min : "0000-00-00T00:00:00.000Z",
+      high: type === "number" ? limits.max : "9999-00-00T00:00:00.000Z",
+    }
+  } else if (operator === "rangeHigh" && value != null && value !== "") {
+    query.range ??= {}
+    query.range[field] = {
+      ...query.range[field],
+      high: value,
+    }
+  } else if (operator === "rangeLow" && value != null && value !== "") {
+    query.range ??= {}
+    query.range[field] = {
+      ...query.range[field],
+      low: value,
+    }
+  } else if (
+    isBasicSearchOperator(operator) ||
+    isArraySearchOperator(operator) ||
+    isRangeSearchOperator(operator)
   ) {
-    // Ensure date value is a valid date and parse into correct format
-    if (!value) {
-      return
-    }
-    try {
-      value = new Date(value).toISOString()
-    } catch (error) {
-      return
-    }
-  }
-  if (type === "number" && typeof value === "string" && !isHbs) {
-    if (queryOperator === "oneOf") {
-      value = value.split(",").map(item => parseFloat(item))
-    } else {
-      value = parseFloat(value)
-    }
-  }
-  if (type === "boolean") {
-    value = `${value}`?.toLowerCase() === "true"
-  }
-  if (
-    ["contains", "notContains", "containsAny"].includes(
-      operator.toLocaleString()
-    ) &&
-    type === "array" &&
-    typeof value === "string"
-  ) {
-    value = value.split(",")
-  }
-  if (operator.toLocaleString().startsWith("range") && query.range) {
-    const minint =
-      SqlNumberTypeRangeMap[externalType as keyof typeof SqlNumberTypeRangeMap]
-        ?.min || Number.MIN_SAFE_INTEGER
-    const maxint =
-      SqlNumberTypeRangeMap[externalType as keyof typeof SqlNumberTypeRangeMap]
-        ?.max || Number.MAX_SAFE_INTEGER
-    if (!query.range[field]) {
-      query.range[field] = {
-        low: type === "number" ? minint : "0000-00-00T00:00:00.000Z",
-        high: type === "number" ? maxint : "9999-00-00T00:00:00.000Z",
-      }
-    }
-    if (operator === "rangeLow" && value != null && value !== "") {
-      query.range[field] = {
-        ...query.range[field],
-        low: value,
-      }
-    } else if (operator === "rangeHigh" && value != null && value !== "") {
-      query.range[field] = {
-        ...query.range[field],
-        high: value,
-      }
-    }
-  } else if (isLogicalSearchOperator(queryOperator)) {
-    // TODO
-  } else if (query[queryOperator] && operator !== "onEmptyFilter") {
     if (type === "boolean") {
-      // Transform boolean filters to cope with null.
-      // "equals false" needs to be "not equals true"
-      // "not equals false" needs to be "equals true"
-      if (queryOperator === "equal" && value === false) {
+      // TODO(samwho): I suspect this boolean transformation isn't needed anymore,
+      // write some tests to confirm.
+
+      // Transform boolean filters to cope with null.  "equals false" needs to
+      // be "not equals true" "not equals false" needs to be "equals true"
+      if (operator === "equal" && value === false) {
         query.notEqual = query.notEqual || {}
         query.notEqual[field] = true
-      } else if (queryOperator === "notEqual" && value === false) {
+      } else if (operator === "notEqual" && value === false) {
         query.equal = query.equal || {}
         query.equal[field] = true
       } else {
-        query[queryOperator] ??= {}
-        query[queryOperator]![field] = value
+        query[operator] ??= {}
+        query[operator][field] = value
       }
     } else {
-      query[queryOperator] ??= {}
-      query[queryOperator]![field] = value
+      query[operator] ??= {}
+      query[operator][field] = value
     }
+  } else {
+    throw new Error(`Unsupported operator: ${operator}`)
   }
 
   return query
 }
 
-export const buildQueryLegacy = (
-  filter?: LegacyFilter[] | SearchFilters
-): SearchFilters | undefined => {
-  // this is of type SearchFilters or is undefined
-  if (!Array.isArray(filter)) {
-    return filter
+export interface LegacyFilterSplit {
+  allOr?: boolean
+  onEmptyFilter?: EmptyFilterOption
+  filters: SearchFilter[]
+}
+
+export function splitFiltersArray(filters: LegacyFilter[]) {
+  const split: LegacyFilterSplit = {
+    filters: [],
   }
 
-  let query: SearchFilters = {
-    string: {},
-    fuzzy: {},
-    range: {},
-    equal: {},
-    notEqual: {},
-    empty: {},
-    notEmpty: {},
-    contains: {},
-    notContains: {},
-    oneOf: {},
-    containsAny: {},
+  for (const filter of filters) {
+    if ("operator" in filter && filter.operator === "allOr") {
+      split.allOr = true
+    } else if ("onEmptyFilter" in filter) {
+      split.onEmptyFilter = filter.onEmptyFilter
+    } else {
+      split.filters.push(filter)
+    }
   }
 
-  if (!Array.isArray(filter)) {
-    return query
-  }
-
-  filter.forEach(expression => {
-    let { operator, field, type, value, externalType, onEmptyFilter } =
-      expression
-    const queryOperator = operator as SearchFilterOperator
-    const isHbs =
-      typeof value === "string" && (value.match(HBS_REGEX) || []).length > 0
-    // Parse all values into correct types
-    if (operator === "allOr") {
-      query.allOr = true
-      return
-    }
-    if (onEmptyFilter) {
-      query.onEmptyFilter = onEmptyFilter
-      return
-    }
-    if (
-      type === "datetime" &&
-      !isHbs &&
-      queryOperator !== "empty" &&
-      queryOperator !== "notEmpty"
-    ) {
-      // Ensure date value is a valid date and parse into correct format
-      if (!value) {
-        return
-      }
-      try {
-        value = new Date(value).toISOString()
-      } catch (error) {
-        return
-      }
-    }
-    if (type === "number" && typeof value === "string" && !isHbs) {
-      if (queryOperator === "oneOf") {
-        value = value.split(",").map(item => parseFloat(item))
-      } else {
-        value = parseFloat(value)
-      }
-    }
-    if (type === "boolean") {
-      value = `${value}`?.toLowerCase() === "true"
-    }
-    if (
-      ["contains", "notContains", "containsAny"].includes(
-        operator.toLocaleString()
-      ) &&
-      type === "array" &&
-      typeof value === "string"
-    ) {
-      value = value.split(",")
-    }
-    if (operator.toLocaleString().startsWith("range") && query.range) {
-      const minint =
-        SqlNumberTypeRangeMap[
-          externalType as keyof typeof SqlNumberTypeRangeMap
-        ]?.min || Number.MIN_SAFE_INTEGER
-      const maxint =
-        SqlNumberTypeRangeMap[
-          externalType as keyof typeof SqlNumberTypeRangeMap
-        ]?.max || Number.MAX_SAFE_INTEGER
-      if (!query.range[field]) {
-        query.range[field] = {
-          low: type === "number" ? minint : "0000-00-00T00:00:00.000Z",
-          high: type === "number" ? maxint : "9999-00-00T00:00:00.000Z",
-        }
-      }
-      if (operator === "rangeLow" && value != null && value !== "") {
-        query.range[field] = {
-          ...query.range[field],
-          low: value,
-        }
-      } else if (operator === "rangeHigh" && value != null && value !== "") {
-        query.range[field] = {
-          ...query.range[field],
-          high: value,
-        }
-      }
-    } else if (isLogicalSearchOperator(queryOperator)) {
-      // ignore
-    } else if (query[queryOperator] && operator !== "onEmptyFilter") {
-      if (type === "boolean") {
-        // Transform boolean filters to cope with null.
-        // "equals false" needs to be "not equals true"
-        // "not equals false" needs to be "equals true"
-        if (queryOperator === "equal" && value === false) {
-          query.notEqual = query.notEqual || {}
-          query.notEqual[field] = true
-        } else if (queryOperator === "notEqual" && value === false) {
-          query.equal = query.equal || {}
-          query.equal[field] = true
-        } else {
-          query[queryOperator] ??= {}
-          query[queryOperator]![field] = value
-        }
-      } else {
-        query[queryOperator] ??= {}
-        query[queryOperator]![field] = value
-      }
-    }
-  })
-  return query
+  return split
 }
 
 /**
- * Converts a **SearchFilterGroup** filter definition into a grouped
+ * Converts a **UISearchFilter** filter definition into a grouped
  * search query of type **SearchFilters**
  *
  * Legacy support remains for the old **SearchFilter[]** format.
  * These will be migrated to an appropriate **SearchFilters** object, if encountered
- *
- * @param filter
- *
- * @returns {SearchFilters}
  */
-
-export const buildQuery = (
-  filter?: SearchFilterGroup | LegacyFilter[]
-): SearchFilters | undefined => {
-  const parsedFilter: SearchFilterGroup | undefined =
-    processSearchFilters(filter)
-
-  if (!parsedFilter) {
+export function buildQuery(filter: undefined): undefined
+export function buildQuery(
+  filter: UISearchFilter | LegacyFilter[]
+): SearchFilters
+export function buildQuery(
+  filter?: UISearchFilter | LegacyFilter[]
+): SearchFilters | undefined {
+  if (!filter) {
     return
   }
 
-  const operatorMap: { [key in FilterGroupLogicalOperator]: LogicalOperator } =
-    {
-      [FilterGroupLogicalOperator.ALL]: LogicalOperator.AND,
-      [FilterGroupLogicalOperator.ANY]: LogicalOperator.OR,
-    }
-
-  const globalOnEmpty = parsedFilter.onEmptyFilter
-    ? parsedFilter.onEmptyFilter
-    : null
-
-  const globalOperator: LogicalOperator =
-    operatorMap[parsedFilter.logicalOperator as FilterGroupLogicalOperator]
-
-  return {
-    ...(globalOnEmpty ? { onEmptyFilter: globalOnEmpty } : {}),
-    [globalOperator]: {
-      conditions: parsedFilter.groups?.map((group: SearchFilterGroup) => {
-        return {
-          [operatorMap[group.logicalOperator]]: {
-            conditions: group.filters
-              ?.map(x => buildCondition(x))
-              .filter(filter => filter),
-          },
-        }
-      }),
-    },
+  if (Array.isArray(filter)) {
+    filter = processSearchFilters(filter)
   }
+
+  const operator = logicalOperatorFromUI(
+    filter.logicalOperator || UILogicalOperator.ALL
+  )
+
+  const query: SearchFilters = {}
+  if (filter.onEmptyFilter) {
+    query.onEmptyFilter = filter.onEmptyFilter
+  } else {
+    query.onEmptyFilter = EmptyFilterOption.RETURN_ALL
+  }
+
+  query[operator] = {
+    conditions: (filter.groups || []).map(group => {
+      const { allOr, onEmptyFilter, filters } = splitFiltersArray(
+        group.filters || []
+      )
+      if (onEmptyFilter) {
+        query.onEmptyFilter = onEmptyFilter
+      }
+      const operator = allOr ? LogicalOperator.OR : LogicalOperator.AND
+      return {
+        [operator]: { conditions: filters.map(buildCondition).filter(f => f) },
+      }
+    }),
+  }
+
+  return query
+}
+
+function logicalOperatorFromUI(operator: UILogicalOperator): LogicalOperator {
+  return operator === UILogicalOperator.ALL
+    ? LogicalOperator.AND
+    : LogicalOperator.OR
 }
 
 // The frontend can send single values for array fields sometimes, so to handle

packages/shared-core/src/utils.ts

@@ -1,18 +1,28 @@
 import {
   LegacyFilter,
-  SearchFilterGroup,
-  FilterGroupLogicalOperator,
+  UISearchFilter,
+  UILogicalOperator,
   SearchFilters,
   BasicOperator,
   ArrayOperator,
   isLogicalSearchOperator,
+  SearchFilter,
+  EmptyFilterOption,
 } from "@budibase/types"
 import * as Constants from "./constants"
-import { removeKeyNumbering } from "./filters"
+import { removeKeyNumbering, splitFiltersArray } from "./filters"
+import _ from "lodash"
 
-// an array of keys from filter type to properties that are in the type
-// this can then be converted using .fromEntries to an object
-type AllowedFilters = [keyof LegacyFilter, LegacyFilter[keyof LegacyFilter]][]
+const FILTER_ALLOWED_KEYS: (keyof SearchFilter)[] = [
+  "field",
+  "operator",
+  "value",
+  "type",
+  "externalType",
+  "valueType",
+  "noValue",
+  "formulaType",
+]
 
 export function unreachable(
   value: never,
@@ -128,97 +138,25 @@ export function isSupportedUserSearch(query: SearchFilters) {
   return true
 }
 
-/**
- * Processes the filter config. Filters are migrated from
- * SearchFilter[] to SearchFilterGroup
- *
- * If filters is not an array, the migration is skipped
- *
- * @param {LegacyFilter[] | SearchFilterGroup} filters
- */
 export const processSearchFilters = (
-  filters: LegacyFilter[] | SearchFilterGroup | undefined
-): SearchFilterGroup | undefined => {
-  if (!filters) {
-    return
+  filterArray: LegacyFilter[]
+): Required<UISearchFilter> => {
+  const { allOr, onEmptyFilter, filters } = splitFiltersArray(filterArray)
+  return {
+    logicalOperator: UILogicalOperator.ALL,
+    onEmptyFilter: onEmptyFilter || EmptyFilterOption.RETURN_ALL,
+    groups: [
+      {
+        logicalOperator: allOr ? UILogicalOperator.ANY : UILogicalOperator.ALL,
+        filters: filters.map(filter => {
+          const trimmedFilter = _.pick(
+            filter,
+            FILTER_ALLOWED_KEYS
+          ) as SearchFilter
+          trimmedFilter.field = removeKeyNumbering(trimmedFilter.field)
+          return trimmedFilter
+        }),
+      },
+    ],
   }
-
-  // Base search config.
-  const defaultCfg: SearchFilterGroup = {
-    logicalOperator: FilterGroupLogicalOperator.ALL,
-    groups: [],
-  }
-
-  const filterAllowedKeys = [
-    "field",
-    "operator",
-    "value",
-    "type",
-    "externalType",
-    "valueType",
-    "noValue",
-    "formulaType",
-  ]
-
-  if (Array.isArray(filters)) {
-    let baseGroup: SearchFilterGroup = {
-      filters: [],
-      logicalOperator: FilterGroupLogicalOperator.ALL,
-    }
-
-    return filters.reduce((acc: SearchFilterGroup, filter: LegacyFilter) => {
-      // Sort the properties for easier debugging
-      const filterPropertyKeys = (Object.keys(filter) as (keyof LegacyFilter)[])
-        .sort((a, b) => {
-          return a.localeCompare(b)
-        })
-        .filter(key => filter[key])
-
-      if (filterPropertyKeys.length == 1) {
-        const key = filterPropertyKeys[0],
-          value = filter[key]
-        // Global
-        if (key === "onEmptyFilter") {
-          // unset otherwise
-          acc.onEmptyFilter = value
-        } else if (key === "operator" && value === "allOr") {
-          // Group 1 logical operator
-          baseGroup.logicalOperator = FilterGroupLogicalOperator.ANY
-        }
-
-        return acc
-      }
-
-      const allowedFilterSettings: AllowedFilters = filterPropertyKeys.reduce(
-        (acc: AllowedFilters, key) => {
-          const value = filter[key]
-          if (filterAllowedKeys.includes(key)) {
-            if (key === "field") {
-              acc.push([key, removeKeyNumbering(value)])
-            } else {
-              acc.push([key, value])
-            }
-          }
-          return acc
-        },
-        []
-      )
-
-      const migratedFilter: LegacyFilter = Object.fromEntries(
-        allowedFilterSettings
-      ) as LegacyFilter
-
-      baseGroup.filters!.push(migratedFilter)
-
-      if (!acc.groups || !acc.groups.length) {
-        // init the base group
-        acc.groups = [baseGroup]
-      }
-
-      return acc
-    }, defaultCfg)
-  } else if (!filters?.groups) {
-    return
-  }
-  return filters
 }

packages/types/src/api/web/searchFilter.ts

@@ -1,23 +1,59 @@
 import { FieldType } from "../../documents"
 import {
   EmptyFilterOption,
-  FilterGroupLogicalOperator,
-  SearchFilters,
+  UILogicalOperator,
+  BasicOperator,
+  RangeOperator,
+  ArrayOperator,
 } from "../../sdk"
 
-export type LegacyFilter = {
-  operator: keyof SearchFilters | "rangeLow" | "rangeHigh"
-  onEmptyFilter?: EmptyFilterOption
-  field: string
-  type?: FieldType
-  value: any
-  externalType?: string
+type AllOr = {
+  operator: "allOr"
 }
 
-// this is a type purely used by the UI
+type OnEmptyFilter = {
+  onEmptyFilter: EmptyFilterOption
+}
+
+// TODO(samwho): this could be broken down further
+export type SearchFilter = {
+  operator:
+    | BasicOperator
+    | RangeOperator
+    | ArrayOperator
+    | "rangeLow"
+    | "rangeHigh"
+  // Field name will often have a numerical prefix when coming from the frontend,
+  // use the ColumnSplitter class to remove it.
+  field: string
+  value: any
+  type?: FieldType
+  externalType?: string
+  noValue?: boolean
+  valueType?: string
+  formulaType?: string
+}
+
+// Prior to v2, this is the type the frontend sent us when filters were
+// involved. We convert this to a SearchFilters before use with the search SDK.
+export type LegacyFilter = AllOr | OnEmptyFilter | SearchFilter
+
 export type SearchFilterGroup = {
-  logicalOperator: FilterGroupLogicalOperator
-  onEmptyFilter?: EmptyFilterOption
+  logicalOperator?: UILogicalOperator
   groups?: SearchFilterGroup[]
   filters?: LegacyFilter[]
 }
+
+// As of v3, this is the format that the frontend always sends when search
+// filters are involved. We convert this to SearchFilters before use with the
+// search SDK.
+//
+// The reason we migrated was that we started to support "logical operators" in
+// tests and SearchFilters because a recursive data structure. LegacyFilter[]
+// wasn't able to support these sorts of recursive structures, so we changed the
+// format.
+export type UISearchFilter = {
+  logicalOperator?: UILogicalOperator
+  onEmptyFilter?: EmptyFilterOption
+  groups?: SearchFilterGroup[]
+}

packages/types/src/documents/app/view.ts

@@ -1,4 +1,4 @@
-import { LegacyFilter, SearchFilterGroup, SortOrder, SortType } from "../../api"
+import { LegacyFilter, UISearchFilter, SortOrder, SortType } from "../../api"
 import { UIFieldMetadata } from "./table"
 import { Document } from "../document"
 import { DBView, SearchFilters } from "../../sdk"
@@ -92,7 +92,7 @@ export interface ViewV2 {
   tableId: string
   query?: LegacyFilter[] | SearchFilters
   // duplicate to store UI information about filters
-  queryUI?: SearchFilterGroup
+  queryUI?: UISearchFilter
   sort?: {
     field: string
     order?: SortOrder

packages/types/src/sdk/search.ts

@@ -32,7 +32,19 @@ export enum LogicalOperator {
 export function isLogicalSearchOperator(
   value: string
 ): value is LogicalOperator {
-  return value === LogicalOperator.AND || value === LogicalOperator.OR
+  return Object.values(LogicalOperator).includes(value as LogicalOperator)
+}
+
+export function isBasicSearchOperator(value: string): value is BasicOperator {
+  return Object.values(BasicOperator).includes(value as BasicOperator)
+}
+
+export function isArraySearchOperator(value: string): value is ArrayOperator {
+  return Object.values(ArrayOperator).includes(value as ArrayOperator)
+}
+
+export function isRangeSearchOperator(value: string): value is RangeOperator {
+  return Object.values(RangeOperator).includes(value as RangeOperator)
 }
 
 export type SearchFilterOperator =
@@ -191,7 +203,7 @@ export enum EmptyFilterOption {
   RETURN_NONE = "none",
 }
 
-export enum FilterGroupLogicalOperator {
+export enum UILogicalOperator {
   ALL = "all",
   ANY = "any",
 }