Repository: CSCfi/rems
Pull request #2908 (2022-04-06 15:34:20)

Save draft improvements
relates #2766 

Successful application save draft command now returns `:warnings` instead of `:errors`, and event is properly processed into application. Previously draft saved event was not processed if any validation errors occurred. Warnings are displayed with yellow flash message.

# Checklist for author

Remove items that aren't applicable, check items that are done.

## Reviewability
- [x] Link to issue
- [x] Consider adding screenshots for ease of review

## Documentation
- [x] Update changelog if necessary

## Testing
- [x] Complex logic is unit tested
- [x] Valuable features are integration / browser / acceptance tested automatically

![Screenshot 2022-04-07 at 18 20 15](https://user-images.githubusercontent.com/794087/162234524-f219a883-a1e7-41db-8458-698e80b2a139.png)
---
![Screenshot 2022-04-07 at 18 19 56](https://user-images.githubusercontent.com/794087/162234555-efbb30d8-df26-402b-a3c8-68184757cc58.png)
---
![Screenshot 2022-04-07 at 18 20 31](https://user-images.githubusercontent.com/794087/162234574-5f8b3649-ee48-41ff-9c1d-ee2b8cc67e5d.png)


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 09964c64a..814a56273 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,6 +34,7 @@ NB: The login has changed to allow more configurable user identity and other att
 - The column names in the tables and the field names in create/edit pages of the administration now match.
 - Improve the accessibility of the small navbar. (#2907)
 - Load config overrides from system properties and env (#2917)
+- Application draft can now be saved even if there are validation warnings. (#2766)
 
 ## v2.25 "Meripuistotie" 2022-02-15
 
diff --git a/resources/translations/en.edn b/resources/translations/en.edn
index 2b762e6e1..da178ab95 100644
--- a/resources/translations/en.edn
+++ b/resources/translations/en.edn
@@ -36,6 +36,7 @@
                      ;; %1 - organization name
                      :organization-does-not-exist "Organization \"%1\" does not exist"
                      :validation-errors "Check the following errors:"
+                     :validation-warnings "Check the following before submit:"
                      :entitlement-end-not-in-future "The expiration date for access rights must be in the future."}
             :export-entitlements "Export entitlements as CSV"
             :generate-ega-api-key-in-profile "Generate one at the Profile page."
diff --git a/resources/translations/fi.edn b/resources/translations/fi.edn
index 0fc085459..8d4ffa065 100644
--- a/resources/translations/fi.edn
+++ b/resources/translations/fi.edn
@@ -34,6 +34,7 @@
                      :licenses-not-accepted "Käyttöehtoja ei ole hyväksytty."
                      :organization-does-not-exist "Organisaatiota \"%1\" ei ole olemassa"
                      :validation-errors "Tarkasta seuraavat virheet:"
+                     :validation-warnings "Tarkasta vielä seuraavat ennen lähetystä:"
                      :entitlement-end-not-in-future "Pääsyoikeuksien vanhenemispäivän täytyy olla tulevaisuudessa."}
             :export-entitlements "Lataa käyttöoikeudet CSV-tiedostona"
             :generate-ega-api-key-in-profile "Luo avain Profiili-sivulla."
diff --git a/resources/translations/sv.edn b/resources/translations/sv.edn
index 1e1838914..e453ef2d9 100644
--- a/resources/translations/sv.edn
+++ b/resources/translations/sv.edn
@@ -34,6 +34,7 @@
                      :licenses-not-accepted "Du har inte godkänt användarvillkoren."
                      :organization-does-not-exist "Organisationen \"%1\" hittades inte."
                      :validation-errors "Var vänlig kontrollera följande:"
+                     :validation-warnings "Vänligen kontrollera följande innan du skickar in:"
                      :entitlement-end-not-in-future "Utgångsdatumet för rättigheter måste vara i framtiden."}
             :export-entitlements "Ladda ner rättigheterna som csv-fil"
             :generate-ega-api-key-in-profile "Hämta den på Profil-sidan."
diff --git a/src/clj/rems/api/schema.clj b/src/clj/rems/api/schema.clj
index 3e6e90515..3d27ea2e2 100644
--- a/src/clj/rems/api/schema.clj
+++ b/src/clj/rems/api/schema.clj
@@ -90,7 +90,8 @@
 
 (s/defschema SuccessResponse
   {:success s/Bool
-   (s/optional-key :errors) [s/Any]})
+   (s/optional-key :errors) [s/Any]
+   (s/optional-key :warnings) [s/Any]})
 
 (s/defschema V2Resource
   {:resource/id s/Int
diff --git a/src/clj/rems/application/commands.clj b/src/clj/rems/application/commands.clj
index 0c5a57028..d3a5d7fe1 100644
--- a/src/clj/rems/application/commands.clj
+++ b/src/clj/rems/application/commands.clj
@@ -252,15 +252,15 @@
   (when-not (application-util/is-handler? application actor)
     (unbundlable-catalogue-items catalogue-item-ids injections)))
 
-(defn- validation-errors-for-draft [forms]
+(defn- form-validation-warnings [forms]
+  (let [warnings (for [form forms
+                       warning (form-validation/validate-fields-for-draft (:form/fields form))]
+                   (assoc warning :form-id (:form/id form)))]
+    (when (seq warnings)
+      {:warnings warnings})))
+
+(defn- form-validation-errors [forms]
   (let [errors (for [form forms
-                     error (form-validation/validate-fields-for-draft (:form/fields form))]
-                 (assoc error :form-id (:form/id form)))]
-    (when (seq errors)
-      {:errors errors})))
-
-(defn- validation-error [application]
-  (let [errors (for [form (:application/forms application)
                      error (form-validation/validate-fields-for-submit (:form/fields form))]
                  (assoc error :form-id (:form/id form)))]
     (when (seq errors)
@@ -376,10 +376,10 @@
                              field (:form/fields form)
                              :when (:field/visible field)]
                          {:form (:form/id form) :field (:field/id field) :value (:field/value field)})]
-    (or (validation-errors-for-draft forms)
-        (ok (-> {:event/type               :application.event/draft-saved
-                 :application/field-values visible-values}
-                (assoc-some :application/duo-codes (:duo-codes cmd)))))))
+    (ok-with-data (form-validation-warnings forms)
+                  (list (-> {:event/type :application.event/draft-saved
+                             :application/field-values visible-values}
+                            (assoc-some :application/duo-codes (:duo-codes cmd)))))))
 
 (defmethod command-handler :application.command/accept-licenses
   [cmd _application _injections]
@@ -390,7 +390,7 @@
   [cmd application _injections]
   (or (merge-with concat
                   (licenses-not-accepted-error application (:actor cmd))
-                  (validation-error application))
+                  (form-validation-errors (:application/forms application)))
       (ok {:event/type :application.event/submitted})))
 
 (defmethod command-handler :application.command/approve
diff --git a/src/cljs/rems/application.cljs b/src/cljs/rems/application.cljs
index d3349223a..b34590d59 100644
--- a/src/cljs/rems/application.cljs
+++ b/src/cljs/rems/application.cljs
@@ -75,27 +75,33 @@
                [:li (get-member-name (:blacklist/user entry))
                 ": " (localized (:catalogue-item/title resource))]))])))
 
