Feature/4202 flags clean

doajtest/testbook/flagged_journals/flagged_journals.yml

@@ -137,4 +137,44 @@ tests:
       - step: Navigate to /editor/your_applications
         results:
           - There is no information about any flags
-          - Only Flagged Records and Flagged to you" facets are **not** displayed
+          - Only Flagged Records and Flagged to you" facets are **not** displayed
+
+  - title: Flag Validation
+    setup:
+      - Open any journal form without a flag
+    context:
+      role: admin
+    steps:
+      - step: Click "Add flag"
+      - step: Enter a note only (leave assignee and deadline empty)
+      - step: Click "Save"
+        results:
+          - An error message is displayed under the assignee input
+          - An error message is displayed under the deadline input
+      - step: Enter a deadline
+      - step: Click "Save"
+        results:
+          - The error under the deadline input is cleared
+          - The error under the assignee input is still displayed
+      - step: Enter an assignee
+      - step: Click "Save"
+        results:
+          - The flag is saved successfully
+          - The flag is displayed correctly with note, assignee, and deadline
+      - step: Edit the flag and remove the deadline value
+      - step: Click "Resolve flag"
+        results:
+          - The flag is marked as resolved
+      - step: Click "Add flag"
+      - step: Enter a note only (leave assignee and deadline empty)
+      - step: Click "Save"
+        results:
+          - An error message is displayed under the assignee input of the new flag
+          - An error message is displayed under the deadline input of the new flag
+          - No validation errors are shown on the resolved flag fields
+      - step: Enter a deadline and an assignee for the new flag
+      - step: Click "Save"
+        results:
+          - The resolved flag is converted to a note
+          - The new flag is saved successfully
+          - The new flag is displayed with note, assignee, and deadline

portality/forms/application_forms.py

@@ -40,7 +40,7 @@
     Year,
     CurrentISOCurrency,
     CurrentISOLanguage,
-    DateInThePast
+    DateInThePast, StopValidationOnOtherValue
 )
 from portality.lib import dates
 from portality.lib.formulaic import Formulaic, WTFormsBuilder, FormulaicContext, FormulaicField
@@ -2026,11 +2026,20 @@ class FieldDefinitions:
 
     FLAG_DEADLINE = {
         "subfield": True,
-        "optional": True,
         "label": "Deadline",
         "name": "flag_deadline",
         "validate": [
-            {"bigenddate": {"message": "This must be a valid date in the BigEnd format (YYYY-MM-DD)"}}
+            {"bigenddate": {"message": "This must be a valid date in the BigEnd format (YYYY-MM-DD)", "ignore_empty": True}},
+            {"stop_validation_on_other_value": {
+                "field": "flag_resolved",
+                "value": "true"
+            }},
+            {"required_if": {
+                "field": "flag_note",
+                "not_empty": True,
+                "message": "The flag must have a deadline",
+                "skip_disabled": True
+            }}
         ],
         "help": {
             "placeholder": "deadline (YYYY-MM-DD)",
@@ -2061,13 +2070,22 @@ class FieldDefinitions:
         "name": "flag_assignee",
         "label": "Assign a user",
         "help": {
-            "placeholder": "assigned_to",
-            "short_help": "A Flag must be assigned to a user. The Flag not assigned to a user will be automatically converted to a note",
+            "placeholder": "assigned_to"
         },
         "group": "flags",
         "validate": [
             "reserved_usernames",
-            "owner_exists"
+            "owner_exists",
+            {"stop_validation_on_other_value": {
+                "field": "flag_resolved",
+                "value": "true"
+            }},
+            {"required_if": {
+                "field": "flag_note",
+                "not_empty": True,
+                "message": "The flag must be assigned to someone",
+                "skip_disabled": True
+            }}
         ],
         "widgets": [
             {"autocomplete": {"type": "admin", "include": False, "allow_clear_input": False}},
@@ -3069,12 +3087,20 @@ class RequiredIfBuilder:
     # ~~->$ RequiredIf:FormValidator~~
     @staticmethod
     def render(settings, html_attrs):
-        val = settings.get("value")
+        val = settings.get("value", "")
         if isinstance(val, list):
             val = ",".join(val)
 
+        if settings.get("skip_disabled"):
+            html_attrs["data-parsley-validate-if-disabled"] = "false"
+
         html_attrs["data-parsley-validate-if-empty"] = "true"
         html_attrs["data-parsley-required-if"] = val
+
+        ne = settings.get("not_empty", False)
+        if ne:
+            html_attrs["data-parsley-required-if-not-empty"] = "true"
+
         html_attrs["data-parsley-required-if-field"] = settings.get("field")
         if "message" in settings:
             html_attrs["data-parsley-required-if-message"] = "<p><small>" + settings["message"] + "</small></p>"
@@ -3083,8 +3109,21 @@ def render(settings, html_attrs):
 
     @staticmethod
     def wtforms(field, settings):
-        return RequiredIfOtherValue(settings.get("field") or field, settings.get("value"), settings.get("message"))
+        set_field = settings.get("field", field)
+        val = settings.get("value")
+        ne = settings.get("not_empty", False)
+        return RequiredIfOtherValue(set_field, val, ne, settings.get("message"))
 
+class StopValidationOnOtherValueBuilder:
+    # ~~->$ StopValidationOnOtherValue:FormValidator~~
+    @staticmethod
+    def render(settings, html_attrs):
+        # no action required here, this is back-end only
+        return
+
+    @staticmethod
+    def wtforms(field, settings):
+        return StopValidationOnOtherValue(settings.get("field", field), settings.get("value"))
 
 class OnlyIfBuilder:
     # ~~->$ OnlyIf:FormValidator~~
@@ -3160,6 +3199,8 @@ class BigEndDateBuilder:
     @staticmethod
     def render(settings, html_attrs):
         html_attrs["data-parsley-validdate"] = ""
+        ignore_empty = settings.get("ignore_empty", False)
+        html_attrs["data-parsley-validdate-ignore_empty"] = "true" if ignore_empty else "false"
         html_attrs["data-parsley-pattern-message"] = settings.get("message")
 
     @staticmethod
@@ -3249,7 +3290,8 @@ def wtforms(field, settings):
             "bigenddate": BigEndDateBuilder.render,
             "no_script_tag": NoScriptTagBuilder.render,
             "year": YearBuilder.render,
-            "date_in_the_past": DateInThePastBuilder.render
+            "date_in_the_past": DateInThePastBuilder.render,
+            "stop_validation_on_other_value": StopValidationOnOtherValueBuilder.render,
         },
         "wtforms": {
             "required": RequiredBuilder.wtforms,
@@ -3276,7 +3318,8 @@ def wtforms(field, settings):
             "year": YearBuilder.wtforms,
             "current_iso_currency": CurrentISOCurrencyBuilder.wtforms,
             "current_iso_language": CurrentISOLanguageBuilder.wtforms,
-            "date_in_the_past": DateInThePastBuilder.wtforms
+            "date_in_the_past": DateInThePastBuilder.wtforms,
+            "stop_validation_on_other_value": StopValidationOnOtherValueBuilder.wtforms
         }
     }
 }

