Repository: CSCfi/rems
Pull request #1205 (2019-05-06 12:16:06)

refactor application attachment storage (BREAKING change)
for #1175 


Patch:
diff --git a/resources/migrations/20190506081718-new-attachment-table.down.sql b/resources/migrations/20190506081718-new-attachment-table.down.sql
new file mode 100644
index 000000000..315f3984f
--- /dev/null
+++ b/resources/migrations/20190506081718-new-attachment-table.down.sql
@@ -0,0 +1,16 @@
+-- Can't recreate data removed in up migration
+DROP TABLE IF EXISTS attachment CASCADE;
+--;;
+CREATE TABLE application_attachments (
+    id serial NOT NULL PRIMARY KEY,
+    catAppId integer DEFAULT NULL,
+    formMapId integer DEFAULT NULL,
+    modifierUserId varchar(255) NOT NULL,
+    filename varchar(255) NOT NULL,
+    type varchar(255) NOT NULL,
+    data bytea NOT NULL,
+    start timestamp with time zone DEFAULT CURRENT_TIMESTAMP,
+    CONSTRAINT application_attachments_ibfk_1 FOREIGN KEY (catAppId) REFERENCES catalogue_item_application (id),
+    CONSTRAINT application_attachments_ibfk_2 FOREIGN KEY (formMapId) REFERENCES application_form_item_map (id),
+    UNIQUE (catAppId, formMapId)
+);
diff --git a/resources/migrations/20190506081718-new-attachment-table.up.sql b/resources/migrations/20190506081718-new-attachment-table.up.sql
new file mode 100644
index 000000000..ab6ae1262
--- /dev/null
+++ b/resources/migrations/20190506081718-new-attachment-table.up.sql
@@ -0,0 +1,13 @@
+-- No migration for data!
+CREATE TABLE attachment (
+    id serial NOT NULL PRIMARY KEY,
+    appId integer NOT NULL, -- link to application for access control
+    modifierUserId varchar(255) NOT NULL,
+    filename varchar(255) NOT NULL,
+    type varchar(255) NOT NULL,
+    data bytea NOT NULL,
+    CONSTRAINT attachment_appid_fkey FOREIGN KEY (appId) REFERENCES catalogue_item_application (id)
+);
+--;;
+DROP TABLE IF EXISTS application_attachments CASCADE;
+--;;
diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql
index 41f05afb5..76e912d6c 100644
--- a/resources/sql/queries.sql
+++ b/resources/sql/queries.sql
@@ -374,29 +374,19 @@ ON CONFLICT (catAppId, formMapId)
 DO UPDATE
 SET (modifierUserId, value) = (:user, :value);
 
--- :name save-attachment! :!
-INSERT INTO application_attachments
-(catAppId, modifierUserId, filename, type, data, formMapId)
+-- :name save-attachment! :insert
+INSERT INTO attachment
+(appId, modifierUserId, filename, type, data)
 VALUES
-(:application, :user, :filename, :type, :data,
- (SELECT id FROM application_form_item_map
-  WHERE formId = :form AND formItemId = :item))
-ON CONFLICT (catAppId, formMapId)
-DO UPDATE
-SET (modifierUserId, filename, type, data) = (:user, :filename, :type, :data);
-
--- :name remove-attachment! :!
-DELETE FROM application_attachments
-WHERE catAppId = :application
-AND formmapid = (SELECT id FROM application_form_item_map
-                 WHERE formId = :form AND formItemId = :item);
+(:application, :user, :filename, :type, :data)
 
 -- :name get-attachment :? :1
-SELECT filename, type, data FROM application_attachments attachments
-LEFT OUTER JOIN application_form_item_map itemmap ON attachments.formMapId = itemmap.id
-WHERE attachments.catAppId = :application
-  AND itemmap.formItemId = :item
-  AND itemmap.formId = :form;
+SELECT appid, filename, type, data FROM attachment
+WHERE id = :id
+
+-- :name get-attachments-for-application :? :*
+SELECT id, filename, type, modifierUserId FROM attachment
+WHERE appid = :application-id
 
 -- :name save-license-approval! :!
 -- NB: this is not atomic
diff --git a/src/clj/rems/api/applications.clj b/src/clj/rems/api/applications.clj
index 604375679..0a6c55667 100644
--- a/src/clj/rems/api/applications.clj
+++ b/src/clj/rems/api/applications.clj
@@ -4,6 +4,7 @@
             [compojure.api.sweet :refer :all]
             [rems.api.schema :refer :all]
             [rems.api.util :as api-util]
