From 32f2db680281cabde3159b9a17702cfa23b2c200 Mon Sep 17 00:00:00 2001
From: jensilo <k@jensheise.com>
Date: Fri, 15 Dec 2023 12:35:46 +0100
Subject: [PATCH] improve ui for optional equal rule, add more tests for
parser, adjust templates to new change to maintain consistency
---
docs/templates/paris/ak.json | 12 +-
docs/templates/paris/esfa.json | 1 +
docs/templates/paris/esqua.json | 1 +
docs/templates/paris/glossar.json | 5 +
src/app/eiffel/parser.go | 69 ++++++-----
src/app/eiffel/parser_test.go | 130 ++++++++++++++++-----
templates/eiffel/_form-elicitation.go.html | 26 ++++-
7 files changed, 184 insertions(+), 60 deletions(-)
diff --git a/docs/templates/paris/ak.json b/docs/templates/paris/ak.json
index a5b3642..3800df2 100644
--- a/docs/templates/paris/ak.json
+++ b/docs/templates/paris/ak.json
@@ -21,6 +21,7 @@
"Gegeben,"
],
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "small"
},
"prae-bedingung-kurz": {
@@ -35,7 +36,8 @@
"hint": "Welche Situation liegt vor?",
"explanation": "Beschreibung der Situation vor dem Ereignis. Beispiel: Angenommen, der Anwender X hat ein Meeting für den Anwender X für den 1.3.201x von 13:00 bis 15:00 angelegt.",
"size": "large",
- "optional": true
+ "optional": true,
+ "ignoreMissingWhenOptional": true
},
"prae-ereignis": {
"name": "Prä-Ereignis",
@@ -46,6 +48,7 @@
"Prüfe"
],
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "small"
},
"prae-ereignis-kurz": {
@@ -60,7 +63,8 @@
"hint": "Was soll passieren?",
"explanation": "Beschreibung Ereignis(se) bzw. Aktion(en). Beispiel: Wenn ein anderer Anwender Y für den Anwender X ein zweites Meeting für den 1.3.201x von 13:00 bis 15:00 anlegen will,",
"size": "large",
- "optional": true
+ "optional": true,
+ "ignoreMissingWhenOptional": true
},
"prae-ergebnis": {
"name": "Prä-Ergebnis",
@@ -71,6 +75,7 @@
"dann"
],
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "small"
},
"prae-ergebnis-kurz": {
@@ -92,7 +97,8 @@
"hint": "Warum ist es so?",
"explanation": "Beschreibung der Begründung für das Ergebnis. Beispiel: weil ein Anwender immer nur in einem Meeting gleichzeitig anwesend sein kann.",
"size": "large",
- "optional": true
+ "optional": true,
+ "ignoreMissingWhenOptional": true
},
"punkt": {
"name": "Punkt",
diff --git a/docs/templates/paris/esfa.json b/docs/templates/paris/esfa.json
index 167f911..f303c76 100644
--- a/docs/templates/paris/esfa.json
+++ b/docs/templates/paris/esfa.json
@@ -98,6 +98,7 @@
"hint": "Warum wird es getan?",
"explanation": "Beispiel: weil dieser manuell nach Auffälligkeiten suchen will",
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "large"
},
"punkt": {
diff --git a/docs/templates/paris/esqua.json b/docs/templates/paris/esqua.json
index 6e3cad9..f7c1e96 100644
--- a/docs/templates/paris/esqua.json
+++ b/docs/templates/paris/esqua.json
@@ -84,6 +84,7 @@
"hint": "Warum?",
"explanation": "Beispiel: weil die Mitarbeiter in Ihren Pausen keine Wartezeiten tolerieren",
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "large"
},
"punkt": {
diff --git a/docs/templates/paris/glossar.json b/docs/templates/paris/glossar.json
index 689dc16..3a56fe2 100644
--- a/docs/templates/paris/glossar.json
+++ b/docs/templates/paris/glossar.json
@@ -30,6 +30,7 @@
"hint": "Mit welchen Synonymen?",
"explanation": "Synonyme für den zu erklärenden Begriff. Beispiel: (Synonyme: Ausleiher, Besucher, Leser)",
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "medium"
},
"uebersetzung": {
@@ -38,6 +39,7 @@
"hint": "Mit welchen Übersetzungen?",
"explanation": "Übersetzung des zu erklärenden Begriffs. Beispiel: (engl. Reader)",
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "medium"
},
"bedeutet": {
@@ -55,6 +57,7 @@
"in der"
],
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "small"
},
"bereich": {
@@ -79,6 +82,7 @@
"den Prozess"
],
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "small"
},
"definiens": {
@@ -102,6 +106,7 @@
"hint": "Warum ist es so festgelegt?",
"explanation": "Begründung für die Definition. Beispiel: weil juristische Personen nicht zur Ausleihe zugelassen sind",
"optional": true,
+ "ignoreMissingWhenOptional": true,
"size": "large"
},
"punkt": {
diff --git a/src/app/eiffel/parser.go b/src/app/eiffel/parser.go
index dfbff29..d805640 100644
--- a/src/app/eiffel/parser.go
+++ b/src/app/eiffel/parser.go
@@ -76,6 +76,10 @@ type BasicRule struct {
// Optional means a rule is not required to be parsed without template.ParsingLogLevelError (parsing error).
// By that parsing an invalid requirement for an optional rule will not result in a parsing error.
Optional bool `json:"optional"`
+ // IgnoreMissingWhenOptional means that a missing or empty segment for this rule will not be displayed at all.
+ // Usually, a missing or empty segment for an optional rule will be an error downgraded to a notice.
+ // However, if this flag is set to true, the segment will not be displayed at all.
+ IgnoreMissingWhenOptional bool `json:"ignoreMissingWhenOptional"`
// Size is the expected size of the rule's value. It is optional. Possible values are:
// - "small" (default): The value is expected to be a short string. 1/4 of the input field width.
// - "medium": The value is expected to be a medium-sized string. 2/4 of the input field width.
@@ -220,11 +224,14 @@ func (bt *BasicTemplate) Parse(ctx context.Context, ruleParsers *RuleParserProvi
return result, RuleMissingError{Rule: ruleName, Template: bt.Name, Variant: variant.Name}
}
+ var parsingLogs []parser.ParsingLog
segment, ok := indexedSegments[ruleName]
- if (!ok || segment.Value == "") && !rule.Optional {
- result.Errors = append(result.Errors, parser.ParsingLog{
+
+ isMissing := !ok || segment.Value == ""
+ if isMissing {
+ parsingLogs = append(parsingLogs, parser.ParsingLog{
Segment: &parser.ParsingSegment{Name: ruleName},
- Level: parser.ParsingLogLevelError,
+ Level: parser.ParsingLogLevelError, // if optional this will be downgraded to a notice in a moment
Message: "eiffel.parser.error.missing-segment",
TranslationArgs: []string{
"name",
@@ -233,36 +240,22 @@ func (bt *BasicTemplate) Parse(ctx context.Context, ruleParsers *RuleParserProvi
ruleName,
},
})
- continue
- }
-
- if !ok || segment.Value == "" {
- continue // rule is optional and segment is missing -> ignore
}
- ruleParser, err := ruleParsers.Parser(rule.Type)
- if err != nil {
- return result, err // should also never happen because the template is expected to be valid
+ if rule.Optional && isMissing && rule.IgnoreMissingWhenOptional {
+ continue
}
- parsingLogs, err := ruleParser.Parse(ctx, rule, segment)
- if err != nil {
- return result, err
- }
+ if !isMissing {
+ var err error
+ parsingLogs, err = parse(ctx, ruleParsers, rule, segment)
+ if err != nil {
+ return result, err
+ }
- be := " "
- af := ""
- before, bOk := rule.Extra["before"]
- after, aOk := rule.Extra["after"]
- if bStr, ok := before.(string); ok && bOk {
- be = bStr
- }
- if aStr, ok := after.(string); ok && aOk {
- af = aStr
+ buildRequirementIncrementally(rule, segment, &result)
}
- result.Requirement += be + strings.TrimSpace(segment.Value) + af
-
for _, log := range parsingLogs {
switch log.Level {
case parser.ParsingLogLevelError:
@@ -286,6 +279,30 @@ func (bt *BasicTemplate) Parse(ctx context.Context, ruleParsers *RuleParserProvi
return result, nil
}
+func parse(ctx context.Context, ruleParsers *RuleParserProvider, rule BasicRule, segment parser.ParsingSegment) ([]parser.ParsingLog, error) {
+ ruleParser, err := ruleParsers.Parser(rule.Type)
+ if err != nil {
+ return nil, err
+ }
+
+ return ruleParser.Parse(ctx, rule, segment)
+}
+
+func buildRequirementIncrementally(rule BasicRule, segment parser.ParsingSegment, result *parser.ParsingResult) {
+ be := " "
+ af := ""
+ before, bOk := rule.Extra["before"]
+ after, aOk := rule.Extra["after"]
+ if bStr, ok := before.(string); ok && bOk {
+ be = bStr
+ }
+ if aStr, ok := after.(string); ok && aOk {
+ af = aStr
+ }
+
+ result.Requirement += be + strings.TrimSpace(segment.Value) + af
+}
+
// Validate makes sure that the template is valid. It checks the structure of and semantic of the values inside.
// Each rule configuration is validated by the corresponding rule parser.
// It returns a slice of validation errors that are safe to show to the user (translatable).
diff --git a/src/app/eiffel/parser_test.go b/src/app/eiffel/parser_test.go
index d11cd62..39a3157 100644
--- a/src/app/eiffel/parser_test.go
+++ b/src/app/eiffel/parser_test.go
@@ -28,30 +28,107 @@ func TestBasicParser_Parse(t *testing.T) {
bt := basicTemplate()
rp := ruleParsers()
- errs := bt.Validate(validation.New(), rp)
- require.Len(t, errs, 0)
+ t.Run("basic variant is valid", func(t *testing.T) {
+ errs := bt.Validate(validation.New(), rp)
+ require.Len(t, errs, 0)
+ })
+
+ t.Run("basic variant parsing with error in optional rule downgraded to notice", func(t *testing.T) {
+ parsingResult, err := bt.Parse(
+ context.Background(),
+ rp,
+ "basicVariant",
+ parser.ParsingSegment{Name: "pre", Value: "This "},
+ parser.ParsingSegment{Name: "stateVerbRule", Value: "is"},
+ parser.ParsingSegment{Name: "mid", Value: "a"},
+ parser.ParsingSegment{Name: "fooRule", Value: " foo "},
+ parser.ParsingSegment{Name: "fooPostfixRule", Value: "example"},
+ parser.ParsingSegment{Name: "end", Value: "."},
+ parser.ParsingSegment{Name: "optionalErrorTestRule", Value: "bar"},
+ )
+
+ require.NoError(t, err)
+ require.Len(t, parsingResult.Notices, 1)
+ assert.True(t, parsingResult.Notices[0].Downgrade)
+ assert.Equal(t, "test-template", parsingResult.TemplateID)
+ assert.True(t, parsingResult.Ok(), "parsing result should be ok but parsing errors occurred")
+ assert.True(t, parsingResult.Flawless(), "parsing result should be flawless")
+ assert.Equal(t, parsingResult.Notices[0].Segment.Name, "optionalErrorTestRule")
+ assert.True(t, parsingResult.Notices[0].Downgrade, "notice should be downgraded for optional rule")
+ })
+
+ t.Run("missing optional rule with error downgraded to notice", func(t *testing.T) {
+ parsingResult, err := bt.Parse(
+ context.Background(),
+ rp,
+ "basicVariant",
+ parser.ParsingSegment{Name: "pre", Value: "This "},
+ parser.ParsingSegment{Name: "stateVerbRule", Value: "is"},
+ parser.ParsingSegment{Name: "mid", Value: "a"},
+ parser.ParsingSegment{Name: "fooRule", Value: " foo "},
+ parser.ParsingSegment{Name: "fooPostfixRule", Value: "example"},
+ parser.ParsingSegment{Name: "end", Value: "."},
+ parser.ParsingSegment{Name: "optionalErrorTestRule", Value: "bar"},
+ )
+
+ require.NoError(t, err)
+ require.Len(t, parsingResult.Notices, 1)
+ assert.True(t, parsingResult.Notices[0].Downgrade)
+ })
+
+ t.Run("missing input for a required (non-optional) rule error", func(t *testing.T) {
+ bt := basicTemplate()
+
+ parsingResult, err := bt.Parse(
+ context.Background(),
+ rp,
+ "basicVariant",
+ parser.ParsingSegment{Name: "pre", Value: "This "},
+ parser.ParsingSegment{Name: "stateVerbRule", Value: "wrong-state-verb"},
+ parser.ParsingSegment{Name: "mid", Value: "a"},
+ parser.ParsingSegment{Name: "fooRule", Value: "bar-missing-foo"},
+ parser.ParsingSegment{Name: "fooPostfixRule", Value: "example"},
+ parser.ParsingSegment{Name: "end", Value: "."},
+ )
- parsingResult, err := bt.Parse(
- context.Background(),
- rp,
- "basicVariant",
- parser.ParsingSegment{Name: "pre", Value: "This "},
- parser.ParsingSegment{Name: "stateVerbRule", Value: "is"},
- parser.ParsingSegment{Name: "mid", Value: "a"},
- parser.ParsingSegment{Name: "fooRule", Value: " foo "},
- parser.ParsingSegment{Name: "fooPostfixRule", Value: "example"},
- parser.ParsingSegment{Name: "end", Value: "."},
- parser.ParsingSegment{Name: "optionalErrorTestRule", Value: "bar"},
- )
-
- require.NoError(t, err)
- require.Len(t, parsingResult.Notices, 1)
- assert.True(t, parsingResult.Notices[0].Downgrade)
- assert.Equal(t, "test-template", parsingResult.TemplateID)
- assert.True(t, parsingResult.Ok(), "parsing result should be ok but parsing errors occurred")
- assert.True(t, parsingResult.Flawless(), "parsing result should be flawless")
- assert.Equal(t, parsingResult.Notices[0].Segment.Name, "optionalErrorTestRule")
- assert.True(t, parsingResult.Notices[0].Downgrade, "notice should be downgraded for optional rule")
+ require.NoError(t, err)
+ require.Len(t, parsingResult.Errors, 2)
+ })
+
+ t.Run("invalid variant, expecting error from parse", func(t *testing.T) {
+ _, err := bt.Parse(context.Background(), rp, "invalidVariant")
+ assert.Error(t, err)
+ })
+
+ t.Run("invalid template config, expecting error from parse", func(t *testing.T) {
+ bt.Rules = map[string]BasicRule{}
+ _, err := bt.Parse(context.Background(), rp, "basicVariant")
+ assert.Error(t, err)
+ })
+
+ t.Run("valid variant, valid configuration, valid requirement, no errors, flawless result w/o notices", func(t *testing.T) {
+ bt := basicTemplate()
+ rp := ruleParsers()
+
+ parsingResult, err := bt.Parse(
+ context.Background(),
+ rp,
+ "basicVariant",
+ parser.ParsingSegment{Name: "pre", Value: "This "},
+ parser.ParsingSegment{Name: "stateVerbRule", Value: "is"},
+ parser.ParsingSegment{Name: "mid", Value: "a"},
+ parser.ParsingSegment{Name: "fooRule", Value: " foo "},
+ parser.ParsingSegment{Name: "fooPostfixRule", Value: "example"},
+ parser.ParsingSegment{Name: "end", Value: "."},
+ parser.ParsingSegment{Name: "optionalErrorTestRule", Value: "foo"},
+ )
+
+ require.NoError(t, err)
+ require.Len(t, parsingResult.Errors, 0)
+ require.Len(t, parsingResult.Notices, 0)
+ assert.True(t, parsingResult.Ok(), "parsing result should be ok but parsing errors occurred")
+ assert.True(t, parsingResult.Flawless(), "parsing result should be flawless")
+ })
}
func basicTemplate() *BasicTemplate {
@@ -87,9 +164,10 @@ func basicTemplate() *BasicTemplate {
Optional: true,
},
"optionalMissingTestRule": {
- Name: "Optional Missing Test Rule",
- Type: "placeholder",
- Optional: true,
+ Name: "Optional Missing Test Rule",
+ Type: "placeholder",
+ Optional: true,
+ IgnoreMissingWhenOptional: true,
},
"optionalErrorTestRule": {
Name: "Optional Empty Error Test Rule",
diff --git a/templates/eiffel/_form-elicitation.go.html b/templates/eiffel/_form-elicitation.go.html
index 350dae4..f52c6e9 100644
--- a/templates/eiffel/_form-elicitation.go.html
+++ b/templates/eiffel/_form-elicitation.go.html
@@ -46,6 +46,10 @@
{{ if or (eq $displayType "input-text")
(eq $displayType "text")
(eq $displayType "input-single-select") }}
+
+ {{ $optionalText := and (eq $displayType "text") $rule.Optional }}
+ {{ $nonOptionalText := and (eq $displayType "text") (not $optionalText) }}
+
<div class="mb-3">
<div class="input-group {{ if $violations }}has-validation{{ end }}">
<span data-bs-target="#eiffelRule-{{ $ruleName }}-info" class="input-group-text" role="button" data-bs-toggle="modal">i</span>
@@ -59,7 +63,7 @@
</datalist>
{{ end }}
- {{ if eq $displayType "text" }}
+ {{ if $nonOptionalText }}
<input type="hidden" name="{{ $inputName }}" value="{{ $rule.Value }}" />
{{ end }}
@@ -67,12 +71,24 @@
id="eiffelFormInput-{{ $ruleName }}"
class="form-control {{ if $violations }}is-invalid{{ end }}"
name="{{ $inputName }}"
- placeholder="{{ $displayName}}"
aria-label="{{ $displayName }}"
aria-description="{{ $rule.Hint }}"
- {{ if eq $displayType "text" }}
- disabled value="{{ $rule.Value }}"
- {{ else if $parsingResult }}value="{{ index $segments $ruleName }}"{{ end }}
+
+ {{ if $nonOptionalText }} {{/* show fixed text in input */}}
+ value="{{ $rule.Value }}"
+ disabled
+ {{ else if $parsingResult }} {{/* show content from last submit for optional text show value as placeholder */}}
+ value="{{ index $segments $ruleName }}"
+ {{ else if $optionalText }} {{/* show value that's suggested by the rule */}}
+ value="{{ $rule.Value }}"
+ {{ end }}
+
+ {{ if $optionalText }}
+ placeholder="{{ $rule.Value }}"
+ {{ else }}
+ placeholder="{{ $displayName }}"
+ {{ end }}
+
{{ if eq $displayType "input-single-select" }}list="eiffelFormInput-{{ $ruleName }}-datalist"{{ end }}
{{ if not $rule.Optional }}required{{ end }}
{{ if $first }}autofocus{{ end }}
--
GitLab