From cb41861d13342d6ffcda69d2d4058dcf08857084 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 17 Oct 2024 11:54:34 +0100 Subject: [PATCH] Making progress toward testing buildCondition and friends. --- .../src/api/routes/tests/viewV2.spec.ts | 52 +++++++-- packages/server/src/sdk/app/views/external.ts | 7 ++ packages/server/src/sdk/app/views/internal.ts | 18 +--- packages/server/src/sdk/app/views/utils.ts | 30 ++++++ packages/shared-core/src/filters.ts | 101 +++++++++--------- packages/shared-core/src/utils.ts | 6 +- packages/types/src/api/web/searchFilter.ts | 11 +- packages/types/src/sdk/search.ts | 14 ++- 8 files changed, 156 insertions(+), 83 deletions(-) create mode 100644 packages/server/src/sdk/app/views/utils.ts diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 7c866e19fe..b81c731485 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/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> = { + const newView: Required> = { name: generator.name(), tableId: table._id!, primaryDisplay: "id", - query: [ - { - operator: BasicOperator.EQUAL, - field: "field", - value: "value", - }, - ], + queryUI: { + logicalOperator: FilterGroupLogicalOperator.ALL, + onEmptyFilter: EmptyFilterOption.RETURN_ALL, + groups: [ + { + 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 () => { diff --git a/packages/server/src/sdk/app/views/external.ts b/packages/server/src/sdk/app/views/external.ts index b69ac0b9eb..d5251122c9 100644 --- a/packages/server/src/sdk/app/views/external.ts +++ b/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 { const { tableId } = utils.extractViewInfoFromID(viewId) @@ -18,6 +19,7 @@ export async function get(viewId: string): Promise { if (!found) { throw new Error("No view found") } + ensureQueryUISet(found) return found } @@ -33,6 +35,7 @@ export async function getEnriched(viewId: string): Promise { 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 { throw new HTTPError(`Cannot update view type after creation`, 400) } + ensureQuerySet(view) + delete views[existingView.name] views[view.name] = view await db.put(ds) diff --git a/packages/server/src/sdk/app/views/internal.ts b/packages/server/src/sdk/app/views/internal.ts index 30e0c18d05..33b68759d7 100644 --- a/packages/server/src/sdk/app/views/internal.ts +++ b/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" - -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) - } -} +import { ensureQuerySet, ensureQueryUISet } from "./utils" export async function get(viewId: string): Promise { 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 { 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) diff --git a/packages/server/src/sdk/app/views/utils.ts b/packages/server/src/sdk/app/views/utils.ts new file mode 100644 index 0000000000..45b091e046 --- /dev/null +++ b/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) + } +} diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 64958dd4f7..3b9253af6e 100644 --- a/packages/shared-core/src/filters.ts +++ b/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 - let query: SearchFilters = { - string: {}, - fuzzy: {}, - range: {}, - equal: {}, - notEqual: {}, - empty: {}, - notEmpty: {}, - contains: {}, - notContains: {}, - oneOf: {}, - containsAny: {}, - } - let { operator, field, type, value, externalType, onEmptyFilter } = expression + const query: SearchFilters = {} + const { operator, field, type, externalType, onEmptyFilter } = expression + let { value } = 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" && - queryOperator !== "notEmpty" + operator !== "empty" && + 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 = - 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 (isRangeSearchOperator(operator)) { + const key = externalType as keyof typeof SqlNumberTypeRangeMap + const limits = SqlNumberTypeRangeMap[key] || { + min: Number.MIN_SAFE_INTEGER, + max: Number.MAX_SAFE_INTEGER, } - 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, - } + + 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 (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[queryOperator]![field] = value + query[operator] ??= {} + query[operator][field] = value } } else { - query[queryOperator] ??= {} - query[queryOperator]![field] = value + query[operator] ??= {} + 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 diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 7f19371d79..98f7c9b215 100644 --- a/packages/shared-core/src/utils.ts +++ b/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 = { - filters: [], + let baseGroup: SearchFilterChild = { logicalOperator: FilterGroupLogicalOperator.ALL, } diff --git a/packages/types/src/api/web/searchFilter.ts b/packages/types/src/api/web/searchFilter.ts index 23c599027e..305f2bab00 100644 --- a/packages/types/src/api/web/searchFilter.ts +++ b/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 - groups?: SearchFilterGroup[] - filters?: LegacyFilter[] + onEmptyFilter: EmptyFilterOption + groups: SearchFilterChild[] } diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index d41bb0fb99..d64e87d434 100644 --- a/packages/types/src/sdk/search.ts +++ b/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 =