Repository: CSCfi/rems
Pull request #1066 (2019-04-04 14:12:21)

Remove v1 application compatibility layer
#852 


Patch:
diff --git a/src/clj/rems/api/applications.clj b/src/clj/rems/api/applications.clj
index 9735a2f30..84dd99fe7 100644
--- a/src/clj/rems/api/applications.clj
+++ b/src/clj/rems/api/applications.clj
@@ -1,16 +1,14 @@
 (ns rems.api.applications
   (:require [clj-time.core :as time]
             [clojure.string :as str]
-            [clojure.test :refer [deftest is]]
             [compojure.api.sweet :refer :all]
-            [rems.api.applications-v2 :refer [get-user-applications-v2 api-get-application-v2 api-get-application-v1]]
+            [rems.api.applications-v2 :as applications-v2]
             [rems.api.schema :refer :all]
             [rems.api.util]
             [rems.application.commands :as commands]
             [rems.auth.util :refer [throw-forbidden]]
             [rems.db.applications :as applications]
             [rems.db.attachments :as attachments]
-            [rems.db.core :as db]
             [rems.db.users :as users]
             [rems.pdf :as pdf]
             [rems.util :refer [getx-user-id update-present]]
@@ -30,15 +28,6 @@
   {:success s/Bool
    (s/optional-key :application-id) s/Int})
 
-(s/defschema Phases
-  [{:phase s/Keyword
-    (s/optional-key :active?) s/Bool
-    (s/optional-key :approved?) s/Bool
-    (s/optional-key :closed?) s/Bool
-    (s/optional-key :completed?) s/Bool
-    (s/optional-key :rejected?) s/Bool
-    :text s/Keyword}])
-
 (s/defschema User
   {:userid s/Str
    :name (s/maybe s/Str)
@@ -150,7 +139,7 @@
       :roles #{:logged-in}
       :path-params [application-id :- (describe s/Num "application id")]
       :produces ["application/pdf"]
-      (if-let [app (api-get-application-v1 (getx-user-id) application-id)]
+      (if-let [app (applications-v2/get-application (getx-user-id) application-id)]
         (-> app
             (pdf/application-to-pdf-bytes)
             (ByteArrayInputStream.)
@@ -202,7 +191,7 @@
       :summary "Get current user's all applications"
       :roles #{:logged-in}
       :return [ApplicationOverview]
-      (ok (get-user-applications-v2 (getx-user-id))))
+      (ok (applications-v2/get-own-applications (getx-user-id))))
 
     (POST "/create" []
       :summary "Create a new application"
@@ -217,6 +206,6 @@
       :path-params [application-id :- (describe s/Num "application id")]
       :responses {200 {:schema Application}
                   404 {:schema s/Str :description "Not found"}}
-      (if-let [app (api-get-application-v2 (getx-user-id) application-id)]
+      (if-let [app (applications-v2/get-application (getx-user-id) application-id)]
         (ok app)
         (not-found! "not found")))))
diff --git a/src/clj/rems/api/applications_v2.clj b/src/clj/rems/api/applications_v2.clj
index f47785d5d..815755527 100644
--- a/src/clj/rems/api/applications_v2.clj
+++ b/src/clj/rems/api/applications_v2.clj
@@ -39,7 +39,7 @@
       nil
       (model/build-application-view events injections))))
 
-(defn api-get-application-v2
+(defn get-application
   "Returns the part of application state which the specified user
    is allowed to see. Suitable for returning from public APIs as-is."
   [user-id application-id]
@@ -50,113 +50,7 @@
               (model/apply-user-permissions user-id))
           (throw-forbidden)))))
 
