From 6875554799d37ecd859ec26046d5a16ed008afdd Mon Sep 17 00:00:00 2001 From: Mel O'Hagan Date: Wed, 21 Sep 2022 15:25:53 +0100 Subject: [PATCH 1/5] Only call startsWith if string --- .../builder/src/components/common/bindings/BindingPanel.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/components/common/bindings/BindingPanel.svelte b/packages/builder/src/components/common/bindings/BindingPanel.svelte index ffa0e98819..e568b73472 100644 --- a/packages/builder/src/components/common/bindings/BindingPanel.svelte +++ b/packages/builder/src/components/common/bindings/BindingPanel.svelte @@ -43,7 +43,7 @@ let helpers = handlebarsCompletions() let getCaretPosition let search = "" - let initialValueJS = value?.startsWith("{{ js ") + let initialValueJS = typeof value === "string" && value?.startsWith("{{ js ") let mode = initialValueJS ? "JavaScript" : "Handlebars" let jsValue = initialValueJS ? value : null let hbsValue = initialValueJS ? null : value From cf5dbfe055d77e8ebdbc6a23f51636215adc108b Mon Sep 17 00:00:00 2001 From: Mel O'Hagan Date: Thu, 22 Sep 2022 09:40:45 +0100 Subject: [PATCH 2/5] Handle valueType change --- .../controls/FilterEditor/FilterDrawer.svelte | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) 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 9bd66a4cb1..553db99fc5 100644 --- a/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte +++ b/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte @@ -100,13 +100,37 @@ } if ( operator === Constants.OperatorOptions.In.value && - !Array.isArray(expression.value) + !Array.isArray(expression.value) && + expression.valueType === "Value" ) { if (expression.value) { expression.value = [expression.value] } else { expression.value = [] } + } else if ( + operator !== Constants.OperatorOptions.In.value && + Array.isArray(expression.value) + ) { + expression.value = null + } + } + + const onValueTypeChange = (expression, valueType) => { + if (Array.isArray(expression.value) && valueType === "Binding") { + expression.value = null + } else if ( + expression.operator === Constants.OperatorOptions.In.value && + !Array.isArray(expression.value) && + valueType === "Value" + ) { + if (typeof expression.value === "string") { + expression.value = expression.value.split(",") + } else if (expression.value) { + expression.value = [expression.value] + } else { + expression.value = [] + } } } @@ -167,6 +191,7 @@ options={valueTypeOptions} bind:value={filter.valueType} placeholder={null} + on:change={e => onValueTypeChange(filter, e.detail)} /> {#if filter.valueType === "Binding"} Date: Wed, 28 Sep 2022 15:08:14 +0100 Subject: [PATCH 3/5] Update casing in 'does not contain' filter operator --- packages/frontend-core/src/constants.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/frontend-core/src/constants.js b/packages/frontend-core/src/constants.js index 633534dddb..eb7a8849a5 100644 --- a/packages/frontend-core/src/constants.js +++ b/packages/frontend-core/src/constants.js @@ -40,7 +40,7 @@ export const OperatorOptions = { }, NotContains: { value: "notContains", - label: "Does Not Contain", + label: "Does not contain", }, In: { value: "oneOf", From 9e1f19e5e18089d9786788bc443ab0aef7ce94d9 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 28 Sep 2022 15:08:44 +0100 Subject: [PATCH 4/5] Simplify, strengthen and make consistent filter drawer validation logic --- .../controls/FilterEditor/FilterDrawer.svelte | 117 ++++++++---------- 1 file changed, 53 insertions(+), 64 deletions(-) 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 000ae346a9..bbe81d866d 100644 --- a/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte +++ b/packages/builder/src/components/design/settings/controls/FilterEditor/FilterDrawer.svelte @@ -20,6 +20,8 @@ import { createEventDispatcher, onMount } from "svelte" const dispatch = createEventDispatcher() + const { OperatorOptions } = Constants + const { getValidOperatorsForType } = LuceneUtils export let schemaFields export let filters = [] @@ -45,7 +47,7 @@ { id: generate(), field: null, - operator: Constants.OperatorOptions.Equals.value, + operator: OperatorOptions.Equals.value, value: null, valueType: "Value", }, @@ -66,73 +68,60 @@ return schemaFields.find(field => field.name === filter.field) } - const onFieldChange = (expression, field) => { - // Update the field types - expression.type = enrichedSchemaFields.find(x => x.name === field)?.type - expression.externalType = getSchema(expression)?.externalType + const validateTypes = filter => { + // Update type based on field + const fieldSchema = enrichedSchemaFields.find(x => x.name === filter.field) + filter.type = fieldSchema?.type - // Ensure a valid operator is set - const validOperators = LuceneUtils.getValidOperatorsForType( - expression.type - ).map(x => x.value) - if (!validOperators.includes(expression.operator)) { - expression.operator = - validOperators[0] ?? Constants.OperatorOptions.Equals.value - onOperatorChange(expression, expression.operator) - } - - // if changed to an array, change default value to empty array - const idx = filters.findIndex(x => x.id === expression.id) - if (expression.type === "array") { - filters[idx].value = [] - } else { - filters[idx].value = null - } + // Update external type based on field + filter.externalType = getSchema(filter)?.externalType } - const onOperatorChange = (expression, operator) => { + const validateOperator = filter => { + // Ensure a valid operator is selected + const operators = getValidOperatorsForType(filter.type).map(x => x.value) + if (!operators.includes(filter.operator)) { + filter.operator = operators[0] ?? OperatorOptions.Equals.value + } + + // Update the noValue flag if the operator does not take a value const noValueOptions = [ - Constants.OperatorOptions.Empty.value, - Constants.OperatorOptions.NotEmpty.value, + OperatorOptions.Empty.value, + OperatorOptions.NotEmpty.value, ] - expression.noValue = noValueOptions.includes(operator) - if (expression.noValue) { - expression.value = null + filter.noValue = noValueOptions.includes(filter.operator) + } + + const validateValue = filter => { + // Check if the operator allows a value at all + if (filter.noValue) { + filter.value = null + return } - if ( - operator === Constants.OperatorOptions.In.value && - !Array.isArray(expression.value) && - expression.valueType === "Value" - ) { - if (expression.value) { - expression.value = [expression.value] - } else { - expression.value = [] + + // Ensure array values are properly set and cleared + if (Array.isArray(filter.value)) { + if (filter.valueType !== "Value" || filter.type !== "array") { + filter.value = null } - } else if ( - operator !== Constants.OperatorOptions.In.value && - Array.isArray(expression.value) - ) { - expression.value = null + } else if (filter.type === "array" && filter.valueType === "Value") { + filter.value = [] } } - const onValueTypeChange = (expression, valueType) => { - if (Array.isArray(expression.value) && valueType === "Binding") { - expression.value = null - } else if ( - expression.operator === Constants.OperatorOptions.In.value && - !Array.isArray(expression.value) && - valueType === "Value" - ) { - if (typeof expression.value === "string") { - expression.value = expression.value.split(",") - } else if (expression.value) { - expression.value = [expression.value] - } else { - expression.value = [] - } - } + const onFieldChange = filter => { + validateTypes(filter) + validateOperator(filter) + validateValue(filter) + } + + const onOperatorChange = filter => { + validateOperator(filter) + validateValue(filter) + } + + const onValueTypeChange = filter => { + validateValue(filter) } const getFieldOptions = field => { @@ -177,24 +166,24 @@ onOperatorChange(filter, e.detail)} + on:change={() => onOperatorChange(filter)} placeholder={null} />