+            [rems.application-util :as application-util]
             [rems.application.commands :as commands]
             [rems.auth.util :refer [throw-forbidden]]
             [rems.db.applications :as applications]
@@ -61,6 +62,10 @@
    (s/optional-key :application-id) s/Num
    (s/optional-key :errors) [s/Any]})
 
+(s/defschema SaveAttachmentResponse
+  (merge SuccessResponse
+         {(s/optional-key :id) s/Num}))
+
 ;; Api implementation
 
 (defn invalid-user? [u]
@@ -193,36 +198,31 @@
       :return Deciders
       (ok (get-deciders)))
 
-    (GET "/attachments" []
-      :summary "Get an attachment for a field in an application"
+    (GET "/attachment/:attachment-id" []
+      :summary "Get an attachment"
       :roles #{:logged-in}
-      :query-params [application-id :- (describe s/Int "application id")
-                     field-id :- (describe s/Int "application form field id the attachment is related to")]
-      (if-let [attachment (attachments/get-attachment (getx-user-id) application-id field-id)]
-        (-> (ok (:data attachment))
-            (content-type (:content-type attachment)))
-        (api-util/not-found-json-response)))
+      :path-params [attachment-id :- (describe s/Int "attachment id")]
+      (let [attachment (attachments/get-attachment attachment-id)
+            application-id (:application/id attachment)]
+        (when application-id
+          (applications/get-application (getx-user-id) application-id)) ;; check that user is allowed to read application
+        (if attachment
+          (-> (ok (ByteArrayInputStream. (:attachment/data attachment)))
+              (content-type (:attachment/type attachment)))
+          (api-util/not-found-json-response))))
 
     ;; TODO: think about size limit
     (POST "/add-attachment" []
-      :summary "Add an attachment file related to an application field"
+      :summary "Add an attachment file related to an application"
       :roles #{:applicant}
       :multipart-params [file :- upload/TempFileUpload]
-      :query-params [application-id :- (describe s/Int "application id")
-                     field-id :- (describe s/Int "application form field id the attachment is related to")]
+      :query-params [application-id :- (describe s/Int "application id")]
       :middleware [multipart/wrap-multipart-params]
-      :return SuccessResponse
-      (attachments/save-attachment! file (getx-user-id) application-id field-id)
-      (ok {:success true}))
-
-    (POST "/remove-attachment" []
-      :summary "Remove an attachment file related to an application field"
-      :roles #{:applicant}
-      :query-params [application-id :- (describe s/Int "application id")
-                     field-id :- (describe s/Int "application form field id the attachment is related to")]
-      :return SuccessResponse
-      (attachments/remove-attachment! (getx-user-id) application-id field-id)
-      (ok {:success true}))
+      :return SaveAttachmentResponse
+      (let [application (applications/get-application (getx-user-id) application-id)]
+        (when-not (application-util/form-fields-editable? application)
+          (throw-forbidden))
+        (ok (attachments/save-attachment! file (getx-user-id) application-id))))
 
     (POST "/accept-invitation" []
       :summary "Accept an invitation by token"
diff --git a/src/clj/rems/api/licenses.clj b/src/clj/rems/api/licenses.clj
index c94055392..e2592a5d3 100644
--- a/src/clj/rems/api/licenses.clj
+++ b/src/clj/rems/api/licenses.clj
@@ -2,6 +2,7 @@
   (:require [compojure.api.sweet :refer :all]
             [rems.api.schema :refer :all]
             [rems.api.util :as api-util]
+            [rems.db.attachments :as attachments]
             [rems.db.core :as db]
             [rems.db.licenses :as licenses]
             [rems.util :refer [getx-user-id]]
@@ -22,20 +23,6 @@
 (s/defschema AttachmentMetadata
   {:id s/Num})
 
-(defn- check-attachment-content-type
-  "Checks that content-type matches the allowed ones listed on the UI side:
-   .pdf, .doc, .docx, .ppt, .pptx, .txt, image/*"
-  [content-type]
-  (when-not (or (#{"application/pdf"
-                   "application/msword"
-                   "application/vnd.openxmlformats-officedocument.wordprocessingml.document"
-                   "application/vnd.ms-powerpoint"
-                   "application/vnd.openxmlformats-officedocument.presentationml.presentation"
-                   "text/plain"}
-                 content-type)
-                (.startsWith content-type "image/"))
-    (throw (rems.InvalidRequestException. (str "Unsupported content-type: " content-type)))))
-
 (s/defschema CreateLicenseResponse
   {:success s/Bool
    :id s/Num})
@@ -82,7 +69,7 @@
       :multipart-params [file :- upload/TempFileUpload]
       :middleware [multipart/wrap-multipart-params]
       :return AttachmentMetadata
-      (check-attachment-content-type (:content-type file))
+      (attachments/check-attachment-content-type (:content-type file))
       (ok (licenses/create-license-attachment! file (getx-user-id))))
 
     (POST "/remove_attachment" []
@@ -99,7 +86,7 @@
       :roles #{:owner}
       :path-params [attachment-id :- (describe s/Int "attachment id")]
       (if-let [attachment (db/get-license-attachment {:attachmentId attachment-id})]
-        (do (check-attachment-content-type (:type attachment))
+        (do (attachments/check-attachment-content-type (:type attachment))
             (-> (:data attachment)
                 (java.io.ByteArrayInputStream.)
                 (ok)
diff --git a/src/clj/rems/api/schema.clj b/src/clj/rems/api/schema.clj
index 99720937b..e2222cf5f 100644
--- a/src/clj/rems/api/schema.clj
+++ b/src/clj/rems/api/schema.clj
@@ -124,6 +124,11 @@
    :form/title s/Str
    :form/fields [Field]})
 
+(s/defschema ApplicationAttachment
+  {:attachment/id s/Num
+   :attachment/filename s/Str
+   :attachment/type s/Str})
+
 (s/defschema Application
   {:application/id s/Int
    :application/external-id (s/maybe s/Str)
@@ -147,7 +152,8 @@
                           :workflow/type s/Keyword
                           (s/optional-key :workflow.dynamic/handlers) #{s/Str}}
    :application/roles #{s/Keyword}
-   :application/permissions #{s/Keyword}})
+   :application/permissions #{s/Keyword}
+   :application/attachments [ApplicationAttachment]})
 
 (s/defschema ApplicationOverview
   (dissoc Application
diff --git a/src/clj/rems/application/model.clj b/src/clj/rems/application/model.clj
index e41c21cc3..6a6337333 100644
--- a/src/clj/rems/application/model.clj
+++ b/src/clj/rems/application/model.clj
@@ -3,7 +3,8 @@
             [clojure.test :refer [deftest is testing]]
             [medley.core :refer [map-vals]]
             [rems.application.events :as events]
-            [rems.permissions :as permissions]))
+            [rems.permissions :as permissions]
+            [rems.util :refer [getx]]))
 
 ;;;; Roles & Permissions
 
@@ -492,7 +493,8 @@
       (permissions/give-role-to-users :reporter (get-users-with-role :reporter))))
 
 (defn enrich-with-injections [application {:keys [get-form get-catalogue-item get-license
-                                                  get-user get-users-with-role get-workflow]}]
+                                                  get-user get-users-with-role get-workflow
+                                                  get-attachments-for-application]}]
   (let [answer-versions (remove nil? [(::draft-answers application)
                                       (::submitted-answers application)
                                       (::previous-submitted-answers application)])
@@ -514,6 +516,7 @@
         (update :application/resources enrich-resources get-catalogue-item)
         (update :application/licenses enrich-licenses get-license)
         (assoc :application/applicant-attributes (get-user (:application/applicant application)))
+        (assoc :application/attachments (get-attachments-for-application (getx application :application/id)))
         (enrich-user-attributes get-user)
         (enrich-workflow-handlers get-workflow)
         (enrich-super-users get-users-with-role))))
diff --git a/src/clj/rems/db/applications.clj b/src/clj/rems/db/applications.clj
index a1f9e16ec..713b46240 100644
--- a/src/clj/rems/db/applications.clj
+++ b/src/clj/rems/db/applications.clj
@@ -11,6 +11,7 @@
             [rems.application.events-cache :as events-cache]
             [rems.application.model :as model]
             [rems.auth.util :refer [throw-forbidden]]
+            [rems.db.attachments :as attachments]
             [rems.db.catalogue :as catalogue]
             [rems.db.core :as db]
             [rems.db.entitlements :as entitlements]
@@ -156,7 +157,8 @@
 ;;; Fetching applications (for API) (incl. caching)
 
 (def ^:private injections
-  {:get-form form/get-form
+  {:get-attachments-for-application attachments/get-attachments-for-application
+   :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/legacy.clj b/src/clj/rems/db/applications/legacy.clj
index 9c83763d9..5e66c79b5 100644
--- a/src/clj/rems/db/applications/legacy.clj
+++ b/src/clj/rems/db/applications/legacy.clj
@@ -122,7 +122,7 @@
            :value
            (cond
              (nil? application-id) ""
-             (= "attachment" (:type field)) (or (:filename (db/get-attachment query-params)) "")
+             (= "attachment" (:type field)) "" ;; there used to be support for legacy attachments here, but not any more
              :else (or (:value (db/get-field-value query-params)) "")))))
 
 (defn- assoc-field-previous-values [application fields]
diff --git a/src/clj/rems/db/attachments.clj b/src/clj/rems/db/attachments.clj
index 2802db41d..f55e4aa66 100644
--- a/src/clj/rems/db/attachments.clj
+++ b/src/clj/rems/db/attachments.clj
@@ -1,12 +1,12 @@
 (ns rems.db.attachments
   (:require [rems.application-util :refer [form-fields-editable?]]
-            [rems.db.applications :as applications]
+            [rems.InvalidRequestException]
             [rems.auth.util :refer [throw-forbidden]]
             [rems.db.core :as db])
   (:import [java.io ByteArrayOutputStream FileInputStream File ByteArrayInputStream]
-           [rems InvalidRequestException]))
+           rems.InvalidRequestException))
 
-(defn- check-attachment-content-type
+(defn check-attachment-content-type
   "Checks that content-type matches the allowed ones listed on the UI side:
    .pdf, .doc, .docx, .ppt, .pptx, .txt, image/*"
   [content-type]
@@ -21,38 +21,31 @@
     (throw (InvalidRequestException. (str "Unsupported content-type: " content-type)))))
 
 (defn save-attachment!
-  [{:keys [tempfile filename content-type]} user-id application-id item-id]
+  [{:keys [tempfile filename content-type]} user-id application-id]
   (check-attachment-content-type content-type)
-  (let [application (applications/get-application user-id application-id)
-        byte-array (with-open [input (FileInputStream. ^File tempfile)
+  (let [byte-array (with-open [input (FileInputStream. ^File tempfile)
                                buffer (ByteArrayOutputStream.)]
                      (clojure.java.io/copy input buffer)
-                     (.toByteArray buffer))]
-    (when-not (form-fields-editable? application)
-      (throw-forbidden))
-    (db/save-attachment! {:application application-id
-                          :form (get-in application [:application/form :form/id])
-                          :item item-id
-                          :user user-id
-                          :filename filename
-                          :type content-type
-                          :data byte-array})))
+                     (.toByteArray buffer))
+        id (:id (db/save-attachment! {:application application-id
+                                      :user user-id
+                                      :filename filename
+                                      :type content-type
+                                      :data byte-array}))]
+    {:id id
+     :success true}))
 
-(defn remove-attachment! [user-id application-id item-id]
-  (let [application (applications/get-application user-id application-id)]
-    (when-not (form-fields-editable? application)
-      (throw-forbidden))
-    (db/remove-attachment! {:application application-id
-                            :form (get-in application [:application/form :form/id])
-                            :item item-id})))
+(defn get-attachment [attachment-id]
+  (when-let [{:keys [type appid filename data]} (db/get-attachment {:id attachment-id})]
+    (check-attachment-content-type type)
+    {:application/id appid
+     :attachment/filename filename
+     :attachment/data data
+     :attachment/type type}))
 
-(defn get-attachment [user-id application-id field-id]
-  (let [application (applications/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
-                                              :application application-id})]
-      (check-attachment-content-type (:type attachment))
-      {:data (-> (:data attachment)
-                 (ByteArrayInputStream.))
-       :content-type (:type attachment)})))
+(defn get-attachments-for-application [application-id]
+  (vec
+   (for [{:keys [id filename type]} (db/get-attachments-for-application {:application-id application-id})]
+     {:attachment/id id
+      :attachment/filename filename
+      :attachment/type type})))
diff --git a/src/cljs/rems/application.cljs b/src/cljs/rems/application.cljs
index 7264659e0..badad2647 100644
--- a/src/cljs/rems/application.cljs
+++ b/src/cljs/rems/application.cljs
@@ -27,7 +27,7 @@
             [rems.spinner :as spinner]
             [rems.status-modal :as status-modal]
             [rems.text :refer [localize-decision localize-event localized localize-item localize-state localize-time text text-format]]
-            [rems.util :refer [dispatch! fetch post!]])
+            [rems.util :refer [dispatch! fetch parse-int post!]])
   (:require-macros [rems.guide-macros :refer [component-info example]]))
 
 ;;;; Helpers
@@ -195,37 +195,20 @@
            {:url-params {:application-id application-id
                          :field-id field-id}
             :body file
-            ;; adding an attachment and not clicking [Save] will leave
-            ;; a dangling attachment in the db. consider forcing a
-            ;; save here. see also comment in remove-attachment
-            :handler (partial status-modal/common-success-handler! (fn []))
-            :error-handler status-modal/common-error-handler!})
-    {}))
-
-(rf/reg-event-fx ::save-attachment save-attachment)
-
-(defn- remove-attachment [{:keys [db]} [_ field-id description]]
-  (let [application-id (get-in db [::application :application/id])]
-    (status-modal/common-pending-handler! description)
-    (post! "/api/applications/remove-attachment"
-           {:url-params {:application-id application-id
-                         :field-id field-id}
-            :body {}
+            ;; force saving a draft when you upload an attachment.
+            ;; this ensures that the attachment is not left
+            ;; dangling (with no references to it)
             :handler (fn [response]
-                       ;; if we just remove the attachment from the backend but
-                       ;; don't save the field, the application will be left in an
-                       ;; inconsistent state (referring to a nonexistant attachment)
-                       ;; TODO: save only the value for the attachment field
                        (if (:success response)
-                         (rf/dispatch [::save-application (text :t.form/save)])
+                         (do
+                           ;; no race condition here: events are handled in a FIFO manner
+                           (rf/dispatch [::set-field-value field-id (str (:id response))])
+                           (rf/dispatch [::save-application (text :t.form/upload)]))
                          (status-modal/common-error-handler! response)))
             :error-handler status-modal/common-error-handler!})
     {}))
 
-(rf/reg-event-fx ::remove-attachment remove-attachment)
-
-
-
+(rf/reg-event-fx ::save-attachment save-attachment)
 
 ;;;; UI components
 
@@ -298,6 +281,7 @@
   (let [field-values (:field-values edit-application)
         show-diff (:show-diff edit-application)
         field-validations (index-by [:field-id] (:validation-errors edit-application))
+        attachments (index-by [:attachment/id] (:application/attachments application))
         form-fields-editable? (form-fields-editable? application)
         readonly? (not form-fields-editable?)]
     [collapsible/component
@@ -310,8 +294,10 @@
                [fields/field (assoc fld
                                     :on-change #(rf/dispatch [::set-field-value (:field/id fld) %])
                                     :on-set-attachment #(rf/dispatch [::save-attachment (:field/id fld) %1 %2])
-                                    :on-remove-attachment #(rf/dispatch [::remove-attachment (:field/id fld) %])
+                                    :on-remove-attachment #(rf/dispatch [::set-field-value (:field/id fld) ""])
                                     :on-toggle-diff #(rf/dispatch [::toggle-diff (:field/id fld)])
+                                    :field/attachment (when (= :attachment (:field/type fld))
+                                                        (get attachments (parse-int (:field/value fld))))
                                     :field/value (get field-values (:field/id fld))
                                     :diff (get show-diff (:field/id fld))
                                     :validation (field-validations (:field/id fld))
diff --git a/src/cljs/rems/fields.cljs b/src/cljs/rems/fields.cljs
index 58374c9b8..86f6680a6 100644
--- a/src/cljs/rems/fields.cljs
+++ b/src/cljs/rems/fields.cljs
@@ -211,12 +211,13 @@
   (let [id (:field/id opts)
         title (localized (:field/title opts))
         value (:field/value opts)
+        filename (get-in opts [:field/attachment :attachment/filename])
         click-upload (fn [e] (when-not (:readonly opts) (.click (.getElementById js/document (id-to-name id)))))
         filename-field [:div.field
                         [:a.btn.btn-secondary.mr-2
-                         {:href (str "/api/applications/attachments?application-id=" app-id "&field-id=" id)
+                         {:href (str "/api/applications/attachment/" value)
                           :target :_new}
-                         value " " (external-link)]]
+                         filename " " (external-link)]]
         upload-field [:div.upload-file.mr-2
                       [:input {:style {:display "none"}
                                :type "file"
@@ -236,7 +237,7 @@
         remove-button [:button.btn.btn-secondary.mr-2
                        {:on-click (fn [event]
                                     (on-change "")
-                                    (on-remove-attachment (text :t.form/attachment-remove)))}
+                                    (on-remove-attachment))}
                        (text :t.form/attachment-remove)]]
     [field-wrapper (assoc opts :readonly-component (if (empty? value)
                                                      [:span]


Test patch:
diff --git a/test/clj/rems/api/test_applications.clj b/test/clj/rems/api/test_applications.clj
index 58052e4a3..143222296 100644
--- a/test/clj/rems/api/test_applications.clj
+++ b/test/clj/rems/api/test_applications.clj
@@ -434,31 +434,49 @@
         form-id (create-form-with-fields [{:title {:en "some attachment"}
                                            :type "attachment"
                                            :optional true}])
-        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]
-                         (-> (request :post (str "/api/applications/add-attachment?application-id=" app-id "&field-id=" field-id))
+                         (-> (request :post (str "/api/applications/add-attachment?application-id=" app-id))
                              (assoc :params {"file" file})
                              (assoc :multipart-params {"file" file})))
-        read-request (request :get "/api/applications/attachments" {:application-id app-id :field-id field-id})]
+        read-request #(request :get (str "/api/applications/attachment/" %))]
     (testing "uploading attachment for a draft"
       (let [body (-> (upload-request filecontent)
                      (authenticate api-key user-id)
                      handler
-                     read-ok-body)]
-        (is (= {:success true} body))))
+                     read-ok-body)
+            id (:id body)]
+        (is (:success body))
+        (is (number? id))
+        (testing "and retrieving it as the applicant"
+          (let [response (-> (read-request id)
+                             (authenticate api-key user-id)
+                             handler
+                             assert-response-is-ok)]
+            (is (= (slurp testfile) (slurp (:body response))))))
+        (testing "and retrieving it as non-applicant"
+          (let [response (-> (read-request id)
+                             (authenticate api-key "carl")
+                             handler)]
+            (is (response-is-forbidden? response))))))
+    (testing "retrieving nonexistent attachment"
+      (let [response (-> (read-request 999999999999999)
+                         (authenticate api-key "carl")
+                         handler)]
+        (is (response-is-not-found? response))))
+    (testing "uploading attachment for nonexistent application"
+      (let [response (-> (request :post "/api/applications/add-attachment?application-id=99999999")
+                         (assoc :params {"file" filecontent})
+                         (assoc :multipart-params {"file" filecontent})
+                         (authenticate api-key user-id)
+                         handler)]
+        (is (response-is-forbidden? response))))
     (testing "uploading malicious file for a draft"
       (let [response (-> (upload-request malicious-content)
                          (authenticate api-key user-id)
                          handler)]
         (is (response-is-bad-request? response))))
-    (testing "retrieving attachment for a draft"
-      (let [response (-> read-request
-                         (authenticate api-key user-id)
-                         handler
-                         assert-response-is-ok)]
-        (is (= (slurp testfile) (slurp (:body response))))))
     (testing "uploading attachment without authentication"
       (let [response (-> (upload-request filecontent)
                          handler)]
@@ -474,11 +492,6 @@
                          (authenticate api-key "carl")
                          handler)]
         (is (response-is-forbidden? response))))
-    (testing "retrieving attachment as non-applicant"
-      (let [response (-> read-request
-                         (authenticate api-key "carl")
-                         handler)]
-        (is (response-is-forbidden? response))))
     (testing "uploading attachment for a submitted application"
       (assert (= {:success true} (send-command user-id {:type :application.command/submit
                                                         :application-id app-id})))
diff --git a/test/clj/rems/application/test_model.clj b/test/clj/rems/application/test_model.clj
index 21612b831..b60bad8a3 100644
--- a/test/clj/rems/application/test_model.clj
+++ b/test/clj/rems/application/test_model.clj
@@ -174,13 +174,18 @@
        :archived false
        :expired false}})
 
+;; no attachments here for now
+(defn ^:private get-attachments-for-application [id]
+  [])
+
 (deftest test-application-view
   (let [injections {:get-form get-form
                     :get-catalogue-item get-catalogue-item
                     :get-license get-license
                     :get-user get-user
                     :get-users-with-role get-users-with-role
-                    :get-workflow get-workflow}
+                    :get-workflow get-workflow
+                    :get-attachments-for-application get-attachments-for-application}
         apply-events (fn [events]
                        (let [application (-> events
                                              events/validate-events
@@ -304,6 +309,7 @@
                                                                     :field/optional false
                                                                     :field/options []
                                                                     :field/max-length 100}]}
+                                  :application/attachments []
                                   :application/workflow {:workflow/id 50
                                                          :workflow/type :workflow/dynamic
                                                          :workflow.dynamic/handlers #{"handler"}