fix: interrupt submission if field is blank (#1096)

* fix: interrupt submission if field is blank * feat: check individual field $invalid prop * feat: add console error before throwing error * docs: add explanation for validity check * feat: suggest different browser in error message
opengovsg · Feb 9, 2021 · 2239e48 · 2239e48
1 parent 39c4fc9
commit 2239e48
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 2 deletions.
diff --git a/src/public/modules/forms/base/directives/submit-form.directive.js b/src/public/modules/forms/base/directives/submit-form.directive.js
@@ -138,8 +138,21 @@ function submitFormDirective(
         }
       }
 
+      const isAnyFieldInvalid = () => {
+        // Check the validity of each individual field. We do this due to
+        // a possible bug in WebKit where form submission may not be correctly
+        // prevented when the form is invalid.
+        return (
+          scope.forms.myForm.$invalid ||
+          scope.form.form_fields.some(
+            ({ _id }) =>
+              scope.forms.myForm[_id] && scope.forms.myForm[_id].$invalid,
+          )
+        )
+      }
+
       scope.checkCaptchaAndSubmit = () => {
-        if (scope.forms.myForm.$invalid) {
+        if (isAnyFieldInvalid()) {
           displayInvalidSubmit()
           return
         }
@@ -302,7 +315,8 @@ function submitFormDirective(
         } catch (err) {
           return handleSubmitFailure(
             err,
-            'Could not prepare your submission. Please contact the form administrator.',
+            'There was an error while processing your submission. Please refresh and try again. ' +
+              'If the problem persists, try using a different browser.',
           )
         }
 

diff --git a/src/public/modules/forms/viewmodels/Fields/CheckboxField.class.js b/src/public/modules/forms/viewmodels/Fields/CheckboxField.class.js
@@ -17,6 +17,17 @@ class CheckboxField extends MixIns.RangeValidation(ArrayAnswerField) {
 
   getResponse() {
     const response = super.getResponse()
+    // Throw error if field value is missing
+    if (this.isVisible && this.required && this.fieldValue.every((v) => !v)) {
+      console.error(
+        `Attempt to getResponse on required field with no answer:\tfieldType=${
+          this.fieldType
+        }, fieldId=${this._id}, typeof fieldValue=${typeof this.fieldValue}`,
+      )
+      throw new Error(
+        `Missing answer for required field, fieldType ${this.fieldType}.`,
+      )
+    }
     // The backend will look for answerArray instead of answer for checkbox
     response.answerArray = this.fieldOptions.filter(
       (_, i) => this.fieldValue[i],

diff --git a/src/public/modules/forms/viewmodels/Fields/SingleAnswerField.class.js b/src/public/modules/forms/viewmodels/Fields/SingleAnswerField.class.js
@@ -19,6 +19,22 @@ class SingleAnswerField extends AnswerField {
    */
   getResponse() {
     const response = super.getResponse()
+    // Throw error if field value is missing
+    if (
+      this.isVisible &&
+      this.required &&
+      this.fieldType !== 'section' &&
+      !this.fieldValue
+    ) {
+      console.error(
+        `Attempt to getResponse on required field with no answer:\tfieldType=${
+          this.fieldType
+        }, fieldId=${this._id}, typeof fieldValue=${typeof this.fieldValue}`,
+      )
+      throw new Error(
+        `Missing answer for required field, fieldType ${this.fieldType}.`,
+      )
+    }
     response.answer =
       this.fieldValue === undefined || this.fieldValue === null
         ? ''