Repository: CSCfi/rems
Pull request #1305 (2019-05-29 12:31:25)

misc form code cleanup
for #913


Patch:
diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql
index 0ea847cbf..6dcbbac4b 100644
--- a/resources/sql/queries.sql
+++ b/resources/sql/queries.sql
@@ -199,6 +199,18 @@ WHERE 1=1
 /*~ ) ~*/
 ;
 
+-- :name get-form-templates :? :*
+SELECT
+  id,
+  organization,
+  title,
+  start,
+  endt as "end",
+  fields::TEXT,
+  enabled,
+  archived
+FROM form_template;
+
 -- :name get-form-template :? :1
 SELECT
   id,
diff --git a/src/clj/rems/api/forms.clj b/src/clj/rems/api/forms.clj
index b0a060d4e..91abd9e25 100644
--- a/src/clj/rems/api/forms.clj
+++ b/src/clj/rems/api/forms.clj
@@ -51,9 +51,9 @@
    :enabled enabled
    :archived archived})
 
-(defn- get-forms [filters]
+(defn- get-form-templates [filters]
   (doall
-   (for [wf (form/get-forms filters)]
+   (for [wf (form/get-form-templates filters)]
      (format-form wf))))
 
 (s/defschema CreateFormCommand
@@ -76,9 +76,9 @@
                      {expired :- (describe s/Bool "whether to include expired forms") false}
                      {archived :- (describe s/Bool "whether to include archived forms") false}]
       :return Forms
-      (ok (get-forms (merge (when-not expired {:expired false})
-                            (when-not disabled {:enabled true})
-                            (when-not archived {:archived false})))))
+      (ok (get-form-templates (merge (when-not expired {:expired false})
+                                     (when-not disabled {:enabled true})
+                                     (when-not archived {:archived false})))))
 
     (GET "/:form-id" []
       :summary "Get form by id"
diff --git a/src/clj/rems/application/model.clj b/src/clj/rems/application/model.clj
index 2b7a86296..f43d85b56 100644
--- a/src/clj/rems/application/model.clj
+++ b/src/clj/rems/application/model.clj
@@ -398,8 +398,8 @@
          (localization-for :title {:localizations {:en {}
                                                    :fi {}}}))))
 
