From b57a8c11306d3b543e756b75f268d7e0ec76eec4 Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Thu, 17 Aug 2023 13:31:52 +0100 Subject: [PATCH] Move when filter empty option into filter drawer (#11262) * Add when filter empty dropdown * Add unit tests * "fix" typescript issue * Add empty filter check for external * Add unit test * Hide empty filter option for custom query * Make onEmptyFilter optional * Fix unit tests * Remove onEmptyFilter automation input * Remove unused var * Refactor * Fix path * Fix type issue * Fix types --- packages/backend-core/src/db/lucene.ts | 26 +++++-- .../backend-core/src/db/tests/lucene.spec.ts | 72 ++++++++++++++++++- .../controls/FilterEditor/FilterDrawer.svelte | 30 ++++++-- packages/server/jest.config.ts | 1 + .../src/api/controllers/row/external.ts | 9 +++ .../server/src/automations/steps/queryRows.ts | 17 +---- .../app/rows/search/tests/external.spec.ts | 17 +++++ packages/shared-core/src/filters.ts | 15 ++-- packages/types/src/api/web/searchFilter.ts | 5 +- packages/types/src/sdk/search.ts | 6 ++ 10 files changed, 165 insertions(+), 33 deletions(-) diff --git a/packages/backend-core/src/db/lucene.ts b/packages/backend-core/src/db/lucene.ts index a491451a62..7451d581b5 100644 --- a/packages/backend-core/src/db/lucene.ts +++ b/packages/backend-core/src/db/lucene.ts @@ -1,7 +1,6 @@ import fetch from "node-fetch" import { getCouchInfo } from "./couch" -import { SearchFilters, Row } from "@budibase/types" -import { createUserIndex } from "./searchIndexes/searchIndexes" +import { SearchFilters, Row, EmptyFilterOption } from "@budibase/types" const QUERY_START_REGEX = /\d[0-9]*:/g @@ -65,6 +64,7 @@ export class QueryBuilder { this.#index = index this.#query = { allOr: false, + onEmptyFilter: EmptyFilterOption.RETURN_ALL, string: {}, fuzzy: {}, range: {}, @@ -218,6 +218,10 @@ export class QueryBuilder { this.#query.allOr = true } + setOnEmptyFilter(value: EmptyFilterOption) { + this.#query.onEmptyFilter = value + } + handleSpaces(input: string) { if (this.#noEscaping) { return input @@ -289,8 +293,9 @@ export class QueryBuilder { const builder = this let allOr = this.#query && this.#query.allOr let query = allOr ? "" : "*:*" + let allFiltersEmpty = true const allPreProcessingOpts = { escape: true, lowercase: true, wrap: true } - let tableId + let tableId: string = "" if (this.#query.equal!.tableId) { tableId = this.#query.equal!.tableId delete this.#query.equal!.tableId @@ -305,7 +310,7 @@ export class QueryBuilder { } const contains = (key: string, value: any, mode = "AND") => { - if (Array.isArray(value) && value.length === 0) { + if (!value || (Array.isArray(value) && value.length === 0)) { return null } if (!Array.isArray(value)) { @@ -384,6 +389,12 @@ export class QueryBuilder { built += ` ${mode} ` } built += expression + if ( + (typeof value !== "string" && value != null) || + (typeof value === "string" && value !== tableId && value !== "") + ) { + allFiltersEmpty = false + } } if (opts?.returnBuilt) { return built @@ -463,6 +474,13 @@ export class QueryBuilder { allOr = false build({ tableId }, equal) } + if (allFiltersEmpty) { + if (this.#query.onEmptyFilter === EmptyFilterOption.RETURN_NONE) { + return "" + } else if (this.#query?.allOr) { + return query.replace("()", "(*:*)") + } + } return query } diff --git a/packages/backend-core/src/db/tests/lucene.spec.ts b/packages/backend-core/src/db/tests/lucene.spec.ts index a82828d8f2..7716661d88 100644 --- a/packages/backend-core/src/db/tests/lucene.spec.ts +++ b/packages/backend-core/src/db/tests/lucene.spec.ts @@ -1,6 +1,6 @@ import { newid } from "../../docIds/newid" import { getDB } from "../db" -import { Database } from "@budibase/types" +import { Database, EmptyFilterOption } from "@budibase/types" import { QueryBuilder, paginatedSearch, fullSearch } from "../lucene" const INDEX_NAME = "main" @@ -156,6 +156,76 @@ describe("lucene", () => { expect(resp.rows.length).toBe(2) }) + describe("empty filters behaviour", () => { + it("should return all rows by default", async () => { + const builder = new QueryBuilder(dbName, INDEX_NAME) + builder.addEqual("property", "") + builder.addEqual("number", null) + builder.addString("property", "") + builder.addFuzzy("property", "") + builder.addNotEqual("number", undefined) + builder.addOneOf("number", null) + builder.addContains("array", undefined) + builder.addNotContains("array", null) + builder.addContainsAny("array", null) + + const resp = await builder.run() + expect(resp.rows.length).toBe(3) + }) + + it("should return all rows when onEmptyFilter is ALL", async () => { + const builder = new QueryBuilder(dbName, INDEX_NAME) + builder.setOnEmptyFilter(EmptyFilterOption.RETURN_ALL) + builder.setAllOr() + builder.addEqual("property", "") + builder.addEqual("number", null) + builder.addString("property", "") + builder.addFuzzy("property", "") + builder.addNotEqual("number", undefined) + builder.addOneOf("number", null) + builder.addContains("array", undefined) + builder.addNotContains("array", null) + builder.addContainsAny("array", null) + + const resp = await builder.run() + expect(resp.rows.length).toBe(3) + }) + + it("should return no rows when onEmptyFilter is NONE", async () => { + const builder = new QueryBuilder(dbName, INDEX_NAME) + builder.setOnEmptyFilter(EmptyFilterOption.RETURN_NONE) + builder.addEqual("property", "") + builder.addEqual("number", null) + builder.addString("property", "") + builder.addFuzzy("property", "") + builder.addNotEqual("number", undefined) + builder.addOneOf("number", null) + builder.addContains("array", undefined) + builder.addNotContains("array", null) + builder.addContainsAny("array", null) + + const resp = await builder.run() + expect(resp.rows.length).toBe(0) + }) + + it("should return all matching rows when onEmptyFilter is NONE, but a filter value is provided", async () => { + const builder = new QueryBuilder(dbName, INDEX_NAME) + builder.setOnEmptyFilter(EmptyFilterOption.RETURN_NONE) + builder.addEqual("property", "") + builder.addEqual("number", 1) + builder.addString("property", "") + builder.addFuzzy("property", "") + builder.addNotEqual("number", undefined) + builder.addOneOf("number", null) + builder.addContains("array", undefined) + builder.addNotContains("array", null) + builder.addContainsAny("array", null) + + const resp = await builder.run() + expect(resp.rows.length).toBe(1) + }) + }) + describe("skip", () => { const skipDbName = `db-${newid()}` let docs: { diff --git a/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte b/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte index e9f6e5629f..ef8699824e 100644 --- a/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte +++ b/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte @@ -35,22 +35,28 @@ { value: "and", label: "Match all filters" }, { value: "or", label: "Match any filter" }, ] + const onEmptyOptions = [ + { value: "all", label: "Return all table rows" }, + { value: "none", label: "Return no rows" }, + ] let rawFilters let matchAny = false + let onEmptyFilter = "all" $: parseFilters(filters) - $: dispatch("change", enrichFilters(rawFilters, matchAny)) + $: dispatch("change", enrichFilters(rawFilters, matchAny, onEmptyFilter)) $: enrichedSchemaFields = getFields(schemaFields || [], { allowLinks: true }) $: fieldOptions = enrichedSchemaFields.map(field => field.name) || [] $: valueTypeOptions = allowBindings ? ["Value", "Binding"] : ["Value"] - // Remove field key prefixes and determine whether to use the "match all" - // or "match any" behaviour + // Remove field key prefixes and determine which behaviours to use const parseFilters = filters => { matchAny = filters?.find(filter => filter.operator === "allOr") != null + onEmptyFilter = + filters?.find(filter => filter.onEmptyFilter)?.onEmptyFilter ?? "all" rawFilters = (filters || []) - .filter(filter => filter.operator !== "allOr") + .filter(filter => filter.operator !== "allOr" && !filter.onEmptyFilter) .map(filter => { const { field } = filter let newFilter = { ...filter } @@ -74,8 +80,8 @@ }) // Add field key prefixes and a special metadata filter object to indicate - // whether to use the "match all" or "match any" behaviour - const enrichFilters = (rawFilters, matchAny) => { + // how to handle filter behaviour + const enrichFilters = (rawFilters, matchAny, onEmptyFilter) => { let count = 1 return rawFilters .filter(filter => filter.field) @@ -84,6 +90,7 @@ field: `${count++}:${filter.field}`, })) .concat(matchAny ? [{ operator: "allOr" }] : []) + .concat([{ onEmptyFilter }]) } const addFilter = () => { @@ -195,6 +202,17 @@ on:change={e => (matchAny = e.detail === "or")} placeholder={null} /> + {#if datasource?.type === "table"} +