portality/forms/validate.py

@@ -360,8 +360,9 @@ class RequiredIfOtherValue(MultiFieldValidator):
     ~~RequiredIfOtherValue:FormValidator~~
     """
 
-    def __init__(self, other_field_name, other_value, message=None, *args, **kwargs):
+    def __init__(self, other_field_name, other_value, not_empty=False, message=None, *args, **kwargs):
         self.other_value = other_value
+        self.not_empty = not_empty
         self.message = message if message is not None else "This field is required when {x} is {y}".format(x=other_field_name, y=other_value)
         super(RequiredIfOtherValue, self).__init__(other_field_name, *args, **kwargs)
 
@@ -372,6 +373,18 @@ def __call__(self, form, field):
         except:
             return
 
+        if self.not_empty:
+            if isinstance(other_field.data, list):
+                vals = [v for v in other_field.data if v]
+                if len(vals) > 0:
+                    dr = validators.DataRequired(self.message)
+                    dr(form, field)
+            else:
+                if other_field.data:
+                    dr = validators.DataRequired(self.message)
+                    dr(form, field)
+            return
+
         if isinstance(self.other_value, list):
             self._match_list(form, field, other_field)
         else:
@@ -401,6 +414,44 @@ def _match_list(self, form, field, other_field):
             if not field.data or len(field.data) == 0:
                 raise validators.StopValidation()
 
+class StopValidationOnOtherValue(MultiFieldValidator):
+    def __init__(self, other_field_name, other_value, *args, **kwargs):
+        self.other_value = other_value
+        super(StopValidationOnOtherValue, self).__init__(other_field_name, *args, **kwargs)
+
+    def __call__(self, form, field):
+        # attempt to get the other field - if it doesn't exist, just take this as valid
+        try:
+            other_field = self.get_other_field(self.other_field_name, form)
+        except:
+            return
+
+        if isinstance(self.other_value, list):
+            self._match_list(form, field, other_field)
+        else:
+            self._match_single(form, field, other_field)
+
+    def _match_single(self, form, field, other_field):
+        if isinstance(other_field.data, list):
+            match = self.other_value in other_field.data
+        else:
+            match = other_field.data == self.other_value
+        if match:
+            raise validators.StopValidation()
+        else:
+            if not field.data or (isinstance(field.data, str) and not field.data.strip()):
+                raise validators.StopValidation()
+
+    def _match_list(self, form, field, other_field):
+        if isinstance(other_field.data, list):
+            match = len(list(set(self.other_value) & set(other_field.data))) > 0
+        else:
+            match = other_field.data in self.other_value
+        if match:
+            raise validators.StopValidation()
+        else:
+            if not field.data or len(field.data) == 0:
+                raise validators.StopValidation()
 
 class OnlyIf(MultiFieldValidator):
     """

portality/static/js/application_form.js