-(defn- enrich-form [app-form get-form]
-  (let [form (get-form (:form/id app-form))
+(defn- enrich-form [app-form get-form-template]
+  (let [form (get-form-template (:form/id app-form))
         app-fields (:form/fields app-form)
         rich-fields (map (fn [field]
                            {:field/id (:id field)
diff --git a/src/clj/rems/db/applications/legacy.clj b/src/clj/rems/db/applications/legacy.clj
index 1f499ccb5..3b9c78dd8 100644
--- a/src/clj/rems/db/applications/legacy.clj
+++ b/src/clj/rems/db/applications/legacy.clj
@@ -2,6 +2,7 @@
   "Functions for dealing with old round-based applications."
   (:require [clojure.test :refer :all]
             [clj-time.core :as time]
+            [cprop.tools :refer [merge-maps]]
             [rems.application.commands :as commands]
             [rems.application.model :as model]
             [rems.application-util :refer [form-fields-editable?]]
@@ -213,6 +214,47 @@
     (assert (applications/is-dynamic-application? application) (pr-str application))
     (apply-dynamic-events application events)))
 
+;;; reading legacy form tables
+
+(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\"}"
+  [field]
+  {:id (:id field)
+   :optional (:formitemoptional field)
+   :type (:type field)
+   :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)})
+
 ;;; The main entry point, get-form-for
 
 (defn get-form-for
@@ -266,7 +308,7 @@
          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 form/process-field)
+                    (mapv process-field)
                     (mapv #(assoc-field-value % form-id application-id))
                     (assoc-field-previous-values application))
          description (-> (filter #(= "description" (:type %)) items)
diff --git a/src/clj/rems/db/form.clj b/src/clj/rems/db/form.clj
index 8373ced5a..f304b2534 100644
--- a/src/clj/rems/db/form.clj
+++ b/src/clj/rems/db/form.clj
@@ -1,6 +1,5 @@
 (ns rems.db.form
   (:require [clojure.test :refer :all]
-            [cprop.tools :refer [merge-maps]]
             [rems.InvalidRequestException]
             [rems.db.catalogue :as catalogue]
             [rems.db.core :as db]
@@ -8,8 +7,8 @@
 
 ;;; form api related code – form "templates"
 
-(defn get-forms [filters]
-  (->> (db/get-forms)
+(defn get-form-templates [filters]
+  (->> (db/get-form-templates)
        (map db/assoc-expired)
        (db/apply-filters filters)))
 
@@ -73,59 +72,3 @@
         (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
-  (clojure.pprint/pprint
-   [(get-form 1)
-    (get-form-template 1)]))
diff --git a/src/clj/rems/db/form_item.clj b/src/clj/rems/db/form_item.clj
deleted file mode 100644
index 9e42c0b83..000000000
--- a/src/clj/rems/db/form_item.clj
+++ /dev/null
@@ -1,7 +0,0 @@
-(ns rems.db.form-item
-  (:require [rems.db.core :as db]))
-
-(defn get-form-items [filters]
-  (->> (db/get-all-form-items)
-       (map db/assoc-expired)
-       (db/apply-filters filters)))
diff --git a/src/clj/rems/validate.clj b/src/clj/rems/validate.clj
index bd19a825e..52bcd206b 100644
--- a/src/clj/rems/validate.clj
+++ b/src/clj/rems/validate.clj
@@ -2,20 +2,11 @@
   "Validating data in the database."
   (:require [clojure.pprint :refer [pprint]]
             [clojure.tools.logging :as log]
-            [rems.api.forms]
-            [rems.db.applications :as applications]
-            [rems.db.form :as form]
-            [schema.core :as s]))
-
-(defn validate-forms []
-  (doseq [form (form/get-forms {})]
-    (let [template (form/get-form-template (:id form))]
-      (s/validate rems.api.forms/FullForm template))))
+            [rems.db.applications :as applications]))
 
 (defn validate []
   (log/info "Validating data")
   (try
-    (validate-forms)
     ;; will throw an exception if there are non-valid events
     (applications/get-all-unrestricted-applications)
     (log/info "Validations passed")


Test patch:
diff --git a/src/clj/rems/db/test_data.clj b/src/clj/rems/db/test_data.clj
index c1fb8f04b..7caf95b48 100644
--- a/src/clj/rems/db/test_data.clj
+++ b/src/clj/rems/db/test_data.clj
@@ -674,7 +674,7 @@
                                  :type "texta"
                                  :input-prompt {:en "The purpose of the project is to..."
                                                 :fi "Projektin tarkoitus on..."}}]}))
-        form (form/get-form form-id)
+        form (form/get-form-template form-id)
         license-id (:id (licenses/create-license!
                          {:licensetype "text"
                           :title "Performance License"
@@ -713,9 +713,9 @@
                         :actor user-id
                         :time (time/now)
                         :application-id app-id
-                        :field-values [{:field (:id (first (:items form)))
+                        :field-values [{:field (:id (first (:fields form)))
                                         :value (str "Performance test application " (UUID/randomUUID))}
-                                       {:field (:id (second (:items form)))
+                                       {:field (:id (second (:fields 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 ee25a4007..33f763666 100644
--- a/test/clj/rems/api/test_applications.clj
+++ b/test/clj/rems/api/test_applications.clj
@@ -396,8 +396,8 @@
                                           {:title {:en "opt"}
                                            :type "text"
                                            :optional true}])
-        [req-id opt-id] (->> (form/get-form form-id)
-                             :items
+        [req-id opt-id] (->> (form/get-form-template form-id)
+                             :fields
                              (map :id))
         cat-id (create-catalogue-item form-id workflow-id)
         app-id (create-application [cat-id] user-id)]
diff --git a/test/clj/rems/test_db.clj b/test/clj/rems/test_db.clj
index 61f1a15b0..eedb99a2b 100644
--- a/test/clj/rems/test_db.clj
+++ b/test/clj/rems/test_db.clj
@@ -43,7 +43,7 @@
                                       :workflow-name))
             "should find same catalogue item by id")))))
 
-;; TODO port this test to look at get-form or build-application-view instead of legacy/get-form-for
+;; TODO port this test to look at get-form-template or build-application-view instead of legacy/get-form-for
 (deftest test-form
   (binding [context/*lang* :en]
     (let [uid "test-user"