-;;; v1 API compatibility layer
-
-(defn- assoc-derived-data [user-id application]
-  (assoc application
-         :can-approve? (applications/can-approve? user-id application)
-         :can-close? (applications/can-close? user-id application)
-         :can-withdraw? (applications/can-withdraw? user-id application)
-         :can-third-party-review? (applications/can-third-party-review? user-id application)
-         :is-applicant? (applications/is-applicant? user-id application)))
-
-(defn- transform-v2-to-v1 [application user-id]
-  (let [form (:application/form application)
-        workflow (:application/workflow application)
-        catalogue-items (map (fn [resource]
-                               {:id (:catalogue-item/id resource)
-                                :resource-id (:resource/id resource)
-                                :resid (:resource/ext-id resource)
-                                :wfid (:workflow/id workflow)
-                                :formid (:form/id form)
-                                :start (:catalogue-item/start resource)
-                                :end (:catalogue-item/end resource)
-                                :archived (:catalogue-item/archived resource)
-                                :enabled (:catalogue-item/enabled resource)
-                                :active (:catalogue-item/active resource)
-                                :title (:default (:catalogue-item/title resource))
-                                :localizations (into {} (for [lang (-> (set (keys (:catalogue-item/title resource)))
-                                                                       (disj :default))]
-                                                          [lang {:title (get-in resource [:catalogue-item/title lang])
-                                                                 :langcode lang
-                                                                 :id (:catalogue-item/id resource)}]))})
-                             (:application/resources application))]
-    {:id (:form/id form)
-     :title (:form/title form)
-     :catalogue-items catalogue-items
-     :applicant-attributes (:application/applicant-attributes application)
-     :application (assoc-derived-data
-                   user-id
-                   {:id (:application/id application)
-                    :formid (:form/id form)
-                    :wfid (:workflow/id workflow)
-                    :applicantuserid (:application/applicant application)
-                    :members (into [{:userid (:application/applicant application)}]
-                                   (:application/members application))
-                    :invited-members (vec (:application/invited-members application))
-                    :start (:application/created application)
-                    :last-modified (:application/last-activity application)
-                    :state (:application/state application) ; TODO: round-based workflows
-                    :description (:application/description application)
-                    :catalogue-items catalogue-items
-                    :events [] ; TODO: round-based workflows
-                    :dynamic-events (:application/events application)
-                    :workflow {:type (:workflow/type workflow)
-                               ;; TODO: add :handlers only when it exists? https://stackoverflow.com/a/16375390
-                               :handlers (vec (:workflow.dynamic/handlers workflow))}
-                    :possible-commands (:application/permissions application)
-                    :fnlround 0 ; TODO: round-based workflows
-                    :review-type nil}) ; TODO: round-based workflows
-     :phases (applications/get-application-phases (:application/state application))
-     :licenses (map (fn [license]
-                      {:id (:license/id license)
-                       :type "license"
-                       :licensetype (name (:license/type license))
-                       ;; TODO: Licenses have three different start times: license.start, workflow_licenses.start, resource_licenses.start
-                       ;;       (also catalogue_item_application_licenses.start but that table looks unused)
-                       ;;       The old API returns either workflow_licenses.start or resource_licenses.start,
-                       ;;       the new one returns license.start for now. Should we keep all three or simplify?
-                       :start (:license/start license)
-                       :end (:license/end license)
-                       :enabled (:license/enabled license)
-                       :archived (:license/archived license)
-                       :active (:license/active license)
-                       :approved (:license/accepted license)
-                       :title (:default (:license/title license))
-                       :textcontent (:default (or (:license/link license)
-                                                  (:license/text license)
-                                                  (:license/attachment-filename license)))
-                       :attachment-id (:default (:license/attachment-id license))
-                       :localizations (into {} (for [lang (-> (set (concat (keys (:license/title license))
-                                                                           (keys (:license/link license))
-                                                                           (keys (:license/text license))))
-                                                              (disj :default))]
-                                                 [lang {:title (get-in license [:license/title lang])
-                                                        :textcontent (or (get-in license [:license/link lang])
-                                                                         (get-in license [:license/text lang])
-                                                                         (get-in license [:license/attachment-filename lang]))
-                                                        :attachment-id (get-in license [:license/attachment-id lang])}]))})
-                    (:application/licenses application))
-     :accepted-licenses (:application/accepted-licenses application)
-     :items (map (fn [field]
-                   {:id (:field/id field)
-                    :type (name (:field/type field))
-                    :optional (:field/optional field)
-                    :options (:field/options field)
-                    :maxlength (:field/max-length field)
-                    :value (:field/value field)
-                    :previous-value (:field/previous-value field)
-                    :localizations (into {} (for [lang (set (concat (keys (:field/title field))
-                                                                    (keys (:field/placeholder field))))]
-                                              [lang {:title (get-in field [:field/title lang])
-                                                     :inputprompt (get-in field [:field/placeholder lang])}]))})
-                 (:form/fields form))}))
-
-(defn api-get-application-v1 [user-id application-id]
-  (when-let [v2 (api-get-application-v2 user-id application-id)]
-    (transform-v2-to-v1 v2 user-id)))
-
-;;; v2 API, listing all applications
+;;; Listing all applications
 
 (defn- all-applications-view
   "Projection for the current state of all applications."
@@ -171,7 +65,7 @@
           :application/form
           :application/licenses))
 