-(defn- format-validation-error [type field]
-  [:a {:href "#" :on-click (case (:field/type field)
-                             ;; workaround for tables: there's no single input to focus
-                             :table #(focus/focus-selector (str "#container-" (fields/field-name field)))
-                             ;; default
-                             (focus-input-field (fields/field-name field)))}
-   (text-format type (localized (:field/title field)))])
-
-(defn- format-validation-errors
-  [application errors]
+(defn- format-validations [{:keys [fields label validations]}]
+  [:div label
+   (into [:ul]
+         (for [{:keys [type form-id field-id]} validations]
+           [:li (if-some [field (get-in fields [form-id field-id])]
+                  [:a {:href "#"
+                       :on-click (case (:field/type field)
+                                   ;; workaround for tables: there's no single input to focus
+                                   :table #(focus/focus-selector (str "#container-" (fields/field-name field)))
+                                   (focus-input-field (fields/field-name field)))}
+                   (text-format type (localized (:field/title field)))]
+                  (text type))]))])
+
+(defn- validations [{:keys [application warnings errors]}]
   (let [fields-index (index-by [:form/id :field/id]
                                (for [form (:application/forms application)
                                      field (:form/fields form)]
                                  (assoc field :form/id (:form/id form))))]
-    [:div (text :t.actions.errors/validation-errors)
-     (into [:ul]
-           (concat
-            (for [{:keys [type form-id field-id]} errors]
-              [:li (if (and form-id field-id)
-                     (format-validation-error type (get-in fields-index [form-id field-id]))
-                     (text type))])))]))
+    [:<>
+     (when (seq errors)
+       [format-validations {:fields fields-index
+                            :label (text :t.actions.errors/validation-errors)
+                            :validations errors}])
+     (when (seq warnings)
+       [format-validations {:fields fields-index
+                            :label (text :t.actions.errors/validation-warnings)
+                            :validations warnings}])]))
 
 ;;;; State
 
