Merge pull request #7049 from Budibase/fix/2585

Updating filters to allow multiple uses of the same property and exposing allOr option

packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte

@@ -9,23 +9,34 @@
     Input,
     Layout,
     Select,
+    Label,
   } from "@budibase/bbui"
   import DrawerBindableInput from "components/common/bindings/DrawerBindableInput.svelte"
   import ClientBindingPanel from "components/common/bindings/ClientBindingPanel.svelte"
   import { generate } from "shortid"
   import { LuceneUtils, Constants } from "@budibase/frontend-core"
   import { getFields } from "helpers/searchFields"
+  import { createEventDispatcher, onMount } from "svelte"
+
+  const dispatch = createEventDispatcher()
 
   export let schemaFields
   export let filters = []
   export let bindings = []
   export let panel = ClientBindingPanel
   export let allowBindings = true
+  export let allOr = false
 
+  $: dispatch("change", filters)
   $: enrichedSchemaFields = getFields(schemaFields || [])
   $: fieldOptions = enrichedSchemaFields.map(field => field.name) || []
   $: valueTypeOptions = allowBindings ? ["Value", "Binding"] : ["Value"]
 
+  let behaviourValue
+  const behaviourOptions = [
+    { value: "and", label: "Match all of the following filters" },
+    { value: "or", label: "Match any of the following filters" },
+  ]
   const addFilter = () => {
     filters = [
       ...filters,
@@ -69,7 +80,7 @@
     }
 
     // if changed to an array, change default value to empty array
-    const idx = filters.findIndex(x => x.field === field)
+    const idx = filters.findIndex(x => x.id === expression.id)
     if (expression.type === "array") {
       filters[idx].value = []
     } else {
@@ -92,6 +103,10 @@
     const schema = enrichedSchemaFields.find(x => x.name === field)
     return schema?.constraints?.inclusion || []
   }
+
+  onMount(() => {
+    behaviourValue = allOr ? "or" : "and"
+  })
 </script>
 
 <DrawerContent>
@@ -107,79 +122,95 @@
       </Body>
       {#if filters?.length}
         <div class="fields">
-          {#each filters as filter, idx}
-            <Select
-              bind:value={filter.field}
-              options={fieldOptions}
-              on:change={e => onFieldChange(filter, e.detail)}
-              placeholder="Column"
-            />
-            <Select
-              disabled={!filter.field}
-              options={LuceneUtils.getValidOperatorsForType(filter.type)}
-              bind:value={filter.operator}
-              on:change={e => onOperatorChange(filter, e.detail)}
-              placeholder={null}
-            />
-            <Select
-              disabled={filter.noValue || !filter.field}
-              options={valueTypeOptions}
-              bind:value={filter.valueType}
-              placeholder={null}
-            />
-            {#if filter.valueType === "Binding"}
-              <DrawerBindableInput
-                disabled={filter.noValue}
-                title={`Value for "${filter.field}"`}
-                value={filter.value}
-                placeholder="Value"
-                {panel}
-                {bindings}
-                on:change={event => (filter.value = event.detail)}
+          <Select
+            label="Behaviour"
+            value={behaviourValue}
+            options={behaviourOptions}
+            getOptionLabel={opt => opt.label}
+            getOptionValue={opt => opt.value}
+            on:change={e => (allOr = e.detail === "or")}
+            placeholder={null}
+          />
+        </div>
+        <div>
+          <div class="filter-label">
+            <Label>Filters</Label>
+          </div>
+          <div class="fields">
+            {#each filters as filter, idx}
+              <Select
+                bind:value={filter.field}
+                options={fieldOptions}
+                on:change={e => onFieldChange(filter, e.detail)}
+                placeholder="Column"
               />
-            {:else if ["string", "longform", "number", "formula"].includes(filter.type)}
-              <Input disabled={filter.noValue} bind:value={filter.value} />
-            {:else if ["options", "array"].includes(filter.type)}
-              <Combobox
-                disabled={filter.noValue}
-                options={getFieldOptions(filter.field)}
-                bind:value={filter.value}
+              <Select
+                disabled={!filter.field}
+                options={LuceneUtils.getValidOperatorsForType(filter.type)}
+                bind:value={filter.operator}
+                on:change={e => onOperatorChange(filter, e.detail)}
+                placeholder={null}
               />
-            {:else if filter.type === "boolean"}
-              <Combobox
-                disabled={filter.noValue}
-                options={[
-                  { label: "True", value: "true" },
-                  { label: "False", value: "false" },
-                ]}
-                bind:value={filter.value}
+              <Select
+                disabled={filter.noValue || !filter.field}
+                options={valueTypeOptions}
+                bind:value={filter.valueType}
+                placeholder={null}
               />
-            {:else if filter.type === "datetime"}
-              <DatePicker
-                disabled={filter.noValue}
-                enableTime={!getSchema(filter).dateOnly}
-                timeOnly={getSchema(filter).timeOnly}
-                bind:value={filter.value}
+              {#if filter.valueType === "Binding"}
+                <DrawerBindableInput
+                  disabled={filter.noValue}
+                  title={`Value for "${filter.field}"`}
+                  value={filter.value}
+                  placeholder="Value"
+                  {panel}
+                  {bindings}
+                  on:change={event => (filter.value = event.detail)}
+                />
+              {:else if ["string", "longform", "number", "formula"].includes(filter.type)}
+                <Input disabled={filter.noValue} bind:value={filter.value} />
+              {:else if ["options", "array"].includes(filter.type)}
+                <Combobox
+                  disabled={filter.noValue}
+                  options={getFieldOptions(filter.field)}
+                  bind:value={filter.value}
+                />
+              {:else if filter.type === "boolean"}
+                <Combobox
+                  disabled={filter.noValue}
+                  options={[
+                    { label: "True", value: "true" },
+                    { label: "False", value: "false" },
+                  ]}
+                  bind:value={filter.value}
+                />
+              {:else if filter.type === "datetime"}
+                <DatePicker
+                  disabled={filter.noValue}
+                  enableTime={!getSchema(filter).dateOnly}
+                  timeOnly={getSchema(filter).timeOnly}
+                  bind:value={filter.value}
+                />
+              {:else}
+                <DrawerBindableInput disabled />
+              {/if}
+              <Icon
+                name="Duplicate"
+                hoverable
+                size="S"
+                on:click={() => duplicateFilter(filter.id)}
               />
-            {:else}
-              <DrawerBindableInput disabled />
-            {/if}
-            <Icon
-              name="Duplicate"
-              hoverable
-              size="S"
-              on:click={() => duplicateFilter(filter.id)}
-            />
-            <Icon
-              name="Close"
-              hoverable
-              size="S"
-              on:click={() => removeFilter(filter.id)}
-            />
-          {/each}
+              <Icon
+                name="Close"
+                hoverable
+                size="S"
+                on:click={() => removeFilter(filter.id)}
+              />
+            {/each}
+          </div>
         </div>
       {/if}
-      <div>
+      <div class="bottom">
         <Button icon="AddCircle" size="M" secondary on:click={addFilter}>
           Add filter
         </Button>
@@ -202,4 +233,14 @@
     align-items: center;
     grid-template-columns: 1fr 120px 120px 1fr auto auto;
   }
+
+  .filter-label {
+    margin-bottom: var(--spacing-s);
+  }
+
+  .bottom {
+    display: flex;
+    justify-content: space-between;
+    align-items: center;
+  }
 </style>

packages/builder/src/components/design/settings/controls/FilterEditor/FilterEditor.svelte

@@ -8,21 +8,73 @@
   import FilterDrawer from "./FilterDrawer.svelte"
   import { currentAsset } from "builderStore"
 
+  const QUERY_START_REGEX = /\d[0-9]*:/g
   const dispatch = createEventDispatcher()
 
   export let value = []
   export let componentInstance
   export let bindings = []
 
-  let drawer
-  let tempValue = value || []
+  let drawer,
+    toSaveFilters = null,
+    allOr,
+    initialAllOr
 
+  $: initialFilters = correctFilters(value || [])
   $: dataSource = getDatasourceForProvider($currentAsset, componentInstance)
   $: schema = getSchemaForDatasource($currentAsset, dataSource)?.schema
   $: schemaFields = Object.values(schema || {})
 
-  const saveFilter = async () => {
-    dispatch("change", tempValue)
+  function addNumbering(filters) {
+    let count = 1
+    for (let value of filters) {
+      if (value.field && value.field?.match(QUERY_START_REGEX) == null) {
+        value.field = `${count++}:${value.field}`
+      }
+    }
+    return filters
+  }
+
+  function correctFilters(filters) {
+    const corrected = []
+    for (let filter of filters) {
+      let field = filter.field
+      if (filter.operator === "allOr") {
+        initialAllOr = allOr = true
+        continue
+      }
+      if (
+        typeof filter.field === "string" &&
+        filter.field.match(QUERY_START_REGEX) != null
+      ) {
+        const parts = field.split(":")
+        const number = parts[0]
+        // it's the new format, remove number
+        if (!isNaN(parseInt(number))) {
+          parts.shift()
+          field = parts.join(":")
+        }
+      }
+      corrected.push({
+        ...filter,
+        field,
+      })
+    }
+    return corrected
+  }
+
+  async function saveFilter() {
+    if (!toSaveFilters && allOr !== initialAllOr) {
+      toSaveFilters = initialFilters
+    }
+    const filters = toSaveFilters?.filter(filter => filter.operator !== "allOr")
+    if (allOr && filters) {
+      filters.push({ operator: "allOr" })
+    }
+    // only save if anything was updated
+    if (filters) {
+      dispatch("change", addNumbering(filters))
+    }
     notifications.success("Filters saved.")
     drawer.hide()
   }
@@ -33,8 +85,12 @@
   <Button cta slot="buttons" on:click={saveFilter}>Save</Button>
   <FilterDrawer
     slot="body"
-    bind:filters={tempValue}
+    filters={initialFilters}
     {bindings}
     {schemaFields}
+    bind:allOr
+    on:change={event => {
+      toSaveFilters = event.detail
+    }}
   />
 </Drawer>

packages/frontend-core/src/utils/lucene.js

@@ -103,6 +103,10 @@ export const buildLuceneQuery = filter => {
       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 (type === "datetime") {
         // Ensure date value is a valid date and parse into correct format
         if (!value) {

packages/server/__mocks__/node-fetch.ts

@@ -57,12 +57,19 @@ module FetchMock {
         404
       )
     } else if (url.includes("_search")) {
+      const body = opts.body
+      const parts = body.split("tableId:")
+      let tableId
+      if (parts && parts[1]) {
+        tableId = parts[1].split('"')[0]
+      }
       return json({
         rows: [
           {
             doc: {
               _id: "test",
-              tableId: opts.body.split("tableId:")[1].split('"')[0],
+              tableId: tableId,
+              query: opts.body,
             },
           },
         ],

packages/server/src/api/controllers/row/internalSearch.js

@@ -1,4 +1,5 @@
 const { SearchIndexes } = require("../../../db/utils")
+const { removeKeyNumbering } = require("./utils")
 const fetch = require("node-fetch")
 const { getCouchInfo } = require("@budibase/backend-core/db")
 const { getAppId } = require("@budibase/backend-core/context")
@@ -197,6 +198,8 @@ class QueryBuilder {
 
     function build(structure, queryFn) {
       for (let [key, value] of Object.entries(structure)) {
+        // check for new format - remove numbering if needed
+        key = removeKeyNumbering(key)
         key = builder.preprocess(key.replace(/ /g, "_"), {
           escape: true,
         })
@@ -310,6 +313,9 @@ class QueryBuilder {
   }
 }
 
+// exported for unit testing
+exports.QueryBuilder = QueryBuilder
+
 /**
  * Executes a lucene search query.
  * @param url The query URL

packages/server/src/api/controllers/row/utils.js

@@ -3,8 +3,9 @@ const { cloneDeep } = require("lodash/fp")
 const { InternalTables } = require("../../../db/utils")
 const userController = require("../user")
 const { FieldTypes } = require("../../../constants")
-const { makeExternalQuery } = require("../../../integrations/base/utils")
 const { getAppDB } = require("@budibase/backend-core/context")
+const { makeExternalQuery } = require("../../../integrations/base/query")
+const { removeKeyNumbering } = require("../../../integrations/base/utils")
 
 validateJs.extend(validateJs.validators.datetime, {
   parse: function (value) {
@@ -16,6 +17,8 @@ validateJs.extend(validateJs.validators.datetime, {
   },
 })
 
+exports.removeKeyNumbering = removeKeyNumbering
+
 exports.getDatasourceAndQuery = async json => {
   const datasourceId = json.endpoint.datasourceId
   const db = getAppDB()

packages/server/src/api/controllers/table/external.js

@@ -14,7 +14,7 @@ const {
   FieldTypes,
   RelationshipTypes,
 } = require("../../../constants")
-const { makeExternalQuery } = require("../../../integrations/base/utils")
+const { makeExternalQuery } = require("../../../integrations/base/query")
 const { cloneDeep } = require("lodash/fp")
 const csvParser = require("../../../utilities/csvParser")
 const { handleRequest } = require("../row/external")

packages/server/src/api/routes/tests/internalSearch.spec.js

@@ -0,0 +1,157 @@
+const search = require("../../controllers/row/internalSearch")
+// this will be mocked out for _search endpoint
+const fetch = require("node-fetch")
+const PARAMS = {
+  tableId: "ta_12345679abcdef",
+  version: "1",
+  bookmark: null,
+  sort: null,
+  sortOrder: "ascending",
+  sortType: "string",
+}
+
+function checkLucene(resp, expected, params = PARAMS) {
+  const query = resp.rows[0].query
+  const json = JSON.parse(query)
+  if (PARAMS.sort) {
+    expect(json.sort).toBe(`${PARAMS.sort}<${PARAMS.sortType}>`)
+  }
+  if (PARAMS.bookmark) {
+    expect(json.bookmark).toBe(PARAMS.bookmark)
+  }
+  expect(json.include_docs).toBe(true)
+  expect(json.q).toBe(`(${expected}) AND tableId:"${params.tableId}"`)
+  expect(json.limit).toBe(params.limit || 50)
+}
+
+describe("internal search", () => {
+  it("default query", async () => {
+    const response = await search.paginatedSearch({
+    }, PARAMS)
+    checkLucene(response, `*:*`)
+  })
+
+  it("test equal query", async () => {
+    const response = await search.paginatedSearch({
+      equal: {
+        "column": "1",
+      }
+    }, PARAMS)
+    checkLucene(response, `*:* AND column:"1"`)
+  })
+
+  it("test notEqual query", async () => {
+    const response = await search.paginatedSearch({
+      notEqual: {
+        "column": "1",
+      }
+    }, PARAMS)
+    checkLucene(response, `*:* AND !column:"1"`)
+  })
+
+  it("test OR query", async () => {
+    const response = await search.paginatedSearch({
+      allOr: true,
+      equal: {
+        "column": "2",
+      },
+      notEqual: {
+        "column": "1",
+      }
+    }, PARAMS)
+    checkLucene(response, `column:"2" OR !column:"1"`)
+  })
+
+  it("test AND query", async () => {
+    const response = await search.paginatedSearch({
+      equal: {
+        "column": "2",
+      },
+      notEqual: {
+        "column": "1",
+      }
+    }, PARAMS)
+    checkLucene(response, `*:* AND column:"2" AND !column:"1"`)
+  })
+
+  it("test pagination query", async () => {
+    const updatedParams = {
+      ...PARAMS,
+      limit: 100,
+      bookmark: "awd",
+      sort: "column",
+    }
+    const response = await search.paginatedSearch({
+      string: {
+        "column": "2",
+      },
+    }, updatedParams)
+    checkLucene(response, `*:* AND column:2*`, updatedParams)
+  })
+
+  it("test range query", async () => {
+    const response = await search.paginatedSearch({
+      range: {
+        "column": { low: 1, high: 2 },
+      },
+    }, PARAMS)
+    checkLucene(response, `*:* AND column:[1 TO 2]`, PARAMS)
+  })
+
+  it("test empty query", async () => {
+    const response = await search.paginatedSearch({
+      empty: {
+        "column": "",
+      },
+    }, PARAMS)
+    checkLucene(response, `*:* AND !column:["" TO *]`, PARAMS)
+  })
+
+  it("test notEmpty query", async () => {
+    const response = await search.paginatedSearch({
+      notEmpty: {
+        "column": "",
+      },
+    }, PARAMS)
+    checkLucene(response, `*:* AND column:["" TO *]`, PARAMS)
+  })
+
+  it("test oneOf query", async () => {
+    const response = await search.paginatedSearch({
+      oneOf: {
+        "column": ["a", "b"],
+      },
+    }, PARAMS)
+    checkLucene(response, `*:* AND column:("a" OR "b")`, PARAMS)
+  })
+
+  it("test contains query", async () => {
+    const response = await search.paginatedSearch({
+      contains: {
+        "column": "a",
+      },
+    }, PARAMS)
+    checkLucene(response, `*:* AND column:a`, PARAMS)
+  })
+
+  it("test multiple of same column", async () => {
+    const response = await search.paginatedSearch({
+      allOr: true,
+      equal: {
+        "1:column": "a",
+        "2:column": "b",
+        "3:column": "c",
+      },
+    }, PARAMS)
+    checkLucene(response, `column:"a" OR column:"b" OR column:"c"`, PARAMS)
+  })
+
+  it("check a weird case for lucene building", async () => {
+    const response = await search.paginatedSearch({
+      equal: {
+        "1:1:column": "a",
+      },
+    }, PARAMS)
+    checkLucene(response, `*:* AND 1\\:column:"a"`, PARAMS)
+  })
+})

packages/server/src/integrations/base/query.ts

@@ -0,0 +1,17 @@
+import { QueryJson } from "../../definitions/datasource"
+import { Datasource } from "../../definitions/common"
+const { integrations } = require("../index")
+
+export async function makeExternalQuery(
+  datasource: Datasource,
+  json: QueryJson
+) {
+  const Integration = integrations[datasource.source]
+  // query is the opinionated function
+  if (Integration.prototype.query) {
+    const integration = new Integration(datasource.config)
+    return integration.query(json)
+  } else {
+    throw "Datasource does not support query."
+  }
+}

packages/server/src/integrations/base/sql.ts

@@ -10,6 +10,7 @@ import {
 import { isIsoDateString, SqlClients } from "../utils"
 import SqlTableQueryBuilder from "./sqlTable"
 import environment from "../../environment"
+import { removeKeyNumbering } from "./utils"
 
 const envLimit = environment.SQL_MAX_ROWS
   ? parseInt(environment.SQL_MAX_ROWS)
@@ -133,12 +134,13 @@ class InternalBuilder {
       fn: (key: string, value: any) => void
     ) {
       for (let [key, value] of Object.entries(structure)) {
-        const isRelationshipField = key.includes(".")
+        const updatedKey = removeKeyNumbering(key)
+        const isRelationshipField = updatedKey.includes(".")
         if (!opts.relationship && !isRelationshipField) {
-          fn(`${opts.tableName}.${key}`, value)
+          fn(`${opts.tableName}.${updatedKey}`, value)
         }
         if (opts.relationship && isRelationshipField) {
-          fn(key, value)
+          fn(updatedKey, value)
         }
       }
     }
@@ -582,4 +584,3 @@ class SqlQueryBuilder extends SqlTableQueryBuilder {
 }
 
 export default SqlQueryBuilder
-module.exports = SqlQueryBuilder

packages/server/src/integrations/base/utils.ts

@@ -1,22 +1,12 @@
-import { QueryJson } from "../../definitions/datasource"
-import { Datasource } from "../../definitions/common"
+const QUERY_START_REGEX = /\d[0-9]*:/g
 
-module DatasourceUtils {
-  const { integrations } = require("../index")
-
-  export async function makeExternalQuery(
-    datasource: Datasource,
-    json: QueryJson
-  ) {
-    const Integration = integrations[datasource.source]
-    // query is the opinionated function
-    if (Integration.prototype.query) {
-      const integration = new Integration(datasource.config)
-      return integration.query(json)
-    } else {
-      throw "Datasource does not support query."
-    }
+export function removeKeyNumbering(key: any): string {
+  if (typeof key === "string" && key.match(QUERY_START_REGEX) != null) {
+    const parts = key.split(":")
+    // remove the number
+    parts.shift()
+    return parts.join(":")
+  } else {
+    return key
   }
-
-  module.exports.makeExternalQuery = makeExternalQuery
 }

packages/server/src/integrations/microsoftSqlServer.ts

@@ -15,10 +15,10 @@ import {
 } from "./utils"
 import { DatasourcePlus } from "./base/datasourcePlus"
 import { Table, TableSchema } from "../definitions/common"
+import Sql from "./base/sql"
 
 module MSSQLModule {
   const sqlServer = require("mssql")
-  const Sql = require("./base/sql")
   const DEFAULT_SCHEMA = "dbo"
 
   interface MSSQLConfig {
@@ -96,7 +96,8 @@ module MSSQLModule {
   class SqlServerIntegration extends Sql implements DatasourcePlus {
     private readonly config: MSSQLConfig
     private index: number = 0
-    static pool: any
+    private readonly pool: any
+    private client: any
     public tables: Record<string, Table> = {}
     public schemaErrors: Record<string, string> = {}
 

packages/server/src/integrations/mysql.ts

@@ -16,10 +16,10 @@ import {
 import { DatasourcePlus } from "./base/datasourcePlus"
 import dayjs from "dayjs"
 const { NUMBER_REGEX } = require("../utilities")
+import Sql from "./base/sql"
 
 module MySQLModule {
   const mysql = require("mysql2/promise")
-  const Sql = require("./base/sql")
 
   interface MySQLConfig {
     host: string

packages/server/src/integrations/postgres.ts

@@ -14,10 +14,10 @@ import {
   SqlClients,
 } from "./utils"
 import { DatasourcePlus } from "./base/datasourcePlus"
+import Sql from "./base/sql"
 
 module PostgresModule {
   const { Client, types } = require("pg")
-  const Sql = require("./base/sql")
   const { escapeDangerousCharacters } = require("../utilities")
 
   // Return "date" and "timestamp" types as plain strings.
@@ -117,6 +117,7 @@ module PostgresModule {
     private readonly client: any
     private readonly config: PostgresConfig
     private index: number = 1
+    private open: boolean
     public tables: Record<string, Table> = {}
     public schemaErrors: Record<string, string> = {}
 

packages/server/src/integrations/queries/sql.ts

@@ -1,5 +1,4 @@
 import { findHBSBlocks, processStringSync } from "@budibase/string-templates"
-import { Integration } from "../../definitions/datasource"
 import { DatasourcePlus } from "../base/datasourcePlus"
 
 const CONST_CHAR_REGEX = new RegExp("'[^']*'", "g")

packages/server/src/integrations/tests/sql.spec.js

@@ -1,4 +1,4 @@
-const Sql = require("../base/sql")
+const Sql = require("../base/sql").default
 const { SqlClients } = require("../utils")
 
 const TABLE_NAME = "test"