From 37aaf60aa5dbfd71c2a4810ed7822db37db5468b Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Jul 2025 14:18:02 +0200 Subject: [PATCH] feat(1-3873)/warn you when adding existing values (#10310) Makes it so that the constraint value input gives you an error if you try to add one or more values that **all** exist in the set of values already. E.g. if you have `a` and `b`, and try to add `a`, it'll tell you that "`a` has already been added". Likewise, if you try to add `a,b`, it'll tell you that all these values already exist. However, if at least one of the values does not exist, then it will allow you to submit the values (we already do deduplication before storing anyway). The background for this is that a user was confused thinking that just one specific value didn't get added to their constraints. As it turns out, they'd already added the value previously, so when it didn't show up at the end of the list, they thought it didn't work at all. image image --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../useEditableConstraint.test.tsx | 21 +++++++++++++++++++ .../useEditableConstraint.tsx | 20 +++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx index bdc671ea45..ff63dae6c8 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx @@ -154,6 +154,27 @@ describe('validators', () => { ]); }, ); + + test.each(multipleValueOperators)( + 'multi-value operator %s should reject fully duplicate inputs and accept new values', + (operator) => { + const initial: IConstraint = { + contextName: 'context-field', + operator: operator, + values: ['a', 'b'], + }; + + const { result } = renderHook(() => + useEditableConstraint(initial, () => {}), + ); + + checkValidator(result.current.validator, [ + ['a', false], + [['a', 'c'], true], + [['a', 'b'], false], + ]); + }, + ); }); describe('legal values', () => { diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx index 701f003458..91f9ce0212 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx @@ -4,6 +4,7 @@ import type { IConstraint } from 'interfaces/strategy'; import { type EditableConstraint, fromIConstraint, + isMultiValueConstraint, isSingleValueConstraint, toIConstraint, } from './editable-constraint-type.ts'; @@ -16,8 +17,8 @@ import { type ConstraintUpdateAction, } from './constraint-reducer.ts'; import { - type ConstraintValidationResult, constraintValidator, + type ConstraintValidationResult, } from './constraint-validator.ts'; import { getDeletedLegalValues, @@ -76,7 +77,20 @@ export const useEditableConstraint = ( [JSON.stringify(context), localConstraint.contextName], ); - const validator = constraintValidator(localConstraint.operator); + const baseValidator = constraintValidator(localConstraint.operator); + + const validator = (...values: string[]) => { + if ( + isMultiValueConstraint(localConstraint) && + values.every((value) => localConstraint.values.has(value)) + ) { + if (values.length === 1) { + return [false, `${values[0]} is already added.`]; + } + return [false, `All the values are already added`]; + } + return baseValidator(...values); + }; useEffect(() => { if ( @@ -104,7 +118,7 @@ export const useEditableConstraint = ( isSingleValueConstraint(localConstraint) ) { return getInvalidLegalValues( - (value) => validator(value)[0], + (value) => baseValidator(value)[0], contextDefinition.legalValues, ); }