Repository: CSCfi/rems
Pull request #1200 (2019-05-03 10:21:15)

move some form stuff from rems.db.application to rems.db.form
follow-up to #1198 


Patch:
diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql
index 9267822cf..41f05afb5 100644
--- a/resources/sql/queries.sql
+++ b/resources/sql/queries.sql
@@ -172,17 +172,7 @@ SELECT
   form.start as start,
   form.endt as "end",
   form.enabled,
-  form.archived,
-  (SELECT json_agg(joined)
-   FROM (SELECT *,
-                (SELECT json_agg(formitemlocalization)
-                 FROM application_form_item_localization formitemlocalization
-                 WHERE (formitemmap.formitemid = formitemlocalization.itemid)
-                 GROUP BY formitemlocalization.itemid)  AS localizations
-         FROM application_form_item_map formitemmap
-         JOIN application_form_item formitem ON (formitemmap.formitemid = formitem.id)
-         WHERE formitemmap.formid = form.id) joined)::TEXT
-         AS fields
+  form.archived
 FROM application_form form
 WHERE form.id = :id;
 
diff --git a/src/clj/rems/api/applications_v2.clj b/src/clj/rems/api/applications_v2.clj
index c45076e44..4b65a3851 100644
--- a/src/clj/rems/api/applications_v2.clj
+++ b/src/clj/rems/api/applications_v2.clj
@@ -9,6 +9,7 @@
             [rems.auth.util :refer [throw-forbidden]]
             [rems.db.applications :as applications]
             [rems.db.catalogue :as catalogue]
+            [rems.db.form :as form]
             [rems.db.licenses :as licenses]
             [rems.db.users :as users]
             [rems.db.workflow :as workflow]