@@ -793,25 +793,69 @@ doaj.af.ReadOnlyJournalForm = class extends doaj.af.TabbedApplicationForm {
 
 window.Parsley.addValidator("requiredIf", {
     validateString : function(value, requirement, parsleyInstance) {
+        let thisElementId = parsleyInstance.$element[0].id;
+        // console.log(thisElementId)
+
+        const getGroupWithIndexFromInputId = (inputId) => {
+          const match = inputId.match(/^([^-]+)-(\d+)-/);
+          return match ? `${match[1]}-${match[2]}` : null;
+        }
+
+        let skipIfDisabled  = parsleyInstance.$element.attr("data-parsley-validate-if-disabled") === "false";
+        if (skipIfDisabled && parsleyInstance.$element[0].disabled) {
+            return true;
+        }
+
         let field = parsleyInstance.$element.attr("data-parsley-required-if-field");
+
+        // determine if this is the "not empty" value
+        let ne = parsleyInstance.$element.attr("data-parsley-required-if-not-empty")
+        if (ne === "true") {
+            ne = true;
+        } else {
+            ne = false;
+        }
+
         if (typeof requirement !== "string") {
             requirement = requirement.toString();
         }
-
         let requirements = requirement.split(",");
 
-        let other = $("[name='" + field + "']");
+        let prefix = getGroupWithIndexFromInputId(thisElementId)
+        let other = $("[name='" + (prefix ? (prefix + "-") : "") + field + "']");
+
+        // there's a chance `other` is not present in the form.  If so, we should not fail validation, as the field
+        // that triggers the requirement is not present, so we return true
+        if (other.length === 0) {
+            return true;
+        }
+
         let type = other.attr("type");
-        if (type === "checkbox" || type === "radio") {
-            let otherVal = other.filter(":checked").val();
-            if ($.inArray(otherVal, requirements) > -1) {
-                return !!value;
-            }
-        } else {
-            if ($.inArray(other.val(), requirements) > -1) {
-                return !!value;
+
+        // get the value from the other field
+        const otherVal = (type === "checkbox" || type === "radio")
+            ? other.filter(":checked").val()
+            : other.val();
+
+        if (ne) {
+            // if the other value is not empty, then this field is required
+            if (otherVal !== undefined && otherVal !== null && otherVal !== "") {
+                // other value is not empty, so this field is required, so we return true if this field is not empty
+                if (value !== undefined && value !== null && value !== "") {
+                    return true;
+                }
+                return false;
             }
+
+            // if the other value is empty, this field is not required
+            return true;
+        }
+
+        // otherwise check that the otherVal is in our requirements
+        if ($.inArray(otherVal, requirements) > -1) {
+            return !!value;
         }
+
         return true;
     },
     messages: {
@@ -989,8 +1033,12 @@ window.Parsley.addValidator("year", {
 });
 
 window.Parsley.addValidator("validdate", {
-    validateString : function(value) {
+    validateString : function(value, requirements, parsleyInstance) {
         // Check if the value matches the YYYY-MM-DD format
+        const ignore_empty = parsleyInstance.$element.attr("data-parsley-validdate-ignore_empty");
+        if (ignore_empty && !value) {
+            return true;
+        }
         const regex = /^\d{4}-\d{2}-\d{2}$/;
         if (!regex.test(value)) {
           return false; // Invalid format

portality/static/js/formulaic.js

@@ -811,7 +811,6 @@ var formulaic = {
                     }
                 }
 
-
                 this.setUpEventListeners();
                 this.setUI();
             }
@@ -1070,12 +1069,24 @@ var formulaic = {
 
             this.markFlagAsResolved = function() {
                 $(this.flagInputsContainer[this.existingFlagIdx]).addClass("flag--resolved");
+                $(this.flagInputsContainer[this.existingFlagIdx])
+                    .find("input")
+                    .addClass("parsley-excluded")
+                $(this.flagInputsContainer[this.existingFlagIdx])
+                    .find("textarea")
+                    .addClass("parsley-excluded")
                 this.getResolveBtn(this.existingFlagIdx).hide();
                 this.getUnresolveBtn(this.existingFlagIdx).show();
             }
 
             this.markFlagAsUnresolved = function() {
                 $(this.flagInputsContainer[this.existingFlagIdx]).removeClass("flag--resolved");
+                $(this.flagInputsContainer[this.existingFlagIdx])
+                    .find("input")
+                    .removeClass("parsley-excluded")
+                $(this.flagInputsContainer[this.existingFlagIdx])
+                    .find("textarea")
+                    .removeClass("parsley-excluded")
                 this.getResolveBtn(this.existingFlagIdx).show();
                 this.getUnresolveBtn(this.existingFlagIdx).hide();
             }

portality/templates-v2/management/_application-form/includes/_editorial_form_body.html

@@ -15,7 +15,8 @@
       action="{{ form_action }}"
       method="post"
       data-formulaic-after="{{ formulaic_after }}"
-      data-parsley-focus="none">
+      data-parsley-focus="none"
+      data-parsley-excluded="[disabled], .parsley-excluded">
 
     <div class="col-md-8 page">
         <section class="page-content">