@@ -146,7 +152,7 @@
     (assoc db ::edit-application {:duo-codes duo-codes
                                   :field-values field-values
                                   :show-diff {}
-                                  :validation-errors nil
+                                  :validation nil
                                   :attachment-status {}})))
 
 (rf/reg-event-db
@@ -157,9 +163,11 @@
        (or initial-fetch? full-reload?) (initialize-edit-application)))))
 
 (rf/reg-event-db
- ::set-validation-errors
- (fn [db [_ errors]]
-   (assoc-in db [::edit-application :validation-errors] errors)))
+ ::set-validations
+ (fn [db [_ errors warnings]]
+   (-> db
+       (assoc-in [::edit-application :validation :errors] errors)
+       (assoc-in [::edit-application :validation :warnings] warnings))))
 
 (defn- field-values-to-api [application field-values]
   (for [form (:application/forms application)
@@ -169,15 +177,23 @@
         :when (form/field-visible? field (get field-values form-id))]
     {:form form-id :field field-id :value (get-in field-values [form-id field-id])}))
 
-(defn- handle-validation-errors [description application on-success]
-  (fn [response]
-    (if (:success response)
-      (on-success)
+(defn- handle-validations! [description application & [on-success]]
+  (fn [{:keys [errors warnings success] :as _response}]
+    (flash-message/clear-message! :top-validation)
+    (rf/dispatch [::set-validations errors warnings])
+    (if-not success
+      (flash-message/show-default-error! :top-validation
+                                         description
+                                         [validations {:application application
+                                                       :errors errors}])
       (do
-        (let [validation-errors (filter :field-id (:errors response))]
-          (rf/dispatch [::set-validation-errors validation-errors]))
-        ;; validation errors can be too long for :actions location
-        (flash-message/show-default-error! :top description [format-validation-errors application (:errors response)])))))
+        (if (seq warnings)
+          (flash-message/show-default-warning! :top-validation
+                                               description
+                                               [validations {:application application
+                                                             :warnings warnings}])
+          (flash-message/show-default-success! :top-validation description))
+        (when on-success (on-success))))))
 
 (defn- duo-codes-to-api [duo-codes]
   (for [duo duo-codes]
@@ -189,15 +205,12 @@
                                 :mondo (map #(select-keys % [:id]) values)
                                 (if (some? values) [{:value values}] []))})}))
 