@@ -18,7 +19,7 @@
   (:import [org.joda.time Duration]))
 
 (def ^:private injections
-  {:get-form applications/get-form
+  {:get-form form/get-form
    :get-catalogue-item catalogue/get-localized-catalogue-item
    :get-license licenses/get-license
    :get-user users/get-user-attributes
diff --git a/src/clj/rems/db/applications.clj b/src/clj/rems/db/applications.clj
index 07be41991..2788c8369 100644
--- a/src/clj/rems/db/applications.clj
+++ b/src/clj/rems/db/applications.clj
@@ -5,7 +5,6 @@
             [clojure.java.jdbc :as jdbc]
             [clojure.test :refer [deftest is]]
             [conman.core :as conman]
-            [cprop.tools :refer [merge-maps]]
             [rems.application.events :as events]
             [rems.application.model :as model]
             [rems.auth.util :refer [throw-forbidden]]
@@ -29,73 +28,18 @@
 
 (declare get-dynamic-application-state)
 
+;;; Query functions
+
 (defn draft?
   "Is the given `application-id` for an unsaved draft application?"
   [application-id]
   (nil? application-id))
 
-;;; Query functions
-
 (declare is-dynamic-application?)
-
 (declare fix-workflow-from-db)
 (declare is-dynamic-handler?)
 
-;;; Form & field stuff
-;; TODO these should be in rems.db.form or so
-
-(defn- get-field-value [field form-id application-id]
-  (let [query-params {:item (:id field)
-                      :form form-id
-                      :application application-id}]
-    (if (= "attachment" (:type field))
-      (:filename (db/get-attachment query-params))
-      (:value (db/get-field-value query-params)))))
-
-(defn- process-field-options [options]
-  (->> options
-       (map (fn [{:keys [key langcode label displayorder]}]
-              {:key key
-               :label {(keyword langcode) label}
-               :displayorder displayorder}))
-       (group-by :key)
-       (map (fn [[_key options]] (apply merge-maps options))) ; merge label translations
-       (sort-by :displayorder)
-       (mapv #(select-keys % [:key :label]))))
-
-(deftest process-field-options-test
-  (is (= [{:key "yes" :label {:en "Yes" :fi "Kyllä"}}
-          {:key "no" :label {:en "No" :fi "Ei"}}]
-         (process-field-options
-          [{:itemid 9, :key "no", :langcode "en", :label "No", :displayorder 1}
-           {:itemid 9, :key "no", :langcode "fi", :label "Ei", :displayorder 1}
-           {:itemid 9, :key "yes", :langcode "en", :label "Yes", :displayorder 0}
-           {:itemid 9, :key "yes", :langcode "fi", :label "Kyllä", :displayorder 0}]))))
-
-(defn process-field
-  "Returns a field structure like this:
-
-    {:id 123
-     :type \"texta\"
-     :title \"Item title\"
-     :inputprompt \"hello\"
-     :optional true
-     :value \"filled value or nil\"}"
-  [application-id form-id field]
-  {:id (:id field)
-   :optional (:formitemoptional field)
-   :type (:type field)
-   ;; TODO here we do a db call per item, for licenses we do one huge
-   ;; db call. Not sure which is better?
-   :localizations (into {} (for [{:keys [langcode title inputprompt]}
-                                 (db/get-form-item-localizations {:item (:id field)})]
-                             [(keyword langcode) {:title title :inputprompt inputprompt}]))
-   :options (process-field-options (db/get-form-item-options {:item (:id field)}))
-   :value (or
-           (when-not (draft? application-id)
-             (get-field-value field form-id application-id))
-           "")
-   :maxlength (:maxlength field)})
+;;; Creating drafts
 
 (defn create-new-draft [user-id wfid]
   (assert user-id)
@@ -252,14 +196,8 @@
 (defn- valid-user? [userid]
   (not (nil? (users/get-user-attributes userid))))
 
-(defn get-form [form-id]
-  (-> (form/get-form form-id)
-      (select-keys [:id :organization :title :start :end])
-      (assoc :items (->> (db/get-form-items {:id form-id})
-                         (mapv #(process-field nil form-id %))))))
-
 (defn- validate-form-answers [form-id answers]
-  (let [form (get-form form-id)
+  (let [form (form/get-form form-id)
         _ (assert form)
         fields (for [field (:items form)]
                  (assoc field :value (get-in answers [:items (:id field)])))]
diff --git a/src/clj/rems/db/applications/legacy.clj b/src/clj/rems/db/applications/legacy.clj
index a057d45c5..49e4c7e02 100644
--- a/src/clj/rems/db/applications/legacy.clj
+++ b/src/clj/rems/db/applications/legacy.clj
@@ -10,6 +10,7 @@
             [rems.db.core :as db]
             [rems.db.catalogue :as catalogue]
             [rems.db.entitlements :as entitlements]
+            [rems.db.form :as form]
             [rems.db.licenses :as licenses]
             [rems.db.users :as users]
             [rems.permissions :as permissions]
@@ -112,6 +113,17 @@
 
 ;;; Legacy form stuff
 
+(defn- assoc-field-value [field form-id application-id]
+  (let [query-params {:item (:id field)
+                      :form form-id
+                      :application application-id}]
+    (assoc field
+           :value
+           (cond
+             (nil? application-id) ""
+             (= "attachment" (:type field)) (or (:filename (db/get-attachment query-params)) "")
+             :else (or (:value (db/get-field-value query-params)) "")))))
+
 (defn- assoc-field-previous-values [application fields]
   (let [previous-values (:items (if (form-fields-editable? application)
                                   (:submitted-form-contents application)
@@ -225,7 +237,8 @@
          catalogue-item-ids (mapv :item (db/get-application-items {:application application-id}))
          catalogue-items (catalogue/get-localized-catalogue-items {:ids catalogue-item-ids})
          items (->> (db/get-form-items {:id form-id})
-                    (mapv #(applications/process-field application-id form-id %))
+                    (mapv form/process-field)
+                    (mapv #(assoc-field-value % form-id application-id))
                     (assoc-field-previous-values application))
          description (-> (filter #(= "description" (:type %)) items)
                          first
diff --git a/src/clj/rems/db/form.clj b/src/clj/rems/db/form.clj
index 28762dda2..f1fb79f40 100644
--- a/src/clj/rems/db/form.clj
+++ b/src/clj/rems/db/form.clj
@@ -1,5 +1,7 @@
 (ns rems.db.form
-  (:require [rems.InvalidRequestException]
+  (:require [clojure.test :refer :all]
+            [cprop.tools :refer [merge-maps]]
+            [rems.InvalidRequestException]
             [rems.db.catalogue :as catalogue]
             [rems.db.core :as db]
             [rems.json :as json]))
@@ -11,11 +13,6 @@
        (map db/assoc-expired)
        (db/apply-filters filters)))
 
-(defn get-form [id]
-  (-> (db/get-form {:id id})
-      (db/assoc-expired)
-      (update :fields json/parse-string)))
-
 (defn get-form-template [id]
   (-> (db/get-form-template {:id id})
       (db/assoc-expired)
@@ -69,3 +66,57 @@
         (db/set-form-state! command)
         (db/set-form-template-state! command)
         {:success true}))))
+
+;;; older, non-form-template code path, for fetching forms for applications
+
+(defn- process-field-options [options]
+  (->> options
+       (map (fn [{:keys [key langcode label displayorder]}]
+              {:key key
+               :label {(keyword langcode) label}
+               :displayorder displayorder}))
+       (group-by :key)
+       (map (fn [[_key options]] (apply merge-maps options))) ; merge label translations
+       (sort-by :displayorder)
+       (mapv #(select-keys % [:key :label]))))
+
+(deftest process-field-options-test
+  (is (= [{:key "yes" :label {:en "Yes" :fi "Kyllä"}}
+          {:key "no" :label {:en "No" :fi "Ei"}}]
+         (process-field-options
+          [{:itemid 9, :key "no", :langcode "en", :label "No", :displayorder 1}
+           {:itemid 9, :key "no", :langcode "fi", :label "Ei", :displayorder 1}
+           {:itemid 9, :key "yes", :langcode "en", :label "Yes", :displayorder 0}
+           {:itemid 9, :key "yes", :langcode "fi", :label "Kyllä", :displayorder 0}]))))
+
+;; TODO figure out a better name
+(defn process-field
+  "Returns a field structure like this:
+
+    {:id 123
+     :type \"texta\"
+     :title \"Item title\"
+     :inputprompt \"hello\"
+     :optional true
+     :value \"filled value or nil\"}"
+  [field]
+  {:id (:id field)
+   :optional (:formitemoptional field)
+   :type (:type field)
+   ;; TODO here we do a db call per item, for licenses we do one huge
+   ;; db call. Not sure which is better?
+   :localizations (into {} (for [{:keys [langcode title inputprompt]}
+                                 (db/get-form-item-localizations {:item (:id field)})]
+                             [(keyword langcode) {:title title :inputprompt inputprompt}]))
+   :options (process-field-options (db/get-form-item-options {:item (:id field)}))
+   :maxlength (:maxlength field)})
+
+(defn get-form [form-id]
+  (-> (db/get-form {:id form-id})
+      (db/assoc-expired)
+      (select-keys [:id :organization :title :start :end])
+      (assoc :items (->> (db/get-form-items {:id form-id})
+                         (mapv process-field)))))
+
+(comment
+  (get-form 1))


Test patch:
diff --git a/src/clj/rems/db/test_data.clj b/src/clj/rems/db/test_data.clj
index 53552740e..3693e87e4 100644
--- a/src/clj/rems/db/test_data.clj
+++ b/src/clj/rems/db/test_data.clj
@@ -709,9 +709,9 @@
                         :actor user-id
                         :time (time/now)
                         :application-id app-id
-                        :field-values [{:field (:id (first (:fields form)))
+                        :field-values [{:field (:id (first (:items form)))
                                         :value (str "Performance test application " (UUID/randomUUID))}
-                                       {:field (:id (second (:fields form)))
+                                       {:field (:id (second (:items form)))
                                         ;; 5000 characters (10 KB) of lorem ipsum generated with www.lipsum.com
                                         ;; to increase the memory requirements of an application
                                         :value (str "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut non diam vel erat dapibus facilisis vel vitae nunc. Curabitur at fermentum lorem. Cras et bibendum ante. Etiam convallis erat justo. Phasellus cursus molestie vehicula. Etiam molestie tellus vitae consectetur dignissim. Pellentesque euismod hendrerit mi sed tincidunt. Integer quis lorem ut ipsum egestas hendrerit. Aenean est nunc, mattis euismod erat in, sodales rutrum mauris. Praesent sit amet risus quis felis congue ultricies. Nulla facilisi. Sed mollis justo id tristique volutpat.\n\nPhasellus augue mi, facilisis ac velit et, pharetra tristique nunc. Pellentesque eget arcu quam. Curabitur dictum nulla varius hendrerit varius. Proin vulputate, ex lacinia commodo varius, ipsum velit viverra est, eget molestie dui nisi non eros. Nulla lobortis odio a magna mollis placerat. Interdum et malesuada fames ac ante ipsum primis in faucibus. Integer consectetur libero ut gravida ullamcorper. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Donec aliquam feugiat mollis. Quisque massa lacus, efficitur vel justo vel, elementum mollis magna. Maecenas at sem sem. Praesent sed ex mattis, egestas dui non, volutpat lorem. Nulla tempor, nisi rutrum accumsan varius, tellus elit faucibus nulla, vel mattis lacus justo at ante. Sed ut mollis ex, sed tincidunt ex.\n\nMauris laoreet nibh eget erat tincidunt pharetra. Aenean sagittis maximus consectetur. Curabitur interdum nibh sed tincidunt finibus. Sed blandit nec lorem at iaculis. Morbi non augue nec tortor hendrerit mollis ut non arcu. Suspendisse maximus nec ligula a efficitur. Etiam ultrices rhoncus leo quis dapibus. Integer vel rhoncus est. Integer blandit varius auctor. Vestibulum suscipit suscipit risus, sit amet venenatis lacus iaculis a. Duis eu turpis sit amet nibh sagittis convallis at quis ligula. Sed eget justo quis risus iaculis lacinia vitae a justo. In hac habitasse platea dictumst. Maecenas euismod et lorem vel viverra.\n\nDonec bibendum nec ipsum in volutpat. Vivamus in elit venenatis, venenatis libero ac, ultrices dolor. Morbi quis odio in neque consequat rutrum. Suspendisse quis sapien id sapien fermentum dignissim. Nam eu est vel risus volutpat mollis sed quis eros. Proin leo nulla, dictum id hendrerit vitae, scelerisque in elit. Proin consectetur sodales arcu ac tristique. Suspendisse ut elementum ligula, at rhoncus mauris. Aliquam lacinia at diam eget mattis. Phasellus quam leo, hendrerit sit amet mi eget, porttitor aliquet velit. Proin turpis ante, consequat in enim nec, tempus consequat magna. Vestibulum fringilla ac turpis nec malesuada. Proin id lectus iaculis, suscipit erat at, volutpat turpis. In quis faucibus elit, ut maximus nibh. Sed egestas egestas dolor.\n\nNulla varius orci quam, id auctor enim ultrices nec. Morbi et tellus ac metus sodales convallis sed vehicula neque. Pellentesque rhoncus mattis massa a bibendum. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Fusce tincidunt nulla non aliquet facilisis. Praesent nisl nisi, finibus id odio sed, consectetur feugiat mauris. Suspendisse sed lacinia ligula. Duis vitae nisl leo. Donec erat arcu, feugiat sit amet sagittis ac, scelerisque nec est. Pellentesque finibus mauris nulla, in maximus sapien pharetra vitae. Sed leo elit, consequat eu aliquam vitae, feugiat ut eros. Pellentesque dictum feugiat odio sed commodo. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin neque quam, varius vel libero sit amet, rhoncus sollicitudin ex. In a dui non neque malesuada pellentesque.\n\nProin tincidunt nisl non commodo faucibus. Sed porttitor arcu neque, vitae bibendum sapien placerat nec. Integer eget tristique orci. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Donec eu molestie eros. Nunc iaculis rhoncus enim, vel mattis felis fringilla condimentum. Interdum et malesuada fames ac ante ipsum primis in faucibus. Aenean ac augue nulla. Phasellus vitae nulla lobortis, mattis magna ac, gravida ipsum. Aenean ornare non nunc non luctus. Aenean lacinia lectus nec velit finibus egestas vel ut ipsum. Cras hendrerit rhoncus erat, vel maximus nunc.\n\nPraesent quis imperdiet quam. Praesent ligula tellus, consectetur sed lacus eu, malesuada condimentum tellus. Donec et diam hendrerit, dictum diam quis, aliquet purus. Suspendisse pulvinar neque at efficitur iaculis. Nulla erat orci, euismod id velit sed, dictum hendrerit arcu. Nulla aliquam molestie aliquam. Duis et semper nisi, eget commodo arcu. Praesent rhoncus, nulla id sodales eleifend, ante ipsum pellentesque augue, id iaculis sem est vitae est. Phasellus cursus diam a lorem vestibulum sodales. Nullam lacinia tortor vel tellus commodo, sit amet sodales quam malesuada.\n\nNulla tempor lectus vel arcu feugiat, vel dapibus ex dapibus. Maecenas purus justo, aliquet et sem sit amet, tincidunt venenatis dui. Nulla eget purus id sapien elementum rutrum eu vel libero. Cras non accumsan justo posuere.\n\n"
diff --git a/test/clj/rems/api/test_applications.clj b/test/clj/rems/api/test_applications.clj
index a9d01314f..58052e4a3 100644
--- a/test/clj/rems/api/test_applications.clj
+++ b/test/clj/rems/api/test_applications.clj
@@ -388,7 +388,7 @@
                                            :type "text"
                                            :optional true}])
         [req-id opt-id] (->> (form/get-form form-id)
-                             :fields
+                             :items
                              (map :id))
         cat-id (create-catalogue-item form-id workflow-id)
         app-id (create-application [cat-id] user-id)]
@@ -434,7 +434,7 @@
         form-id (create-form-with-fields [{:title {:en "some attachment"}
                                            :type "attachment"
                                            :optional true}])
-        field-id (-> (form/get-form form-id) :fields first :id)
+        field-id (-> (form/get-form form-id) :items first :id)
         cat-id (create-catalogue-item form-id workflow-id)
         app-id (create-application [cat-id] user-id)
         upload-request (fn [file]