PR feedback

packages/builder/src/stores/builder/components.js

@@ -206,9 +206,15 @@ export class ComponentStore extends BudiStore {
       setting?.type?.startsWith("filter")
     )
     for (let setting of filterableTypes || []) {
-      enrichedComponent[setting.key] = utils.processSearchFilters(
-        enrichedComponent[setting.key]
-      )
+      const isLegacy = Array.isArray(enrichedComponent[setting.key])
+
+      if (isLegacy) {
+        const processedSetting = utils.processSearchFilters(
+          enrichedComponent[setting.key]
+        )
+        enrichedComponent[setting.key] = processedSetting
+        migrated = true
+      }
     }
     return migrated
   }
@@ -575,9 +581,7 @@ export class ComponentStore extends BudiStore {
       const patchResult = patchFn(component, screen)
 
       // Post processing
-      const migrated = null
-
-      this.migrateSettings(component)
+      const migrated = this.migrateSettings(component)
 
       // Returning an explicit false signifies that we should skip this
       // update. If we migrated something, ensure we never skip.

packages/client/src/components/app/DataProvider.svelte

@@ -19,9 +19,7 @@
   let queryExtensions = {}
   let localFilters
 
-  $: if (filter) {
-    localFilters = Helpers.cloneDeep(filter)
-  }
+  $: localFilters = filter ? Helpers.cloneDeep(filter) : null
 
   $: defaultQuery = QueryUtils.buildQuery(localFilters)
 
@@ -131,7 +129,7 @@
   }
 
   const extendQuery = (defaultQuery, extensions) => {
-    return {
+    const extended = {
       [LogicalOperator.AND]: {
         conditions: [
           ...(defaultQuery ? [defaultQuery] : []),
@@ -140,6 +138,11 @@
       },
       onEmptyFilter: EmptyFilterOption.RETURN_NONE,
     }
+
+    // If there are no conditions applied at all, clear the request.
+    return extended[LogicalOperator.AND]?.conditions?.length > 0
+      ? extended
+      : null
   }
 
   const setUpAutoRefresh = autoRefresh => {

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

@@ -58,8 +58,13 @@
     return { value: entry, label: Helpers.capitalise(entry) }
   })
 
+  const onEmptyLabelling = {
+    [OnEmptyFilter.RETURN_ALL]: "All rows",
+    [OnEmptyFilter.RETURN_NONE]: "No rows",
+  }
+
   const onEmptyOptions = Object.values(OnEmptyFilter).map(entry => {
-    return { value: entry, label: Helpers.capitalise(entry) }
+    return { value: entry, label: onEmptyLabelling[entry] }
   })
 
   const context = getContext("context")
@@ -151,7 +156,7 @@
 
   const getGroupPrefix = groupIdx => {
     if (groupIdx == 0) {
-      return "Where"
+      return "When"
     }
     const operatorMapping = {
       [FilterOperator.ANY]: "or",
@@ -191,16 +196,17 @@
     if (targetFilter) {
       if (deleteFilter) {
         targetGroup.filters.splice(filterIdx, 1)
+
+        // Clear the group entirely if no valid filters remain
+        if (targetGroup.filters.length === 0) {
+          editable.groups.splice(groupIdx, 1)
+        }
       } else if (filter) {
         targetGroup.filters[filterIdx] = filter
       }
     } else if (targetGroup) {
       if (deleteGroup) {
-        if (editable.groups.length > 1) {
-          editable.groups.splice(groupIdx, 1)
-        } else {
-          editable = {}
-        }
+        editable.groups.splice(groupIdx, 1)
       } else if (addFilter) {
         targetGroup.filters.push({
           valueType: FilterValueType.VALUE,
@@ -239,6 +245,9 @@
       }
     }
 
+    // Set the request to null if the groups are emptied
+    editable = editable.groups.length ? editable : null
+
     dispatch("change", editable)
   }
 </script>
@@ -294,7 +303,7 @@
                       placeholder={false}
                     />
                   </span>
-                  <span>of the following filters are met:</span>
+                  <span>of the following filters are matched:</span>
                 </div>
                 <div class="group-actions">
                   <Icon
@@ -386,49 +395,51 @@
       {/if}
 
       <div class="filters-footer">
-        {#if behaviourFilters && editableFilters?.groups?.length}
-          <div class="empty-filter">
-            <span>Return</span>
-            <span class="empty-filter-picker">
-              <Select
-                value={editableFilters?.onEmptyFilter}
-                options={onEmptyOptions}
-                getOptionLabel={opt => opt.label}
-                getOptionValue={opt => opt.value}
-                on:change={e => {
-                  handleFilterChange({
-                    onEmptyFilter: e.detail,
-                  })
-                }}
-                placeholder={false}
+        <Layout noPadding>
+          {#if behaviourFilters && editableFilters?.groups?.length}
+            <div class="empty-filter">
+              <span>Return</span>
+              <span class="empty-filter-picker">
+                <Select
+                  value={editableFilters?.onEmptyFilter}
+                  options={onEmptyOptions}
+                  getOptionLabel={opt => opt.label}
+                  getOptionValue={opt => opt.value}
+                  on:change={e => {
+                    handleFilterChange({
+                      onEmptyFilter: e.detail,
+                    })
+                  }}
+                  placeholder={false}
+                />
+              </span>
+              <span>when all filters are empty</span>
+            </div>
+          {/if}
+          <div class="add-group">
+            <Button
+              icon="AddCircle"
+              size="M"
+              secondary
+              on:click={() => {
+                handleFilterChange({
+                  addGroup: true,
+                })
+              }}
+            >
+              Add filter group
+            </Button>
+            <a
+              href="https://docs.budibase.com/docs/searchfilter-data"
+              target="_blank"
+            >
+              <Icon
+                name="HelpOutline"
+                color="var(--spectrum-global-color-gray-600)"
               />
-            </span>
-            <span>when all filters are empty</span>
+            </a>
           </div>
-        {/if}
-        <div class="add-group">
-          <Button
-            icon="AddCircle"
-            size="M"
-            secondary
-            on:click={() => {
-              handleFilterChange({
-                addGroup: true,
-              })
-            }}
-          >
-            Add filter group
-          </Button>
-          <a
-            href="https://docs.budibase.com/docs/searchfilter-data"
-            target="_blank"
-          >
-            <Icon
-              name="HelpOutline"
-              color="var(--spectrum-global-color-gray-600)"
-            />
-          </a>
-        </div>
+        </Layout>
       </div>
     {:else}
       <Body size="S">None of the table column can be used for filtering.</Body>
@@ -446,7 +457,7 @@
   .empty-filter,
   .group-options {
     display: flex;
-    gap: var(--spacing-xl);
+    gap: var(--spacing-m);
     align-items: center;
   }
 
@@ -461,14 +472,13 @@
   }
 
   .empty-filter-picker {
-    width: 80px;
+    width: 92px;
   }
 
   .filter-groups {
     display: flex;
     flex-direction: column;
     gap: var(--spacing-xl);
-    /* overflow-x: scroll; */
   }
 
   .group {
@@ -489,7 +499,7 @@
   .filter {
     display: grid;
     gap: var(--spacing-l);
-    grid-template-columns: minmax(150px, 1fr) 170px 200px 40px 40px;
+    grid-template-columns: minmax(150px, 1fr) 170px minmax(200px, 1fr) 40px 40px;
   }
 
   .filters-footer {

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

@@ -20,12 +20,9 @@
   export let bindings = []
   export let allowBindings = false
   export let schemaFields
-
-  // Export these to another field type
   export let panel
   export let toReadable
   export let toRuntime
-  // Only required if you're in the builder, which we aint.
 
   const dispatch = createEventDispatcher()
   const { OperatorOptions, FilterValueType } = Constants
@@ -48,17 +45,21 @@
     return schemaFields.find(field => field.name === filter.field)
   }
 
+  const drawerOnChange = e => {
+    drawerValue = e.detail
+  }
+
   const onChange = e => {
     fieldValue = e.detail
     dispatch("change", {
-      value: toRuntime(bindings, fieldValue),
+      value: toRuntime ? toRuntime(bindings, fieldValue) : fieldValue,
     })
   }
 
-  const onConfirm = () => {
+  const onConfirmBinding = () => {
     dispatch("change", {
-      value: fieldValue,
-      valueType: fieldValue ? FilterValueType.BINDING : FilterValueType.VALUE,
+      value: toRuntime ? toRuntime(bindings, drawerValue) : drawerValue,
+      valueType: drawerValue ? FilterValueType.BINDING : FilterValueType.VALUE,
     })
   }
 
@@ -139,7 +140,7 @@
       cta
       slot="buttons"
       on:click={() => {
-        onConfirm()
+        onConfirmBinding()
         bindingDrawer.hide()
       }}
     >
@@ -153,7 +154,7 @@
       allowJS
       allowHelpers
       allowHBS
-      on:change={onChange}
+      on:change={drawerOnChange}
       {bindings}
     />
   </Drawer>

packages/frontend-core/src/fetch/CustomFetch.js

@@ -1,14 +1,6 @@
 import DataFetch from "./DataFetch.js"
 
 export default class CustomFetch extends DataFetch {
-  determineFeatureFlags() {
-    return {
-      supportsSearch: false,
-      supportsSort: false,
-      supportsPagination: false,
-    }
-  }
-
   // Gets the correct Budibase type for a JS value
   getType(value) {
     if (value == null) {

packages/frontend-core/src/fetch/DataFetch.js

@@ -183,7 +183,7 @@ export default class DataFetch {
     }
 
     // Build the query
-    let query = this.options.query || null
+    let query = this.options.query
 
     if (!query && this.features.supportsSearch) {
       query = buildQuery(filter || defaultQuery)

packages/shared-core/src/filters.ts

@@ -307,11 +307,11 @@ export class ColumnSplitter {
 }
 
 /**
- * Builds a JSON query from the filter structure generated in the builder
+ * Builds a JSON query from the filter a SearchFilter definition
  * @param filter the builder filter structure
  */
 
-const builderFilter = (expression: SearchFilter) => {
+const buildCondition = (expression: SearchFilter) => {
   // Filter body
   let query: SearchFilters = {
     string: {},
@@ -378,13 +378,15 @@ const builderFilter = (expression: SearchFilter) => {
     value = `${value}`?.toLowerCase() === "true"
   }
   if (
-    ["contains", "notContains", "containsAny"].includes(operator) &&
+    ["contains", "notContains", "containsAny"].includes(
+      operator.toLocaleString()
+    ) &&
     type === "array" &&
     typeof value === "string"
   ) {
     value = value.split(",")
   }
-  if (operator.startsWith("range") && query.range) {
+  if (operator.toLocaleString().startsWith("range") && query.range) {
     const minint =
       SqlNumberTypeRangeMap[externalType as keyof typeof SqlNumberTypeRangeMap]
         ?.min || Number.MIN_SAFE_INTEGER
@@ -497,13 +499,15 @@ export const buildQueryLegacy = (
       value = `${value}`?.toLowerCase() === "true"
     }
     if (
-      ["contains", "notContains", "containsAny"].includes(operator) &&
+      ["contains", "notContains", "containsAny"].includes(
+        operator.toLocaleString()
+      ) &&
       type === "array" &&
       typeof value === "string"
     ) {
       value = value.split(",")
     }
-    if (operator.startsWith("range") && query.range) {
+    if (operator.toLocaleString().startsWith("range") && query.range) {
       const minint =
         SqlNumberTypeRangeMap[
           externalType as keyof typeof SqlNumberTypeRangeMap
@@ -555,28 +559,40 @@ export const buildQueryLegacy = (
   return query
 }
 
+/**
+ * Converts a **SearchFilterGroup** 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 | SearchFilter[]
 ): SearchFilters | undefined => {
-  if (!filter) {
-    return
-  }
-
-  const parsedFilter = processSearchFilters(filter)
+  const parsedFilter: SearchFilterGroup | undefined =
+    processSearchFilters(filter)
 
   if (!parsedFilter) {
     return
   }
 
-  const operatorMap = {
-    [FilterGroupLogicalOperator.ALL]: LogicalOperator.AND,
-    [FilterGroupLogicalOperator.ANY]: LogicalOperator.OR,
-  }
+  const operatorMap: { [key in FilterGroupLogicalOperator]: LogicalOperator } =
+    {
+      [FilterGroupLogicalOperator.ALL]: LogicalOperator.AND,
+      [FilterGroupLogicalOperator.ANY]: LogicalOperator.OR,
+    }
 
   const globalOnEmpty = parsedFilter.onEmptyFilter
     ? parsedFilter.onEmptyFilter
     : null
-  const globalOperator = operatorMap[parsedFilter.logicalOperator]
+
+  const globalOperator: LogicalOperator =
+    operatorMap[parsedFilter.logicalOperator as FilterGroupLogicalOperator]
 
   const coreRequest: SearchFilters = {
     ...(globalOnEmpty ? { onEmptyFilter: globalOnEmpty } : {}),
@@ -585,7 +601,7 @@ export const buildQuery = (
         return {
           [operatorMap[group.logicalOperator]]: {
             conditions: group.filters
-              ?.map(x => builderFilter(x))
+              ?.map(x => buildCondition(x))
               .filter(filter => filter),
           },
         }

packages/shared-core/src/utils.ts

@@ -97,8 +97,8 @@ export function trimOtherProps(object: any, allowedProps: string[]) {
  * @param {SearchFilter[] | SearchFilterGroup} filters
  */
 export const processSearchFilters = (
-  filters: SearchFilter[] | SearchFilterGroup
-) => {
+  filters: SearchFilter[] | SearchFilterGroup | undefined
+): SearchFilterGroup | undefined => {
   if (!filters) {
     return
   }
@@ -182,7 +182,7 @@ export const processSearchFilters = (
 
     return migratedSetting
   } else if (!filters?.groups) {
-    return null
+    return
   }
   return filters
 }