Making progress toward testing buildCondition and friends.

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

@@ -27,6 +27,8 @@ import {
   ViewV2Schema,
   ViewV2Type,
   JsonTypes,
+  FilterGroupLogicalOperator,
+  EmptyFilterOption,
 } from "@budibase/types"
 import { generator, mocks } from "@budibase/backend-core/tests"
 import { DatabaseName, getDatasource } from "../../../integrations/tests/utils"
@@ -145,17 +147,26 @@ describe.each([
       })
 
       it.only("can persist views with all fields", async () => {
-        const newView: Required<Omit<CreateViewRequest, "queryUI" | "type">> = {
+        const newView: Required<Omit<CreateViewRequest, "query" | "type">> = {
           name: generator.name(),
           tableId: table._id!,
           primaryDisplay: "id",
-          query: [
+          queryUI: {
-            {
+            logicalOperator: FilterGroupLogicalOperator.ALL,
-              operator: BasicOperator.EQUAL,
+            onEmptyFilter: EmptyFilterOption.RETURN_ALL,
-              field: "field",
+            groups: [
-              value: "value",
+              {
-            },
+                logicalOperator: FilterGroupLogicalOperator.ALL,
-          ],
+                filters: [
+                  {
+                    operator: BasicOperator.EQUAL,
+                    field: "field",
+                    value: "value",
+                  },
+                ],
+              },
+            ],
+          },
           sort: {
             field: "fieldToSort",
             order: SortOrder.DESCENDING,
@@ -170,7 +181,7 @@ describe.each([
         }
         const res = await config.api.viewV2.create(newView)
 
-        expect(res).toEqual({
+        const expected: ViewV2 = {
           ...newView,
           schema: {
             id: { visible: true },
@@ -178,10 +189,29 @@ describe.each([
               visible: true,
             },
           },
-          queryUI: {},
+          query: {
+            onEmptyFilter: EmptyFilterOption.RETURN_ALL,
+            $and: {
+              conditions: [
+                {
+                  $and: {
+                    conditions: [
+                      {
+                        equal: {
+                          field: "value",
+                        },
+                      },
+                    ],
+                  },
+                },
+              ],
+            },
+          },
           id: expect.any(String),
           version: 2,
-        })
+        }
+
+        expect(res).toEqual(expected)
       })
 
       it("persist only UI schema overrides", async () => {

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,6 +19,7 @@ export async function get(viewId: string): Promise<ViewV2> {
   if (!found) {
     throw new Error("No view found")
   }
+  ensureQueryUISet(found)
   return found
 }
 
@@ -33,6 +35,7 @@ export async function getEnriched(viewId: string): Promise<ViewV2Enriched> {
   if (!found) {
     throw new Error("No view found")
   }
+  ensureQueryUISet(found)
   return await enrichSchema(found, table.schema)
 }
 
@@ -46,6 +49,8 @@ export async function create(
     version: 2,
   }
 
+  ensureQuerySet(view)
+
   const db = context.getAppDB()
 
   const { datasourceId, tableName } = breakExternalTableId(tableId)
@@ -74,6 +79,8 @@ export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
     throw new HTTPError(`Cannot update view type after creation`, 400)
   }
 
+  ensureQuerySet(view)
+
   delete views[existingView.name]
   views[view.name] = view
   await db.put(ds)

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

@@ -4,19 +4,7 @@ import { context, HTTPError } from "@budibase/backend-core"
 import sdk from "../../../sdk"
 import * as utils from "../../../db/utils"
 import { enrichSchema, isV2 } from "."
-import { utils as sharedUtils, dataFilters } from "@budibase/shared-core"
+import { ensureQuerySet, ensureQueryUISet } from "./utils"
-
-function ensureQueryUISet(view: ViewV2) {
-  if (!view.queryUI && view.query) {
-    view.queryUI = sharedUtils.processSearchFilters(view.query)
-  }
-}
-
-function ensureQuerySet(view: ViewV2) {
-  if (!view.query && view.queryUI) {
-    view.query = dataFilters.buildQuery(view.queryUI)
-  }
-}
 
 export async function get(viewId: string): Promise<ViewV2> {
   const { tableId } = utils.extractViewInfoFromID(viewId)
@@ -52,6 +40,8 @@ export async function create(
     version: 2,
   }
 
+  ensureQuerySet(view)
+
   const db = context.getAppDB()
 
   const table = await sdk.tables.getTable(tableId)
@@ -78,6 +68,8 @@ export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {
     throw new HTTPError(`Cannot update view type after creation`, 400)
   }
 
+  ensureQuerySet(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,30 @@
+import { ViewV2 } from "@budibase/types"
+import { utils, dataFilters } from "@budibase/shared-core"
+
+export function ensureQueryUISet(view: ViewV2) {
+  if (!view.queryUI && 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 SearchFilterGroup (the type of queryUI) to SearchFilters,
+      // but not LegacyFilter[], they are incompatible due to SearchFilterGroup
+      // 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 Error("view is missing queryUI field")
+    }
+
+    view.queryUI = utils.processSearchFilters(view.query)
+  }
+}
+
+export function ensureQuerySet(view: ViewV2) {
+  if (!view.query && view.queryUI) {
+    view.query = dataFilters.buildQuery(view.queryUI)
+  }
+}

packages/shared-core/src/filters.ts

@@ -21,6 +21,9 @@ import {
   isLogicalSearchOperator,
   SearchFilterGroup,
   FilterGroupLogicalOperator,
+  isBasicSearchOperator,
+  isArraySearchOperator,
+  isRangeSearchOperator,
 } from "@budibase/types"
 import dayjs from "dayjs"
 import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants"
@@ -307,36 +310,23 @@ export class ColumnSplitter {
  * Builds a JSON query from the filter a SearchFilter definition
  * @param filter the builder filter structure
  */
-
 const buildCondition = (expression: LegacyFilter) => {
-  // Filter body
+  const query: SearchFilters = {}
-  let query: SearchFilters = {
+  const { operator, field, type, externalType, onEmptyFilter } = expression
-    string: {},
+  let { value } = expression
-    fuzzy: {},
-    range: {},
-    equal: {},
-    notEqual: {},
-    empty: {},
-    notEmpty: {},
-    contains: {},
-    notContains: {},
-    oneOf: {},
-    containsAny: {},
-  }
-  let { operator, field, type, value, externalType, onEmptyFilter } = expression
 
   if (!operator || !field) {
     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
@@ -344,15 +334,15 @@ const buildCondition = (expression: LegacyFilter) => {
 
   // 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" &&
+    operator !== "empty" &&
-    queryOperator !== "notEmpty"
+    operator !== "notEmpty"
   ) {
     // Ensure date value is a valid date and parse into correct format
     if (!value) {
@@ -365,7 +355,7 @@ const buildCondition = (expression: LegacyFilter) => {
     }
   }
   if (type === "number" && typeof value === "string" && !isHbs) {
-    if (queryOperator === "oneOf") {
+    if (operator === "oneOf") {
       value = value.split(",").map(item => parseFloat(item))
     } else {
       value = parseFloat(value)
@@ -383,50 +373,55 @@ const buildCondition = (expression: LegacyFilter) => {
   ) {
     value = value.split(",")
   }
-  if (operator.toLocaleString().startsWith("range") && query.range) {
+
-    const minint =
+  if (isRangeSearchOperator(operator)) {
-      SqlNumberTypeRangeMap[externalType as keyof typeof SqlNumberTypeRangeMap]
+    const key = externalType as keyof typeof SqlNumberTypeRangeMap
-        ?.min || Number.MIN_SAFE_INTEGER
+    const limits = SqlNumberTypeRangeMap[key] || {
-    const maxint =
+      min: Number.MIN_SAFE_INTEGER,
-      SqlNumberTypeRangeMap[externalType as keyof typeof SqlNumberTypeRangeMap]
+      max: Number.MAX_SAFE_INTEGER,
-        ?.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[operator] ??= {}
-        ...query.range[field],
+    query[operator][field] = {
-        low: value,
+      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[field] = {
-        ...query.range[field],
-        high: value,
-      }
     }
-  } else if (isLogicalSearchOperator(queryOperator)) {
+  } 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 (isLogicalSearchOperator(operator)) {
     // TODO
-  } else if (query[queryOperator] && operator !== "onEmptyFilter") {
+  } else if (
+    isBasicSearchOperator(operator) ||
+    isArraySearchOperator(operator) ||
+    isRangeSearchOperator(operator)
+  ) {
     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) {
+      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[operator] ??= {}
-        query[queryOperator]![field] = value
+        query[operator][field] = value
       }
     } else {
-      query[queryOperator] ??= {}
+      query[operator] ??= {}
-      query[queryOperator]![field] = value
+      query[operator][field] = value
     }
   }
 
@@ -604,7 +599,7 @@ export function buildQuery(
   return {
     ...(globalOnEmpty ? { onEmptyFilter: globalOnEmpty } : {}),
     [globalOperator]: {
-      conditions: parsedFilter.groups?.map((group: SearchFilterGroup) => {
+      conditions: parsedFilter.groups?.map(group => {
         return {
           [operatorMap[group.logicalOperator]]: {
             conditions: group.filters

packages/shared-core/src/utils.ts

@@ -6,6 +6,8 @@ import {
   BasicOperator,
   ArrayOperator,
   isLogicalSearchOperator,
+  EmptyFilterOption,
+  SearchFilterChild,
 } from "@budibase/types"
 import * as Constants from "./constants"
 import { removeKeyNumbering } from "./filters"
@@ -142,6 +144,7 @@ export const processSearchFilters = (
   // Base search config.
   const defaultCfg: SearchFilterGroup = {
     logicalOperator: FilterGroupLogicalOperator.ALL,
+    onEmptyFilter: EmptyFilterOption.RETURN_ALL,
     groups: [],
   }
 
@@ -156,8 +159,7 @@ export const processSearchFilters = (
     "formulaType",
   ]
 
-  let baseGroup: SearchFilterGroup = {
+  let baseGroup: SearchFilterChild = {
-    filters: [],
     logicalOperator: FilterGroupLogicalOperator.ALL,
   }
 

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

@@ -14,10 +14,15 @@ export type LegacyFilter = {
   externalType?: string
 }
 
+export type SearchFilterChild = {
+  logicalOperator: FilterGroupLogicalOperator
+  groups?: SearchFilterChild[]
+  filters?: LegacyFilter[]
+}
+
 // this is a type purely used by the UI
 export type SearchFilterGroup = {
   logicalOperator: FilterGroupLogicalOperator
-  onEmptyFilter?: EmptyFilterOption
+  onEmptyFilter: EmptyFilterOption
-  groups?: SearchFilterGroup[]
+  groups: SearchFilterChild[]
-  filters?: LegacyFilter[]
 }

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 =