Repository: CSCfi/rems
Pull request #1097 (2019-04-09 10:39:20)

fix form validation on submit
for #1092


Patch:
diff --git a/src/clj/rems/application/commands.clj b/src/clj/rems/application/commands.clj
index 9342ed6ce..d12eb14b0 100644
--- a/src/clj/rems/application/commands.clj
+++ b/src/clj/rems/application/commands.clj
@@ -16,7 +16,9 @@
 
 (s/defschema SaveDraftCommand
   (assoc CommandBase
-         :field-values {s/Num s/Str}))
+         ;; {s/Num s/Str} is what we want, but that isn't nicely representable as JSON
+         :field-values [{:field s/Num
+                         :value s/Str}]))
 
 (s/defschema AcceptLicensesCommand
   (assoc CommandBase
diff --git a/src/clj/rems/workflow/dynamic.clj b/src/clj/rems/workflow/dynamic.clj
index 2cf3c06cf..ef3d229bc 100644
--- a/src/clj/rems/workflow/dynamic.clj
+++ b/src/clj/rems/workflow/dynamic.clj
@@ -4,7 +4,7 @@
             [rems.application.commands :as commands]
             [rems.application.model :as model]
             [rems.permissions :as permissions]
-            [rems.util :refer [getx]])
+            [rems.util :refer [getx getx-in]])
   (:import [clojure.lang ExceptionInfo]
            [java.util UUID]
            [org.joda.time DateTime]))
@@ -73,8 +73,8 @@
   (apply merge-with into (keep #(invalid-user-error % injections) user-ids)))
 
 (defn- validation-error [application {:keys [validate-form-answers]}]
-  (let [form-id (:form/id application)
-        answers (:rems.application.model/draft-answers application)
+  (let [form-id (getx-in application [:application/form :form/id])
+        answers (get application :rems.application.model/draft-answers {}) ;; the key does not exist before the first save
         errors (validate-form-answers form-id {:items answers})]
     (when (seq errors)
       {:errors errors})))
@@ -101,7 +101,9 @@
 (defmethod command-handler :application.command/save-draft
   [cmd _application _injections]
   (ok {:event/type :application.event/draft-saved
-       :application/field-values (:field-values cmd)}))
+       :application/field-values (into {}
+                                       (for [{:keys [field value]} (:field-values cmd)]
+                                         [field value]))}))
 
 (defmethod command-handler :application.command/accept-licenses
   [cmd _application _injections]
@@ -257,7 +259,7 @@
                                         :workflow.dynamic/handlers #{"assistant"}}])
         command {:application-id 123 :time (DateTime. 1000)
                  :type :application.command/save-draft
-                 :field-values {}
+                 :field-values []
                  :actor "applicant"}]
     (testing "executes command when user is authorized"
       (is (:success (handle-command command application {}))))
diff --git a/src/cljs/rems/application.cljs b/src/cljs/rems/application.cljs
index aae9dbfe8..19ea6bffb 100644
--- a/src/cljs/rems/application.cljs
+++ b/src/cljs/rems/application.cljs
@@ -138,11 +138,15 @@
    (prn errors)
    (assoc-in db [::edit-application :validation-errors] errors)))
 
+(defn- field-values-to-api [field-values]
+  (for [[field value] field-values]
+    {:field field :value value}))
+
 (defn- save-application! [description application-id field-values]
   (status-modal/common-pending-handler! description)
   (post! "/api/applications/save-draft"
          {:params {:application-id application-id
-                   :field-values field-values}
+                   :field-values (field-values-to-api field-values)}
           :handler (partial status-modal/common-success-handler! #(rf/dispatch [::enter-application-page application-id]))
           :error-handler status-modal/common-error-handler!}))
 