-(defn get-all-applications-v2 [user-id]
+(defn get-all-applications [user-id]
   ;; TODO: cache the applications and build the projection incrementally as new events are published
   (let [events (applications/get-dynamic-application-events-since 0)
         applications (reduce all-applications-view nil events)]
@@ -187,6 +81,6 @@
           :member}
         (:application/roles application)))
 
-(defn get-user-applications-v2 [user-id]
-  (->> (get-all-applications-v2 user-id)
+(defn get-own-applications [user-id]
+  (->> (get-all-applications user-id)
        (filter own-application?)))
diff --git a/src/clj/rems/api/reviews.clj b/src/clj/rems/api/reviews.clj
index 91587587f..ce9375167 100644
--- a/src/clj/rems/api/reviews.clj
+++ b/src/clj/rems/api/reviews.clj
@@ -1,14 +1,9 @@
 (ns rems.api.reviews
   (:require [compojure.api.sweet :refer :all]
-            [rems.api.applications-v2 :refer [get-all-applications-v2]]
+            [rems.api.applications-v2 :as applications-v2]
             [rems.api.schema :refer :all]
-            [rems.api.util]
-            [rems.config :refer [env]]
-            [rems.db.applications :as applications]
             [rems.util :refer [getx-user-id]]
-            [ring.util.http-response :refer :all]
-            [schema.core :as s]))
-
+            [ring.util.http-response :refer :all]))
 
 (defn- review? [application]
   (and (some #{:handler
@@ -20,7 +15,7 @@
        (not= :application.state/draft (:application/state application))))
 
 (defn get-all-reviews [user-id]
-  (->> (get-all-applications-v2 user-id)
+  (->> (applications-v2/get-all-applications user-id)
        (filter review?)))
 
 (defn- open-review? [application]
diff --git a/src/clj/rems/db/attachments.clj b/src/clj/rems/db/attachments.clj
index c9c76101e..228a45c81 100644
--- a/src/clj/rems/db/attachments.clj
+++ b/src/clj/rems/db/attachments.clj
@@ -23,7 +23,7 @@
 (defn save-attachment!
   [{:keys [tempfile filename content-type]} user-id application-id item-id]
   (check-attachment-content-type content-type)
-  (let [application (applications-v2/api-get-application-v2 user-id application-id)
+  (let [application (applications-v2/get-application user-id application-id)
         byte-array (with-open [input (FileInputStream. ^File tempfile)
                                buffer (ByteArrayOutputStream.)]
                      (clojure.java.io/copy input buffer)
@@ -39,7 +39,7 @@
                           :data byte-array})))
 
 (defn remove-attachment! [user-id application-id item-id]
-  (let [application (applications-v2/api-get-application-v2 user-id application-id)]
+  (let [application (applications-v2/get-application user-id application-id)]
     (when-not (form-fields-editable? application)
       (throw-forbidden))
     (db/remove-attachment! {:application application-id
@@ -47,7 +47,7 @@
                             :item item-id})))
 
 (defn get-attachment [user-id application-id field-id]
-  (let [application (applications-v2/api-get-application-v2 user-id application-id)
+  (let [application (applications-v2/get-application user-id application-id)
         form-id (get-in application [:application/form :form/id])]
     (when-let [attachment (db/get-attachment {:item field-id
                                               :form form-id
diff --git a/src/clj/rems/pdf.clj b/src/clj/rems/pdf.clj
index 0de9e1385..60c4df348 100644
--- a/src/clj/rems/pdf.clj
+++ b/src/clj/rems/pdf.clj
@@ -1,14 +1,14 @@
 (ns rems.pdf
   "Rendering applications as pdf"
   (:require [clj-pdf.core :refer :all]
-            [rems.context :as context]
-            [rems.text :refer [localize-event localize-state localize-time text with-language]]))
+            [rems.text :refer [localized localize-event localize-state localize-time text with-language]])
+  (:import [java.io ByteArrayOutputStream]))
 
-(defn- render-header [form]
-  (let [state (get-in form [:application :state])
-        events (get-in form [:application :events])
-        user (:applicant-attributes form)
-        catalogue-items (:catalogue-items form)]
+(defn- render-header [application]
+  (let [state (:application/state application)
+        events (:application/events application)
+        user (:application/applicant-attributes application)
+        resources (:application/resources application)]
     (list
      [:paragraph
       (text :t.applications/state)
@@ -21,63 +21,64 @@
      [:heading (text :t.form/resources)]
      (into
       [:list]
-      (for [ci catalogue-items]
+      (for [resource resources]
         [:phrase
-         (get-in ci [:localizations context/*lang* :title])
-         " (" (:resid ci) ")"]))
+         (localized (:catalogue-item/title resource))
+         " (" (:resource/ext-id resource) ")"]))
      [:heading (text :t.form/events)]
      (if (empty? events)
-       [:paragraph "–"] ;; TODO localize?
+       [:paragraph "–"]
        (into
-        [:table {:header [(text :t.form/user) (text :t.form/event) (text :t.form/comment) (text :t.form/date)]}]
-        (for [e events]
-          [(:userid e) (localize-event (:event e)) (:comment e) (localize-time (:time e))]))))))
+        [:table {:header [(text :t.form/user)
+                          (text :t.form/event)
+                          (text :t.form/comment)
+                          (text :t.form/date)]}]
+        (for [event events]
+          [(:event/actor event)
+           (localize-event (:event/type event))
+           (:application/comment event)
+           (localize-time (:event/time event))]))))))
 
 (defn- render-field [field]
   (list
-   [:heading (:title field)]
-   [:paragraph (:value field)]))
+   [:heading (localized (:field/title field))]
+   [:paragraph (:field/value field)]))
 
-(defn- apply-localization [item]
-  (merge item (get-in item [:localizations context/*lang*])))
-
-(defn- render-fields [form]
+(defn- render-fields [application]
   (apply concat
-         (for [field (:items form)]
-           (render-field (apply-localization field)))))
+         (for [field (get-in application [:application/form :form/fields])]
+           (render-field field))))
 
 (defn- render-license [license]
   ;; TODO nicer checkbox rendering
   ;; TODO license text?
   [:paragraph
-   (if (:approved license) "[x]" "[ ]")
-   (:title license)])
+   (if (:license/accepted license) "[x] " "[ ] ")
+   (localized (:license/title license))])
 
-(defn- render-licenses [form]
+(defn- render-licenses [application]
   (into
    [[:heading (text :t.form/licenses)]]
-   (for [license (:licenses form)]
-     (render-license (apply-localization license)))))
+   (for [license (:application/licenses application)]
+     (render-license license))))
 
-(defn- render-form [form]
+(defn- render-application [application]
   [{}
    [:heading (text :t.applications/application)]
-   (render-header form)
-   (render-fields form)
-   (render-licenses form)])
+   (render-header application)
+   (render-fields application)
+   (render-licenses application)])
 
-(defn application-to-pdf [form out]
-  (pdf (render-form form) out))
+(defn application-to-pdf [application out]
+  (pdf (render-application application) out))
 
-(defn application-to-pdf-bytes [form]
-  (let [out (java.io.ByteArrayOutputStream.)]
-    (pdf (render-form form) out)
+(defn application-to-pdf-bytes [application]
+  (let [out (ByteArrayOutputStream.)]
+    (application-to-pdf application out)
     (.toByteArray out)))
 
 (comment
-  (def form
-    (applications/get-form-for "developer" 9))
   (with-language :en
-    #(clojure.pprint/pprint (render-form form)))
+    #(clojure.pprint/pprint (render-application application)))
   (with-language :en
-    #(application-to-pdf form "/tmp/application.pdf")))
+    #(application-to-pdf application "/tmp/application.pdf")))


Test patch:
diff --git a/test/clj/rems/test_db.clj b/test/clj/rems/test_db.clj
index 3e662e0aa..d4f3145bb 100644
--- a/test/clj/rems/test_db.clj
+++ b/test/clj/rems/test_db.clj
@@ -217,7 +217,7 @@
                                       :application-id app-id
                                       :time (time/now)
                                       :comment ""})))
-    (is (= :application.state/approved (:application/state (applications-v2/api-get-application-v2 uid app-id))))
+    (is (= :application.state/approved (:application/state (applications-v2/get-application uid app-id))))
 
     (rems.poller.entitlements/run)
     (is (= ["resid111" "resid222"] (sort (map :resid (db/get-entitlements {:application app-id}))))