-(defn- save-application! [description application {:keys [field-values duo-codes]} on-success]
+(defn- save-draft! [description application edit-application handler]
   (post! "/api/applications/save-draft"
          {:params {:application-id (:application/id application)
-                   :field-values field-values
-                   :duo-codes duo-codes}
-          :handler (handle-validation-errors
-                    description
-                    application
-                    on-success)
+                   :field-values (field-values-to-api application (:field-values edit-application))
+                   :duo-codes (duo-codes-to-api (vals (:duo-codes edit-application)))}
+          :handler handler
           :error-handler (flash-message/default-error-handler :actions description)}))
 
 (rf/reg-event-fx
@@ -205,39 +218,38 @@
  (fn [{:keys [db]} [_ description on-success]]
    (let [application (:data (::application db))
          edit-application (::edit-application db)]
-     (save-application! description
-                        application
-                        {:field-values (field-values-to-api application
-                                                            (:field-values edit-application))
-                         :duo-codes (duo-codes-to-api (vals (:duo-codes edit-application)))}
-                        #(do
-                           (rf/dispatch [::fetch-application (:application/id application)])
-                           (if on-success
-                             (on-success)
-                             (flash-message/show-default-success! :actions description)))))
-   {:db (assoc-in db [::edit-application :validation-errors] nil)}))
+     (save-draft! description
+                  application
+                  edit-application
+                  (fn [response]
+                    (rf/dispatch [::fetch-application (:application/id application)])
+                    (apply (handle-validations! description application on-success) [response]))))
+   {:db (-> db
+            (assoc-in [::edit-application :validation :errors] nil)
+            (assoc-in [::edit-application :validation :warnings] nil))}))
 
 (rf/reg-event-fx
  ::submit-application
  (fn [{:keys [db]} [_ description]]
    (let [application (:data (::application db))
          edit-application (::edit-application db)]
-     (save-application! description
-                        application
-                        {:field-values (field-values-to-api application
-                                                            (:field-values edit-application))
-                         :duo-codes (duo-codes-to-api (vals (:duo-codes edit-application)))}
-                        (fn []
-                          (post! "/api/applications/submit"
-                                 {:params {:application-id (:application/id application)}
-                                  :handler (handle-validation-errors
-                                            description
-                                            application
-                                            #(do
-                                               (flash-message/show-default-success! :actions description)
-                                               (rf/dispatch [::fetch-application (:application/id application)])))
-                                  :error-handler (flash-message/default-error-handler :actions description)}))))
-   {:db (assoc-in db [::edit-application :validation-errors] nil)}))
+     (save-draft! description
+                  application
+                  edit-application
+                  (fn [response] ; because warnings are treated as errors in submit, skip validation handling here
+                    (if-not (:success response)
+                      (apply (handle-validations! description application) [response])
+                      (post! "/api/applications/submit"
+                             {:params {:application-id (:application/id application)}
+                              :handler (handle-validations!
+                                        description
+                                        application
+                                        (fn []
+                                          (rf/dispatch [::fetch-application (:application/id application)])))
+                              :error-handler (flash-message/default-error-handler :actions description)})))))
+   {:db (-> db
+            (assoc-in [::edit-application :validation :errors] nil)
+            (assoc-in [::edit-application :validation :warnings] nil))}))
 
 (rf/reg-event-fx
  ::copy-as-new-application
@@ -281,8 +293,7 @@
                          ;; no race condition here: events are handled in a FIFO manner
                          (rf/dispatch [::set-field-value form-id field-id (form/unparse-attachment-ids
                                                                            (conj current-attachments (:id response)))])
-                         (rf/dispatch [::save-application description
-                                       #(rf/dispatch [::set-attachment-status form-id field-id :success])]))
+                         (rf/dispatch [::save-application description #(rf/dispatch [::set-attachment-status form-id field-id :success])]))
               :error-handler (fn [response]
                                (rf/dispatch [::set-attachment-status form-id field-id :error])
                                (cond (= 413 (:status response))
@@ -450,7 +461,9 @@
   (let [edit-application @(rf/subscribe [::edit-application])
         field-values (:field-values edit-application)
         show-diff (:show-diff edit-application)
-        field-validations (index-by [:form-id :field-id] (:validation-errors edit-application))
+        validations (:validation edit-application)
+        field-validations (index-by [:form-id :field-id]
+                                    (some seq [(:errors validations) (:warnings validations)]))
         attachments (index-by [:attachment/id] (:application/attachments application))
         form-fields-editable? (form-fields-editable? application)
         readonly? (not form-fields-editable?)
@@ -1035,7 +1048,9 @@
                           (when-not (str/blank? (:application/description application))
                             (str ": " (:application/description application))))]
      ^{:key application-id} ; re-render to clear flash messages when navigating to another application
-     [flash-message/component :top]
+     [:<>
+      [flash-message/component :top]
+      [flash-message/component :top-validation]]
      (when loading?
        [spinner/big])
      (when application
@@ -1236,7 +1251,7 @@
                                                     :license/link {:en "https://creativecommons.org/licenses/by/4.0/deed.en"}}]}
               :edit-application {:field-values {1 {"fld1" "abc"}}
                                  :show-diff {}
-                                 :validation-errors nil
+                                 :validation {:errors nil :warnings nil}
                                  :accepted-licenses {"applicant" #{5}}}
               :userid "applicant"}])
    (example "application, applied"
diff --git a/src/cljs/rems/flash_message.cljs b/src/cljs/rems/flash_message.cljs
index e5cf6d468..aba466945 100644
--- a/src/cljs/rems/flash_message.cljs
+++ b/src/cljs/rems/flash_message.cljs
@@ -1,61 +1,80 @@
 (ns rems.flash-message
-  (:require [clojure.test :refer [deftest is testing]]
-            [re-frame.core :as rf]
+  (:require [clojure.test :refer [deftest is]]
             [reagent.core :as reagent]
+            [re-frame.core :as rf]
             [rems.administration.status-flags :as status-flags]
             [rems.atoms :as atoms]
             [rems.focus :as focus]
             [rems.text :refer [text text-format]]))
 
-(rf/reg-sub ::message (fn [db _]
-                        (-> (::message db)
-                            (assoc ::message-id (::message-id db)))))
+(defn- location-to-id [location]
+  (let [_ (assert (keyword? location))]
+    (str "flash-message-" (name location))))
 
-(rf/reg-event-fx
- ::reset
- (fn [{:keys [db]} _]
-   {:db (dissoc db ::message)}))
+(deftest test-location-to-id
+  (is (= "flash-message-top" (location-to-id :top))))
 
 (defn- current-time-millis []
   (.getTime (js/Date.)))
 
 (defn- expired? [message]
-  (let [expires (get message :expires 0)]
-    (< expires (current-time-millis))))
+  (< (:expires message 0) (current-time-millis)))
+
+(rf/reg-sub ::message
+            (fn [db [_ location]]
+              (get-in db [::message location])))
+(rf/reg-event-fx ::reset
+                 (fn [{:keys [db]} [_ location]]
+                   {:db (if (some? location)
+                          (update db ::message #(dissoc % location))
+                          (dissoc db ::message))}))
+(rf/reg-event-fx ::show-flash-message
+                 (fn [{:keys [db]} [_ message opts]]
+                   (focus/focus-element-async (str "#" (location-to-id (:location message))))
+                   (when-some [timeout (:timeout opts)]
+                     (js/setTimeout #(rf/dispatch [::reset (:location message)]) timeout))
+                   ;; TODO: flash the message with CSS
+                   {:db (assoc-in db
+                                  [::message (:location message)]
+                                  (assoc message :expires (+ 500 (current-time-millis))))}))
+
+(defn clear-message! [location]
+  (rf/dispatch [::reset location]))
+
+(defn show-success! [location content & [opts]]
+  (let [message {:status :success
+                 :location location
+                 :content content}]
+    (rf/dispatch [::show-flash-message message opts])))
 
-(defn- location-to-id [location]
-  (str "flash-message-"
-       (cond
-         (keyword? location) (name location)
-         (vector? location) (str (name (first location))
-                                 "-"
-                                 (second location))
-         :else (assert false {:location location}))))
+(defn show-default-success! [location description]
+  (rf/dispatch [::reset])
+  (show-success! location
+                 [:div#status-success.flash-message-title description ": " [text :t.form/success]]))
 
-(deftest test-location-to-id
-  (is (= "flash-message-top" (location-to-id :top)))
-  (is (= "flash-message-attachment-10" (location-to-id [:attachment 10]))))
-
-(rf/reg-event-fx
- ::show-flash-message
- (fn [{:keys [db]} [_ message]]
-   (focus/focus-element-async (str "#" (location-to-id (:location message))))
-   ;; TODO: flash the message with CSS
-   {:db (-> db
-            (assoc ::message (assoc message :expires (+ 500 (current-time-millis))))
-            (update ::message-id inc))}))
-
-(defn show-success! [location content]
-  (rf/dispatch [::show-flash-message {:status :success
-                                      :location location
-                                      :content content
-                                      :page @(rf/subscribe [:page])}]))
-
-(defn show-error! [location content]
-  (rf/dispatch [::show-flash-message {:status :danger
-                                      :location location
-                                      :content content
-                                      :page @(rf/subscribe [:page])}]))
+(defn show-error! [location content & [opts]]
+  (let [message {:status :danger
+                 :location location
+                 :content content}]
+    (rf/dispatch [::show-flash-message message opts])))
+
+(defn show-default-error! [location description & more]
+  (rf/dispatch [::reset])
+  (show-error! location (into [:<>
+                               [:div#status-failed.flash-message-title description ": " [text :t.form/failed]]]
+                              more)))
+
+(defn show-warning! [location content & [opts]]
+  (let [message {:status :warning
+                 :location location
+                 :content content}]
+    (rf/dispatch [::show-flash-message message opts])))
+
+(defn show-default-warning! [location description & more]
+  (rf/dispatch [::reset])
+  (show-warning! location (into [:<>
+                                 [:div#status-warning.flash-message-title description ": " [text :t.form/success]]]
+                                more)))
 
 (defn component [location]
   (reagent/create-class
@@ -63,29 +82,17 @@
 
     :component-will-unmount
     (fn [_this]
-      (let [message @(rf/subscribe [::message])]
-        (when (expired? message)
-          (rf/dispatch [::reset]))))
+      ;; XXX: this expiration check keeps component rendered after page transition,
+      ;; for example navigation
+      (when (expired? @(rf/subscribe [::message location]))
+        (rf/dispatch [::reset location])))
 
     :reagent-render
     (fn []
-      (let [message @(rf/subscribe [::message])]
-        (when (= location (:location message))
-          ^{:key (::message-id message)} ; re-render to trigger CSS animations
-          [atoms/flash-message {:id (location-to-id (:location message))
-                                :status (:status message)
-                                :content (:content message)}])))}))
-
-;;; Helpers for typical messages
-
-(defn show-default-success! [location description]
-  (show-success! location [:div#status-success.flash-message-title
-                           description ": " [text :t.form/success]]))
-
-(defn show-default-error! [location description & more]
-  (show-error! location (into [:<> [:div#status-failed.flash-message-title
-                                    description ": " [text :t.form/failed]]]
-                              more)))
+      (when-some [message @(rf/subscribe [::message location])]
+        [atoms/flash-message {:id (location-to-id location)
+                              :status (:status message)
+                              :content (:content message)}]))}))
 
 (defn format-errors [errors]
   ;; TODO: copied as-is from status-modal; consider refactoring


Test patch:
diff --git a/test/clj/rems/api/test_applications.clj b/test/clj/rems/api/test_applications.clj
index b3e03508f..66b6e1e81 100644
--- a/test/clj/rems/api/test_applications.clj
+++ b/test/clj/rems/api/test_applications.clj
@@ -955,12 +955,31 @@
                                                    {:form form-id :field "req1" :value "req"}
                                                    {:form form-id2 :field "opt2" :value "opt"}
                                                    {:form form-id2 :field "req2" :value "req"}]}))))
-    (testing "can't set value of text field to JSON"
-      (is (= {:success false
-              :errors [{:form-id form-id :field-id "req1" :type "t.form.validation/invalid-value"}]}
+    (testing "validation for set value of text field to JSON"
+      (is (= {:success true
+              :warnings [{:form-id form-id :field-id "req1" :type "t.form.validation/invalid-value"}]}
              (send-command user-id {:type :application.command/save-draft
                                     :application-id app-id
                                     :field-values [{:form form-id :field "req1" :value [[{:column "foo" :value "bar"}]]}]}))))
+    (testing "column name validation for table fields"
+      (is (= {:success true
+              :warnings [{:type "t.form.validation/invalid-value" :form-id form-id2 :field-id "table"}]}
+             (send-command user-id {:type :application.command/save-draft
+                                    :application-id app-id
+                                    :field-values [{:form form-id :field "opt1" :value "opt"}
+                                                   {:form form-id :field "req1" :value "req"}
+                                                   {:form form-id2 :field "opt2" :value "opt"}
+                                                   {:form form-id2 :field "req2" :value "req"}
+                                                   {:form form-id2 :field "table"
+                                                    :value [[{:column "col1" :value "1"}
+                                                             {:column "col2" :value "2"}]
+                                                            [{:column "col1" :value "foo"}
+                                                             {:column "colx" :value "bar"}]]}]}))))
+    (testing "cannot submit with validation errors"
+      (is (= {:success false
+              :errors [{:field-id "table" :form-id form-id2 :type "t.form.validation/invalid-value"}]}
+             (send-command user-id {:type :application.command/submit
+                                    :application-id app-id}))))
     (testing "can set value of table field to JSON"
       (is (= {:success true}
              (send-command user-id {:type :application.command/save-draft
@@ -980,32 +999,16 @@
                {:column "col2" :value "bar"}]]
              (get-in (get-application-for-user app-id user-id)
                      [:application/forms 1 :form/fields 2 :field/value]))))
-    (testing "column name validation for table fields"
-      (is (= {:success false
-              :errors [{:type "t.form.validation/invalid-value",
-                        :form-id form-id2,
-                        :field-id "table"}]}
+    (testing "save draft with non-existing value of option list"
+      (is (= {:success true
+              :warnings [{:field-id "optionlist" :form-id form-id2 :type "t.form.validation/invalid-value"}]}
              (send-command user-id {:type :application.command/save-draft
                                     :application-id app-id
                                     :field-values [{:form form-id :field "opt1" :value "opt"}
                                                    {:form form-id :field "req1" :value "req"}
                                                    {:form form-id2 :field "opt2" :value "opt"}
                                                    {:form form-id2 :field "req2" :value "req"}
-                                                   {:form form-id2 :field "table"
-                                                    :value [[{:column "col1" :value "1"}
-                                                             {:column "col2" :value "2"}]
-                                                            [{:column "col1" :value "foo"}
-                                                             {:column "colx" :value "bar"}]]}]}))))
-    (testing "save-draft fails with non-existing value of option list"
-      (is (= {:success false
-              :errors [{:field-id "optionlist", :form-id form-id2, :type "t.form.validation/invalid-value"}]}
-             (send-command user-id {:type           :application.command/save-draft
-                                    :application-id app-id
-                                    :field-values   [{:form form-id :field "opt1" :value "opt"}
-                                                     {:form form-id :field "req1" :value "req"}
-                                                     {:form form-id2 :field "opt2" :value "opt"}
-                                                     {:form form-id2 :field "req2" :value "req"}
-                                                     {:form form-id2 :field "optionlist" :value "foobar"}]}))))
+                                                   {:form form-id2 :field "optionlist" :value "foobar"}]}))))
 
     (testing "set existing value of option list"
       (is (= {:success true}
@@ -1387,10 +1390,10 @@
                          handler)]
         (is (response-is-forbidden? response))))
     (testing "invalid value for attachment field"
-      (is (= {:success false
-              :errors [{:form-id form-id
-                        :field-id "attach"
-                        :type "t.form.validation/invalid-value"}]}
+      (is (= {:success true
+              :warnings [{:form-id form-id
+                          :field-id "attach"
+                          :type "t.form.validation/invalid-value"}]}
              (send-command user-id {:type :application.command/save-draft
                                     :application-id app-id
                                     :field-values [{:form form-id :field "attach" :value "1,a"}]}))))
diff --git a/test/clj/rems/application/test_commands.clj b/test/clj/rems/application/test_commands.clj
index 3b9842e82..10475834f 100644
--- a/test/clj/rems/application/test_commands.clj
+++ b/test/clj/rems/application/test_commands.clj
@@ -185,6 +185,16 @@
         (first events)
         events))))
 
+(defn- ok-command-with-warnings
+  [application cmd injections]
+  (let [cmd (set-command-defaults cmd)
+        result (commands/handle-command cmd application injections)]
+    (assert-ex (not (:errors result)) {:cmd cmd :result result})
+    (let [events (:events result)]
+      (events/validate-events events)
+      {:warnings (:warnings result)
+       :events events})))
+
 (defn- apply-command
   ([application cmd]
    (apply-command application cmd nil))
@@ -376,32 +386,51 @@
                                          {:form 1 :field "2" :value "bar"}]}
                          injections))))
 