@@ -161,7 +165,7 @@
   (status-modal/common-pending-handler! description)
   (post! "/api/applications/save-draft"
          {:params {:application-id application-id
-                   :field-values field-values}
+                   :field-values (field-values-to-api field-values)}
           :handler (fn [response]
                      (if (:success response)
                        (post! "/api/applications/submit"


Test patch:
diff --git a/src/clj/rems/db/test_data.clj b/src/clj/rems/db/test_data.clj
index 2b199e6de..bf80af6a2 100644
--- a/src/clj/rems/db/test_data.clj
+++ b/src/clj/rems/db/test_data.clj
@@ -469,7 +469,7 @@
                                   :actor user-id
                                   :application-id app-id
                                   :time (get-in form [:application :start])
-                                  :field-values {}})]
+                                  :field-values []})]
     (when dynamic-workflow?
       (applications/add-application-created-event! {:application-id app-id
                                                     :catalogue-item-ids catids
@@ -481,7 +481,7 @@
                              :item item-id :user user-id :value trimmed-value})
       (swap! save-draft-command
              update :field-values
-             conj [item-id trimmed-value]))
+             conj {:field item-id :value trimmed-value}))
     (db/update-application-description! {:id app-id :description field-value})
     (doseq [{license-id :id} (:licenses form)]
       (db/save-license-approval! {:catappid app-id
diff --git a/test/clj/rems/api/test_applications.clj b/test/clj/rems/api/test_applications.clj
index 9a38bc83a..e8515d7a4 100644
--- a/test/clj/rems/api/test_applications.clj
+++ b/test/clj/rems/api/test_applications.clj
@@ -303,7 +303,7 @@
               :errors [{:type "forbidden"}]}
              (send-command "bob" {:type :application.command/save-draft
                                   :application-id application-id
-                                  :field-values {}}))))
+                                  :field-values []}))))
     (testing "submitting"
       (is (= {:success true}
              (send-command user-id {:type :application.command/submit
@@ -314,6 +314,42 @@
                 "application.event/submitted"]
                (map :event/type (get submitted :application/events))))))))
 
+(deftest test-application-validation
+  (let [api-key "42"
+        user-id "alice"
+        workflow-id (create-dynamic-workflow)
+        form-id (create-form-with-fields [{:title {:en "req"}
+                                           :type "text"
+                                           :optional false}
+                                          {:title {:en "opt"}
+                                           :type "text"
+                                           :optional true}])
+        [req-id opt-id] (->> (form/get-form form-id)
+                             :fields
+                             (map :id))
+        cat-id (create-catalogue-item form-id workflow-id)
+        app-id (create-application [cat-id] user-id)]
+    (testing "set value of optional field"
+      (is (= {:success true}
+             (send-command user-id {:type :application.command/save-draft
+                                    :application-id app-id
+                                    :field-values [{:field opt-id :value "opt"}]}))))
+    (testing "can't submit without required field"
+      (is (= {:success false
+              :errors [{:field-id req-id, :type "t.form.validation/required"}]}
+             (send-command user-id {:type :application.command/submit
+                                    :application-id app-id}))))
+    (testing "set value of required field"
+      (is (= {:success true}
+             (send-command user-id {:type :application.command/save-draft
+                                    :application-id app-id
+                                    :field-values [{:field opt-id :value "opt"}
+                                                   {:field req-id :value "req"}]})))
+    (testing "can submit with required field"
+      (is (= {:success true}
+             (send-command user-id {:type :application.command/submit
+                                    :application-id app-id})))))))
+
 (def testfile (clojure.java.io/file "./test-data/test.txt"))
 
 (def malicious-file (clojure.java.io/file "./test-data/malicious_test.html"))
diff --git a/test/clj/rems/workflow/test_dynamic.clj b/test/clj/rems/workflow/test_dynamic.clj
index df46ff324..a52f7d398 100644
--- a/test/clj/rems/workflow/test_dynamic.clj
+++ b/test/clj/rems/workflow/test_dynamic.clj
@@ -88,17 +88,20 @@
              (ok-command application
                          {:type :application.command/save-draft
                           :actor applicant-user-id
-                          :field-values {1 "foo" 2 "bar"}}))))
+                          :field-values [{:field 1 :value "foo"}
+                                         {:field 2 :value "bar"}]}))))
     (testing "only the applicant can save a draft"
       (is (= {:errors [{:type :forbidden}]}
              (fail-command application
                            {:type :application.command/save-draft
                             :actor "non-applicant"
-                            :field-values {1 "foo" 2 "bar"}})
+                            :field-values [{:field 1 :value "foo"}
+                                           {:field 2 :value "bar"}]})
              (fail-command application
                            {:type :application.command/save-draft
                             :actor handler-user-id
-                            :field-values {1 "foo" 2 "bar"}}))))
+                            :field-values [{:field 1 :value "foo"}
+                                           {:field 2 :value "bar"}]}))))
     (testing "draft cannot be updated after submitting"
       (let [application (apply-events application
                                       [{:event/type :application.event/submitted
@@ -109,7 +112,7 @@
                (fail-command application
                              {:type :application.command/save-draft
                               :actor applicant-user-id
-                              :field-values {1 "updated"}})))))
+                              :field-values [{:field 1 :value "updated"}]})))))
     (testing "draft can be updated after returning it to applicant"
       (let [application (apply-events application
                                       [{:event/type :application.event/submitted
@@ -129,7 +132,7 @@
                (ok-command application
                            {:type :application.command/save-draft
                             :actor applicant-user-id
-                            :field-values {1 "updated"}})))))))
+                            :field-values [{:field 1 :value "updated"}]})))))))
 
 (deftest test-accept-licenses
   (let [application (apply-events nil [dummy-created-event])]