From 7a5e6cfa3b72796322393f8e09920bbadf720faf Mon Sep 17 00:00:00 2001 From: Mumbi Francis Date: Thu, 13 Jun 2024 16:42:31 +0300 Subject: [PATCH] Fix module deduplication translations and handling of multiple field errors --- .../src/components/ModuleFields/errors.vue | 6 +- .../corteza-server/compose/record-field.yaml | 3 +- server/compose/service/record.go | 60 ++++----- .../compose/types/record_detect_duplicates.go | 117 ++++++++++-------- .../types/record_detect_duplicates_test.go | 14 ++- 5 files changed, 111 insertions(+), 89 deletions(-) diff --git a/client/web/compose/src/components/ModuleFields/errors.vue b/client/web/compose/src/components/ModuleFields/errors.vue index aa46fbb3c..2a6b92c2a 100644 --- a/client/web/compose/src/components/ModuleFields/errors.vue +++ b/client/web/compose/src/components/ModuleFields/errors.vue @@ -9,7 +9,7 @@ - {{ error.message }} + {{ $t(error.message, { interpolation: { escapeValue: false }, value: error.meta.value}) }} @@ -18,6 +18,10 @@ import { validator } from '@cortezaproject/corteza-js' export default { + i18nOptions: { + namespaces: 'field', + }, + props: { errors: { type: validator.Validated, diff --git a/locale/en/corteza-server/compose/record-field.yaml b/locale/en/corteza-server/compose/record-field.yaml index 40c0af9f3..421eaa53b 100644 --- a/locale/en/corteza-server/compose/record-field.yaml +++ b/locale/en/corteza-server/compose/record-field.yaml @@ -2,4 +2,5 @@ errors: empty: This field is required invalidValue: Invalid field value invalidRef: Invalid field reference - duplicateValueInSet: This value already exists in list \ No newline at end of file + duplicateValueInSet: The values [{{value}}] already exist in another record" + duplicateValue: The value "{{value}}" already exists in another record \ No newline at end of file diff --git a/server/compose/service/record.go b/server/compose/service/record.go index 0ebca6c9c..eff5684c1 100644 --- a/server/compose/service/record.go +++ b/server/compose/service/record.go @@ -1,35 +1,35 @@ package service import ( - "context" - "encoding/json" - "fmt" - "regexp" - "sort" - "strconv" - "strings" - "time" + "context" + "encoding/json" + "fmt" + "regexp" + "sort" + "strconv" + "strings" + "time" - "github.com/cortezaproject/corteza/server/pkg/envoyx" - "github.com/cortezaproject/corteza/server/pkg/filter" - "github.com/cortezaproject/corteza/server/pkg/revisions" - "github.com/spf13/cast" + "github.com/cortezaproject/corteza/server/pkg/envoyx" + "github.com/cortezaproject/corteza/server/pkg/filter" + "github.com/cortezaproject/corteza/server/pkg/revisions" + "github.com/spf13/cast" - "github.com/cortezaproject/corteza/server/pkg/dal" - "github.com/cortezaproject/corteza/server/pkg/locale" + "github.com/cortezaproject/corteza/server/pkg/dal" + "github.com/cortezaproject/corteza/server/pkg/locale" - "github.com/cortezaproject/corteza/server/compose/dalutils" - "github.com/cortezaproject/corteza/server/compose/service/event" - "github.com/cortezaproject/corteza/server/compose/service/values" - "github.com/cortezaproject/corteza/server/compose/types" - "github.com/cortezaproject/corteza/server/pkg/actionlog" - "github.com/cortezaproject/corteza/server/pkg/auth" - "github.com/cortezaproject/corteza/server/pkg/corredor" - "github.com/cortezaproject/corteza/server/pkg/envoy/resource" - "github.com/cortezaproject/corteza/server/pkg/errors" - "github.com/cortezaproject/corteza/server/pkg/eventbus" - "github.com/cortezaproject/corteza/server/store" - systemTypes "github.com/cortezaproject/corteza/server/system/types" + "github.com/cortezaproject/corteza/server/compose/dalutils" + "github.com/cortezaproject/corteza/server/compose/service/event" + "github.com/cortezaproject/corteza/server/compose/service/values" + "github.com/cortezaproject/corteza/server/compose/types" + "github.com/cortezaproject/corteza/server/pkg/actionlog" + "github.com/cortezaproject/corteza/server/pkg/auth" + "github.com/cortezaproject/corteza/server/pkg/corredor" + "github.com/cortezaproject/corteza/server/pkg/envoy/resource" + "github.com/cortezaproject/corteza/server/pkg/errors" + "github.com/cortezaproject/corteza/server/pkg/eventbus" + "github.com/cortezaproject/corteza/server/store" + systemTypes "github.com/cortezaproject/corteza/server/system/types" ) const ( @@ -648,13 +648,17 @@ func (svc record) Bulk(ctx context.Context, skipFailed bool, oo ...*types.Record rr[i].DuplicationError = dupErrors if rve := types.IsRecordValueErrorSet(err); rve != nil { - if valueErrors == nil { + if valueErrors == nil { valueErrors = &types.RecordValueErrorSet{} } // Attach additional meta to each value error for FE identification for _, re := range rve.Set { - if p.ID != "" { + if re.Meta == nil { + continue + } + + if p.ID != "" { re.Meta["id"] = p.ID } diff --git a/server/compose/types/record_detect_duplicates.go b/server/compose/types/record_detect_duplicates.go index 3be08e440..9ea22227a 100644 --- a/server/compose/types/record_detect_duplicates.go +++ b/server/compose/types/record_detect_duplicates.go @@ -124,12 +124,12 @@ func (rule DeDupRule) IssueKind() string { return out.String() } -func (rule DeDupRule) IssueMessage(value string) (out string) { - return fmt.Sprintf("The value %s already exists in another record", value) +func (rule DeDupRule) IssueMessage() (out string) { + return "record-field.errors.duplicateValue" } -func (rule DeDupRule) IssueMultivalueMessage(values []string) (out string) { - return fmt.Sprintf("The values [%s] already exist in another record", strings.Join(values, ", ")) +func (rule DeDupRule) IssueMultivalueMessage() (out string) { + return "record-field.errors.duplicateValueInSet" } func (rule DeDupRule) String() string { @@ -146,6 +146,8 @@ func (rule DeDupRule) checkDuplication(ctx context.Context, ls localeService, re recVal = rec.Values ) + out = &RecordValueErrorSet{} + for _, c := range rule.ConstraintSet { rvv := recVal.FilterByName(c.Attribute) if rvv.Len() == 0 { @@ -160,73 +162,80 @@ func (rule DeDupRule) checkDuplication(ctx context.Context, ls localeService, re moduleField := rec.module.Fields.FindByName(c.Attribute) if moduleField.Multi && c.IsAllEqual() { - return rule.multiValueAllEqual(ctx, ls, c, rvv, existingVv) - } + multiValErr := rule.multiValueAllEqual(ctx, ls, c, rvv, existingVv) + out.Push(multiValErr) + } else { + rvvValueMap := make(map[string]*RecordValue) + _ = rvv.Walk(func(rv *RecordValue) error { + if len(rv.Value) > 0 { + rvvValueMap[rv.Value] = rv + } + return nil + }) - _ = vv.Walk(func(v *RecordValue) error { - if v.RecordID != rec.ID { - _ = rvv.Walk(func(rv *RecordValue) error { - if len(rv.Value) > 0 && matchValue(c.Modifier, rv.Value, v.Value) { - valErr.Push(RecordValueError{ - Kind: rule.IssueKind(), - Message: ls.T(ctx, "compose", rule.IssueMessage(v.Value)), - Meta: map[string]interface{}{ - "field": v.Name, - "value": v.Value, - "dupValueField": rv.Name, - "recordID": cast.ToString(v.RecordID), - "rule": rule.String(), - }, - }) - } + _ = vv.Walk(func(v *RecordValue) error { + if v.RecordID == rec.ID { return nil - }) + } + + if rv, exists := rvvValueMap[v.Value]; exists && matchValue(c.Modifier, rv.Value, v.Value) { + valErr.Push(RecordValueError{ + Kind: rule.IssueKind(), + Message: ls.T(ctx, "compose", rule.IssueMessage()), + Meta: map[string]interface{}{ + "field": v.Name, + "value": v.Value, + "dupValueField": rv.Name, + "recordID": cast.ToString(v.RecordID), + "rule": rule.String(), + }, + }) + } // 1. multiValue is empty, then all value needs to be a match then return error/warning // 2. multiValue is oneOf, then one or more value needs to be a match then return error/warning // 3. multiValue is equal, then all value needs to be a match then return error/warning if (!valErr.IsValid() && (!c.HasMultiValue() || c.IsAllEqual()) && valErr.Len() == rvv.Len()) || (c.IsOneOf() && valErr.Len() > 0) { - if out == nil { - out = &RecordValueErrorSet{} - } out.Push(valErr.Set...) } - } - return nil - }) + + return nil + }) + } } return } -func (rule DeDupRule) multiValueAllEqual(ctx context.Context, ls localeService, c *DeDupRuleConstraint, rvv RecordValueSet, existingVv RecordValueSet) (out *RecordValueErrorSet) { - var ( - valErr = &RecordValueErrorSet{} - ) +func (rule DeDupRule) multiValueAllEqual(ctx context.Context, ls localeService, c *DeDupRuleConstraint, rvv RecordValueSet, existingVv RecordValueSet) (out RecordValueError) { + var valErr = RecordValueError{} - if rvv.Len() == existingVv.Len() { - rvvmap := make(map[string]int) - existingVvmap := make(map[string]int) - - dupValues := recordValueFrequencyMap(rvvmap, c.Modifier, rvv) - _ = recordValueFrequencyMap(existingVvmap, c.Modifier, existingVv) - - if matchRecordValueFrequencyMap(rvvmap, existingVvmap) { - valErr.Push(RecordValueError{ - Kind: rule.IssueKind(), - Message: ls.T(ctx, "compose", rule.IssueMultivalueMessage(dupValues)), - Meta: map[string]interface{}{ - "field": c.Attribute, - "dupValueField": c.Attribute, - "rule": rule.String(), - }, - }) - - return valErr - } + if rvv.Len() != existingVv.Len() { + return valErr } - return nil + rvvmap := make(map[string]int) + existingVvmap := make(map[string]int) + + dupValues := recordValueFrequencyMap(rvvmap, c.Modifier, rvv) + _ = recordValueFrequencyMap(existingVvmap, c.Modifier, existingVv) + + if matchRecordValueFrequencyMap(rvvmap, existingVvmap) { + valErr = RecordValueError{ + Kind: rule.IssueKind(), + Message: ls.T(ctx, "compose", rule.IssueMultivalueMessage()), + Meta: map[string]interface{}{ + "field": c.Attribute, + "value": strings.Join(dupValues, ", "), + "dupValueField": c.Attribute, + "rule": rule.String(), + }, + } + + return valErr + } + + return valErr } func (dr DeDupRuleSet) Validate() (err error) { diff --git a/server/compose/types/record_detect_duplicates_test.go b/server/compose/types/record_detect_duplicates_test.go index 34c08aeb2..f993e68bb 100644 --- a/server/compose/types/record_detect_duplicates_test.go +++ b/server/compose/types/record_detect_duplicates_test.go @@ -67,7 +67,7 @@ func TestDeDupRule_checkCaseSensitiveDuplication(t *testing.T) { Set: []RecordValueError{ { Kind: deDupError.String(), - Message: rule1.IssueMessage("test"), + Message: rule1.IssueMessage(), Meta: map[string]interface{}{ "field": "name", "value": "test", @@ -195,11 +195,12 @@ func TestDedupRule_checkMultiValueEqualDuplication(t *testing.T) { Set: []RecordValueError{ { Kind: deDupError.String(), - Message: rule1.IssueMultivalueMessage([]string{"test", "test test"}), + Message: rule1.IssueMultivalueMessage(), Meta: map[string]interface{}{ "field": "name", "dupValueField": "name", "rule": rule1.String(), + "value": "test, test test", }, }, }, @@ -249,11 +250,12 @@ func TestDedupRule_checkMultiValueEqualDuplication(t *testing.T) { Set: []RecordValueError{ { Kind: deDupError.String(), - Message: rule2.IssueMultivalueMessage([]string{"test", "test tEst"}), + Message: rule2.IssueMultivalueMessage(), Meta: map[string]interface{}{ "field": "name", "dupValueField": "name", "rule": rule2.String(), + "value": "test, test tEst", }, }, }, @@ -302,11 +304,12 @@ func TestDedupRule_checkMultiValueEqualDuplication(t *testing.T) { Set: []RecordValueError{ { Kind: deDupError.String(), - Message: numberRule.IssueMultivalueMessage([]string{"234", "897"}), + Message: numberRule.IssueMultivalueMessage(), Meta: map[string]interface{}{ "field": "count", "dupValueField": "count", "rule": numberRule.String(), + "value": "234, 897", }, }, }, @@ -355,11 +358,12 @@ func TestDedupRule_checkMultiValueEqualDuplication(t *testing.T) { Set: []RecordValueError{ { Kind: deDupError.String(), - Message: locationRule.IssueMultivalueMessage([]string{"{\"coordinates\":[-6.7833479,20.3768206]}", "{\"coordinates\":[0.7833479,10.3768206]}"}), + Message: locationRule.IssueMultivalueMessage(), Meta: map[string]interface{}{ "field": "location", "dupValueField": "location", "rule": locationRule.String(), + "value": "{\"coordinates\":[-6.7833479,20.3768206]}, {\"coordinates\":[0.7833479,10.3768206]}", }, }, },