-    (testing "does not save a draft when validations fail"
-      (is (= {:errors [{:field-id "1", :type :t.form.validation/invalid-value :form-id 1}]}
-             (fail-command application
-                           {:type :application.command/save-draft
-                            :actor applicant-user-id
-                            :field-values [{:form 1 :field "1" :value "nonexistent_option"}]}
-                           injections))))
+    (testing "saves a draft even when validations fail"
+      (is (= {:warnings [{:field-id "1" :form-id 1 :type :t.form.validation/invalid-value}]
+              :events [{:event/type :application.event/draft-saved
+                        :event/time test-time
+                        :event/actor applicant-user-id
+                        :application/id app-id
+                        :application/field-values [{:form 1 :field "1" :value "nonexistent_option"}]}]}
+             (ok-command-with-warnings
+              application
+              {:type :application.command/save-draft
+               :actor applicant-user-id
+               :field-values [{:form 1 :field "1" :value "nonexistent_option"}]}
+              injections))))
 
     (testing "validation of conditional fields"
       (let [created (assoc dummy-created-event :application/forms [{:form/id 7}])
             application (apply-events nil [created])]
-        (is (= {:errors [{:form-id 7 :field-id "8" :type :t.form.validation/invalid-email}]}
-               (fail-command application
-                             {:type :application.command/save-draft
-                              :actor applicant-user-id
-                              :field-values [{:form 7 :field "7" :value "y"}
-                                             {:form 7 :field "8" :value "invalid_email"}]}
-                             injections))
+        (is (= {:warnings [{:field-id "8" :form-id 7 :type :t.form.validation/invalid-email}]
+                :events [{:event/type :application.event/draft-saved
+                          :event/time test-time
+                          :event/actor applicant-user-id
+                          :application/id app-id
+                          :application/field-values [{:form 7 :field "7" :value "y"}
+                                                     {:form 7 :field "8" :value "invalid_email"}]}]}
+               (ok-command-with-warnings
+                application
+                {:type :application.command/save-draft
+                 :actor applicant-user-id
+                 :field-values [{:form 7 :field "7" :value "y"}
+                                {:form 7 :field "8" :value "invalid_email"}]}
+                injections))
             "visible field should not accept invalid values")
-        (is (= {:errors [{:form-id 7 :field-id "8" :type :t.form.validation/invalid-email}]}
-               (fail-command application
-                             {:type :application.command/save-draft
-                              :actor applicant-user-id
-                              :field-values [{:form 7 :field "7" :value "n"}
-                                             {:form 7 :field "8" :value "invalid_email"}]}
-                             injections))
+        (is (= {:warnings [{:field-id "8" :form-id 7 :type :t.form.validation/invalid-email}]
+                :events [{:event/type :application.event/draft-saved
+                          :event/time test-time
+                          :event/actor applicant-user-id
+                          :application/id app-id
+                          :application/field-values [{:form 7 :field "7" :value "n"}]}]} ; invisible field value is not stored
+               (ok-command-with-warnings
+                application
+                {:type :application.command/save-draft
+                 :actor applicant-user-id
+                 :field-values [{:form 7 :field "7" :value "n"}
+                                {:form 7 :field "8" :value "invalid_email"}]}
+                injections))
             "invisible should not accept invalid values")
         (is (= {:event/type :application.event/draft-saved
                 :event/time test-time
diff --git a/test/clj/rems/test_browser.clj b/test/clj/rems/test_browser.clj
index 9978f4dd0..a800594ae 100644
--- a/test/clj/rems/test_browser.clj
+++ b/test/clj/rems/test_browser.clj
@@ -271,7 +271,7 @@
    (mapv btu/get-element-text-el (btu/query-all selector))))
 
 (defn get-validation-summary []
-  (mapv btu/get-element-text-el (btu/query-all {:css "#flash-message-top ul li"})))
+  (mapv btu/get-element-text-el (btu/query-all {:css "#flash-message-top-validation ul li"})))
 
 (defn get-error-summary []
   (mapv btu/get-element-text-el (btu/query-all {:css "#flash-message-actions > *"})))
@@ -456,7 +456,7 @@
         (testing "add invalid value for field, try to save"
           (fill-form-field "Email field" "user")
           (btu/scroll-and-click :save)
-          (is (btu/eventually-visible? :status-failed))
+          (is (btu/eventually-visible? :status-warning))
           (is (= ["Invalid email address."]
                  (get-validation-summary)))
           (is (= "Invalid email address."