Repository: CSCfi/rems
Pull request #1457 (2019-08-05 15:45:18)

Refactor change-resources command & tests


Patch:
diff --git a/resources/translations/en.edn b/resources/translations/en.edn
index 96e520a09..a3dc6df76 100644
--- a/resources/translations/en.edn
+++ b/resources/translations/en.edn
@@ -5,10 +5,15 @@
             :accept-licenses "Accept terms of use"
             :add-licenses "Add terms of use"
             :add-member "Add member"
+            :bundling-error "Application bundling not possible."
+            :bundling-warning "The resources can't normally be bundled into one application because they have different forms or workflows."
+            :change-form-error "The resources can't be applied for with this application. Change them or make a new application."
+            :change-form-warning "The form differs from the original application."
+            :change-workflow-error "The resources can't be applied for with this application. Change them or make a new application."
+            :change-workflow-warning "The workflow differs from the original application."
             :change-resources "Change resources"
             :approve "Approve"
             :approve-reject "Approve or reject"
-            :bundling-intro [:div.intro [:p "Note that only resources that have the same application form and workflow can be bundled together."]]
             :cancel "Cancel"
             :close "Close application"
             :close-intro [:div.intro [:p "Note that if the application is closed, it cannot be opened again."]]
@@ -36,7 +41,7 @@
             :request-decision "Request decision"
             :request-selection "Send requests"
             :request-review "Request review"
-            :resources-selection "Resources included in the application:"
+            :resources-selection "Resources included in the application"
             :return "Return to applicant"
             :review "Review"
             :show-publications "Show publications"
diff --git a/resources/translations/fi.edn b/resources/translations/fi.edn
index e9ad3ea7c..0e0d49d9a 100644
--- a/resources/translations/fi.edn
+++ b/resources/translations/fi.edn
@@ -5,10 +5,15 @@
             :accept-licenses "Hyväksy käyttöehdot"
             :add-licenses "Lisää käyttöehto"
             :add-member "Lisää jäsen"
+            :bundling-error "Resursseja ei voi hakea samalla hakemuksella."
+            :bundling-warning "Resursseja ei tavallisesti voi hakea samalla hakemuksella, koska niillä on eri lomake tai työvuo."
+            :change-form-error "Resursseja ei voi hakea tällä hakemuksella. Muuta niitä tai tee uusi hakemus."
+            :change-form-warning "Lomake on eri kuin alkuperäisessä hakemuksessa."
+            :change-workflow-error "Resursseja ei voi hakea tällä hakemuksella. Muuta niitä tai tee uusi hakemus."
+            :change-workflow-warning "Työvuo on eri kuin alkuperäisessä hakemuksessa."
             :change-resources "Muuta resursseja"
             :approve "Hyväksy"
             :approve-reject "Hyväksy tai hylkää"
-            :bundling-intro [:div.intro [:p "Huomaa, että vain resursseja, joilla on sama hakemuslomake ja työvuo, voidaan liittää yhteen."]]
             :cancel "Peruuta"
             :close "Sulje hakemus"
             :close-intro [:div.intro [:p "Huomaa, että jos hakemus suljetaan, sitä ei voida avata uudelleen."]]
@@ -34,9 +39,9 @@
             :remark-public "Näytä huomio hakijalle"
             :remove-member "Poista jäsen"
             :request-decision "Pyydä päätöstä"
-            :request-review "Pyydä katselmointia"
             :request-selection "Lähetä pyyntöjä"
-            :resources-selection "Hakemukseen liitettävät resurssit:"
+            :request-review "Pyydä katselmointia"
+            :resources-selection "Hakemukseen liitettävät resurssit"
             :return "Palauta hakijalle"
             :review "Katselmoi"
             :show-publications "Näytä julkaisut"
diff --git a/src/clj/rems/application/commands.clj b/src/clj/rems/application/commands.clj
index 9d8f611dd..6376d73f2 100644
--- a/src/clj/rems/application/commands.clj
+++ b/src/clj/rems/application/commands.clj
@@ -367,9 +367,11 @@
                                               (mapv (fn [catalogue-item]
                                                       {:catalogue-item/id (:id catalogue-item)
                                                        :resource/ext-id (:resid catalogue-item)})))
-                  :application/licenses
-                  (mapv (fn [license] {:license/id (:id license)})
-                        ((:get-licenses injections) (:catalogue-item-ids cmd)))}
+                  :application/licenses (->> (:catalogue-item-ids cmd)
+                                             (mapcat (:get-catalogue-item-licenses injections))
+                                             distinct
+                                             (mapv (fn [license]
+                                                     {:license/id (:id license)})))}
                  (when (:comment cmd)
                    {:application/comment (:comment cmd)})))))
 
diff --git a/src/clj/rems/db/applications.clj b/src/clj/rems/db/applications.clj
index 2e66d0f39..4a6d90352 100644
--- a/src/clj/rems/db/applications.clj
+++ b/src/clj/rems/db/applications.clj
@@ -86,9 +86,6 @@
 (defn add-application-created-event! [opts]
   (events/add-event! (application-created-event (assoc opts :allocate-external-id? true))))
 
-(defn- get-workflow-id-for-catalogue-items [catalogue-item-ids]
-  (:workflow/id (application-created-event {:catalogue-item-ids catalogue-item-ids})))
-
 (defn create-application! [user-id catalogue-item-ids]
   (let [start (time/now)
         app-id (:id (db/create-application!))]
@@ -101,10 +98,10 @@
 
 ;;; Running commands
 
-(defn- get-licenses-for-catalogue-items [catalogue-item-ids]
+(defn- get-catalogue-item-licenses [catalogue-item-id]
   (db/get-licenses
-   {:wfid (get-workflow-id-for-catalogue-items catalogue-item-ids)
-    :items catalogue-item-ids}))
+   {:wfid (:wfid (catalogue/get-localized-catalogue-item catalogue-item-id))
+    :items [catalogue-item-id]}))
 
 (defn- valid-user? [userid]
   (not (nil? (users/get-user-attributes userid))))
@@ -114,7 +111,7 @@
    :validate-fields form-validation/validate-fields
    :secure-token secure-token
    :get-catalogue-item catalogue/get-localized-catalogue-item
-   :get-licenses get-licenses-for-catalogue-items})
+   :get-catalogue-item-licenses get-catalogue-item-licenses})
 
 (declare get-unrestricted-application)
 
diff --git a/src/cljs/rems/actions/change_resources.cljs b/src/cljs/rems/actions/change_resources.cljs
index 5c8a669a4..acfd92327 100644
--- a/src/cljs/rems/actions/change_resources.cljs
+++ b/src/cljs/rems/actions/change_resources.cljs
@@ -57,15 +57,65 @@
                   :text (text :t.actions/change-resources)
                   :on-click #(rf/dispatch [::open-form initial-resources])}])
 
-(defn compatible-item? [item original-workflow-id original-form-id]
-  (and (= original-workflow-id (:wfid item))
-       (= original-form-id (:formid item))))
+(defn- show-bundling-warning? [resources]
+  (let [workflows (set (map :wfid resources))
+        forms (set (map :formid resources))]
+    (and (seq resources)
+         (not= 1 (count workflows) (count forms)))))
+
+(defn- bundling-warning [resources can-bundle-all? language]
+  [:div
+   [:div.alert {:class (if can-bundle-all? :alert-warning :alert-danger)}
+    [:p (if can-bundle-all?
+          (text :t.actions/bundling-warning)
+          (text :t.actions/bundling-error))]
+    (into [:ul]
+          (for [group (vals (group-by (juxt :wfid :formid) resources))]
+            [:li (str/join ", " (map #(get-localized-title % language) group))]))]])
+
+(defn- show-change-form-warning? [original-form-id resources]
+  (and (seq resources)
+       (apply not= original-form-id (map :formid resources))))
+
+(defn- change-form-warning [resources can-bundle-all? language]
+  [:div
+   [:div.alert {:class (if can-bundle-all? :alert-warning :alert-danger)}
+    [:p (if can-bundle-all?
+          (text :t.actions/change-form-warning)
+          (text :t.actions/change-form-error))]
+    (into [:ul]
+          (for [group (vals (group-by :formid resources))]
+            [:li (str/join ", " (map #(get-localized-title % language) group))]))]])
+
+(defn- show-change-workflow-warning? [original-workflow-id resources]
+  (and (seq resources)
+       (apply not= original-workflow-id (map :wfid resources))))
+
+(defn- change-workflow-warning [resources can-bundle-all? language]
+  [:div
+   [:div.alert {:class (if can-bundle-all? :alert-warning :alert-danger)}
+    [:p (if can-bundle-all?
+          (text :t.actions/change-workflow-warning)
+          (text :t.actions/change-workflow-error))]
+    (into [:ul]
+          (for [group (vals (group-by :wfid resources))]
+            [:li (str/join ", " (map #(get-localized-title % language) group))]))]])
+
+(defn compatible-item? [item resources original-workflow-id original-form-id]
+  (not (or (show-bundling-warning? (conj resources item))
+           (show-change-form-warning? original-form-id (conj resources item))
+           (show-change-workflow-warning? original-workflow-id (conj resources item)))))
 
 (defn change-resources-view
-  [{:keys [application initial-resources selected-resources full-catalogue catalogue comment can-comment? language on-set-comment on-set-resources on-send]}]
-  (let [original-form-id (get-in application [:application/form :form/id])
+  [{:keys [application initial-resources selected-resources full-catalogue catalogue comment can-bundle-all? can-comment? language on-set-comment on-set-resources on-send]}]
+  (let [indexed-resources (index-by [:id] full-catalogue)
+        enriched-selected-resources (->> selected-resources
+                                         (select-keys indexed-resources)
+                                         vals
+                                         (sort-by #(get-localized-title % language)))
+        original-form-id (get-in application [:application/form :form/id])
         original-workflow-id (get-in application [:application/workflow :workflow/id])
-        compatible-first-sort-fn #(if (compatible-item? % original-workflow-id original-form-id) -1 1)
+        compatible-first-sort-fn #(if (compatible-item? % enriched-selected-resources original-workflow-id original-form-id) -1 1)
         sorted-selected-catalogue (->> catalogue
                                        (sort-by #(get-localized-title % language))
                                        (sort-by compatible-first-sort-fn))]
@@ -74,7 +124,11 @@
      [[button-wrapper {:id "change-resources"
                        :text (text :t.actions/change-resources)
                        :class "btn-primary"
-                       :disabled (or (empty? selected-resources)
+                       :disabled (or (and (not can-bundle-all?)
+                                          (or (show-bundling-warning? enriched-selected-resources))
+                                          (show-change-form-warning? original-form-id enriched-selected-resources)
+                                          (show-change-workflow-warning? original-workflow-id enriched-selected-resources))
+                                     (empty? selected-resources)
                                      (= selected-resources initial-resources))
                        :on-click on-send}]]
      (if (empty? catalogue)
@@ -84,6 +138,12 @@
        ;;   previously showed a warning if the selected resources were
        ;;   incompatible.
        [:div
+        (cond (show-bundling-warning? enriched-selected-resources)
+              [bundling-warning enriched-selected-resources can-bundle-all? language]
+              (show-change-form-warning? original-form-id enriched-selected-resources)
+              [change-workflow-warning enriched-selected-resources can-bundle-all? language]
+              (show-change-workflow-warning? original-workflow-id enriched-selected-resources)
+              [change-form-warning enriched-selected-resources can-bundle-all? language])
         (when can-comment?
           [action-comment {:id action-form-id
                            :label (text :t.form/add-comments-shown-to-applicant)
@@ -94,14 +154,13 @@
          [dropdown/dropdown
           {:id dropdown-id
            :items sorted-selected-catalogue
-           :item-disabled? #(not (compatible-item? % original-workflow-id original-form-id))
+           :item-disabled? #(not (compatible-item? % enriched-selected-resources original-workflow-id original-form-id))
            :item-label #(get-localized-title % language)
            :item-selected? #(contains? (set selected-resources) (% :id))
            :multi? true
-           :on-change on-set-resources}]]
-        (text :t.actions/bundling-intro)])]))
+           :on-change on-set-resources}]]])]))
 
-(defn change-resources-form [application can-comment? on-finished]
+(defn change-resources-form [application can-bundle-all? can-comment? on-finished]
   (let [initial-resources @(rf/subscribe [::initial-resources])
         selected-resources @(rf/subscribe [::selected-resources])
         full-catalogue @(rf/subscribe [:rems.catalogue/full-catalogue])
@@ -114,6 +173,7 @@
                             :full-catalogue full-catalogue
                             :catalogue catalogue
                             :comment comment
+                            :can-bundle-all? can-bundle-all?
                             :can-comment? can-comment?
                             :language language
                             :on-set-comment #(rf/dispatch [::set-comment %])
diff --git a/src/cljs/rems/application.cljs b/src/cljs/rems/application.cljs
index e496230b3..1b6f36434 100644
--- a/src/cljs/rems/application.cljs
+++ b/src/cljs/rems/application.cljs
@@ -586,7 +586,8 @@
   (let [application-id (:application/id application)
         permissions (:application/permissions application)
         applicant? (= (:application/applicant application) userid)
-        can-change-resources? (contains? permissions :application.command/change-resources)
+        can-bundle-all? (not applicant?)
+        can-change? (contains? permissions :application.command/change-resources)
         can-comment? (not applicant?)]
     [collapsible/component
      {:id "resources"
@@ -598,9 +599,9 @@
                        [:div.application-resource (localized (:catalogue-item/title resource))]))]
       :footer [:div
                [:div.commands
-                (when can-change-resources? [change-resources-action-button (:application/resources application)])]
+                (when can-change? [change-resources-action-button (:application/resources application)])]
                [:div#resource-action-forms
-                [change-resources-form application can-comment? (partial reload! application-id)]]]}]))
+                [change-resources-form application can-bundle-all? can-comment? (partial reload! application-id)]]]}]))
 
 (defn- render-application [application edit-application config userid]
   (let [messages (remove nil?


Test patch:
diff --git a/test/clj/rems/application/test_commands.clj b/test/clj/rems/application/test_commands.clj
index 53e6592c3..f429a4dad 100644
--- a/test/clj/rems/application/test_commands.clj
+++ b/test/clj/rems/application/test_commands.clj
@@ -169,169 +169,191 @@
                           :licenses []})))))
 
 (deftest test-change-resources
-  (let [application (apply-events nil [dummy-created-event])
-        submitted-application (apply-events nil [dummy-created-event
-                                                 {:event/type :application.event/submitted
-                                                  :event/time test-time
-                                                  :event/actor applicant-user-id
-                                                  :application/id 123}])
-        approved-application (apply-events nil [dummy-created-event
-                                                {:event/type :application.event/submitted
-                                                 :event/time test-time
-                                                 :event/actor applicant-user-id
-                                                 :application/id 123}
-                                                {:event/type :application.event/approved
-                                                 :event/time test-time
-                                                 :event/actor handler-user-id
-                                                 :application/comment "This is good"
-                                                 :application/id 123}])
-        catalogue-item-to-license {1 1
-                                   2 2
-                                   3 3
-                                   4 1}
-        injections {:get-catalogue-item {1 {:id 1 :resid "abc" :formid 1 :wfid 1}
-                                         2 {:id 2 :resid "efg" :formid 1 :wfid 1}
-                                         3 {:id 3 :resid "hij" :formid 1 :wfid 2}
-                                         4 {:id 4 :resid "klm" :formid 2 :wfid 1}}
-                    :get-licenses
-                    (fn [catalogue-item-ids]
-                      (->> catalogue-item-ids
-                           (mapv (fn [id] {:id (catalogue-item-to-license id)}))
-                           (distinct-by :id)))}]
-    (testing "applicant can change draft resources"
+  (let [cat-1 1
+        cat-2-other-license 2
+        cat-3-other-workflow 3
+        cat-4-other-form 4
+        form-1 1
+        form-2 2
+        wf-1 1
+        wf-2 2
+        license-1 1
+        license-2 2
+        application (apply-events nil [dummy-created-event])
+        submitted-application (apply-events application
+                                            [{:event/type :application.event/submitted
+                                              :event/time test-time
+                                              :event/actor applicant-user-id
+                                              :application/id 123}])
+        approved-application (apply-events submitted-application
+                                           [{:event/type :application.event/approved
+                                             :event/time test-time
+                                             :event/actor handler-user-id
+                                             :application/comment "This is good"
+                                             :application/id 123}])
+        injections {:get-catalogue-item
+                    {cat-1 {:id cat-1 :resid "res1" :formid form-1 :wfid wf-1}
+                     cat-2-other-license {:id cat-2-other-license :resid "res2" :formid form-1 :wfid wf-1}
+                     cat-3-other-workflow {:id cat-3-other-workflow :resid "res3" :formid form-1 :wfid wf-2}
+                     cat-4-other-form {:id cat-4-other-form :resid "res4" :formid form-2 :wfid wf-1}}
+                    :get-catalogue-item-licenses
+                    {cat-1 [{:id license-1}]
+                     cat-2-other-license [{:id license-2}]
+                     cat-3-other-workflow [{:id license-1}]
+                     cat-4-other-form [{:id license-1}]}}]
+
+    (testing "applicant can add resources to a draft application"
       (is (= {:event/type :application.event/resources-changed
               :event/time test-time
               :event/actor applicant-user-id
               :application/id 123
-              :application/resources [{:catalogue-item/id 1 :resource/ext-id "abc"}
-                                      {:catalogue-item/id 2 :resource/ext-id "efg"}]
-              :application/licenses [{:license/id 1}
-                                     {:license/id 2}]}
+              :application/resources [{:catalogue-item/id cat-1 :resource/ext-id "res1"}
+                                      {:catalogue-item/id cat-2-other-license :resource/ext-id "res2"}]
+              :application/licenses [{:license/id license-1}
+                                     {:license/id license-2}]}
              (ok-command application
                          {:type :application.command/change-resources
                           :actor applicant-user-id
-                          :catalogue-item-ids [1 2]}
-                         injections)))
+                          :catalogue-item-ids [cat-1 cat-2-other-license]}
+                         injections))))
+
+    (testing "applicant cannot add resources to a submitted application"
       (is (= {:errors [{:type :forbidden}]}
              (fail-command submitted-application
                            {:type :application.command/change-resources
                             :actor applicant-user-id
-                            :catalogue-item-ids [1 2]}
-                           injections)))
+                            :catalogue-item-ids [cat-1 cat-2-other-license]}
+                           injections))))
+
+    (testing "applicant cannot add resources with different workflow"
       (is (= {:errors [{:type :unbundlable-catalogue-items
-                        :catalogue-item-ids [1 2 3]}]}
+                        :catalogue-item-ids [cat-1 cat-3-other-workflow]}]}
              (fail-command application
                            {:type :application.command/change-resources
                             :actor applicant-user-id
-                            :catalogue-item-ids [1 2 3]}
-                           injections))
-          "can't bundle with different wfid")
+                            :catalogue-item-ids [cat-1 cat-3-other-workflow]}
+                           injections))))
+
+    (testing "applicant cannot add resources with different form"
       (is (= {:errors [{:type :unbundlable-catalogue-items
-                        :catalogue-item-ids [1 2 4]}]}
+                        :catalogue-item-ids [cat-1 cat-4-other-form]}]}
              (fail-command application
                            {:type :application.command/change-resources
                             :actor applicant-user-id
-                            :catalogue-item-ids [1 2 4]}
-                           injections))
-          "can't bundle with different formid")
-      (is (= {:errors [{:type :changes-original-workflow :workflow/id 1 :ids [2]}]}
+                            :catalogue-item-ids [cat-1 cat-4-other-form]}
+                           injections))))
+
+    (testing "applicant can replace resources"
+      (is (= {:event/type :application.event/resources-changed
+              :event/time test-time
+              :event/actor applicant-user-id
+              :application/id 123
+              :application/resources [{:catalogue-item/id cat-2-other-license :resource/ext-id "res2"}]
+              :application/licenses [{:license/id license-2}]}
+             (ok-command application
+                         {:type :application.command/change-resources
+                          :actor applicant-user-id
+                          :catalogue-item-ids [cat-2-other-license]}
+                         injections))))
+
+    (testing "applicant cannot replace resources with different workflow"
+      (is (= {:errors [{:type :changes-original-workflow :workflow/id wf-1 :ids [wf-2]}]}
              (fail-command application
                            {:type :application.command/change-resources
                             :actor applicant-user-id
-                            :catalogue-item-ids [3]}
-                           injections))
-          "can't change workflow from original")
-      (is (= {:errors [{:type :changes-original-form :form/id 1 :ids [2]}]}
+                            :catalogue-item-ids [cat-3-other-workflow]}
+                           injections))))
+
+    (testing "applicant cannot replace resources with different form"
+      (is (= {:errors [{:type :changes-original-form :form/id form-1 :ids [form-2]}]}
              (fail-command application
                            {:type :application.command/change-resources
                             :actor applicant-user-id
-                            :catalogue-item-ids [4]}
-                           injections))
-          "can't change formid from original"))
+                            :catalogue-item-ids [cat-4-other-form]}
+                           injections))))
 
-    (testing "handler can change submitted resources"
+    (testing "handler can add resources to a submitted application"
       (is (= {:event/type :application.event/resources-changed
               :event/time test-time
               :event/actor handler-user-id
               :application/id 123
               :application/comment "Changed these for you"
-              :application/resources [{:catalogue-item/id 1 :resource/ext-id "abc"}
-                                      {:catalogue-item/id 2 :resource/ext-id "efg"}]
-              :application/licenses [{:license/id 1}
-                                     {:license/id 2}]}
+              :application/resources [{:catalogue-item/id cat-1 :resource/ext-id "res1"}
+                                      {:catalogue-item/id cat-2-other-license :resource/ext-id "res2"}]
+              :application/licenses [{:license/id license-1}
+                                     {:license/id license-2}]}
              (ok-command submitted-application
                          {:type :application.command/change-resources
                           :actor handler-user-id
                           :comment "Changed these for you"
-                          :catalogue-item-ids [1 2]}
+                          :catalogue-item-ids [cat-1 cat-2-other-license]}
                          injections)))
 
-      (is (= {:event/type :application.event/resources-changed
-              :event/time test-time
-              :event/actor handler-user-id
-              :application/id 123
-              :application/comment "Changed these for you"
-              :application/resources [{:catalogue-item/id 1 :resource/ext-id "abc"}
-                                      {:catalogue-item/id 2 :resource/ext-id "efg"}
-                                      {:catalogue-item/id 3 :resource/ext-id "hij"}
-                                      {:catalogue-item/id 4 :resource/ext-id "klm"}]
-              :application/licenses [{:license/id 1}
-                                     {:license/id 2}
-                                     {:license/id 3}]}
-             (ok-command submitted-application
-                         {:type :application.command/change-resources
-                          :actor handler-user-id
-                          :comment "Changed these for you"
-                          :catalogue-item-ids [1 2 3 4]}
-                         injections))))
+      (testing "- even with a different workflow or form"
+        (is (= {:event/type :application.event/resources-changed
+                :event/time test-time
+                :event/actor handler-user-id
+                :application/id 123
+                :application/comment "Changed these for you"
+                :application/resources [{:catalogue-item/id cat-1 :resource/ext-id "res1"}
+                                        {:catalogue-item/id cat-3-other-workflow :resource/ext-id "res3"}
+                                        {:catalogue-item/id cat-4-other-form :resource/ext-id "res4"}]
+                :application/licenses [{:license/id license-1}]}
+               (ok-command submitted-application
+                           {:type :application.command/change-resources
+                            :actor handler-user-id
+                            :comment "Changed these for you"
+                            :catalogue-item-ids [cat-1 cat-3-other-workflow cat-4-other-form]}
+                           injections)))))
 
-    (testing "handler can change approved resources"
+    (testing "handler can add resources to an approved application"
       (is (= {:event/type :application.event/resources-changed
               :event/time test-time
               :event/actor handler-user-id
               :application/id 123
               :application/comment "Changed these for you"
-              :application/resources [{:catalogue-item/id 1 :resource/ext-id "abc"}
-                                      {:catalogue-item/id 2 :resource/ext-id "efg"}]
-              :application/licenses [{:license/id 1}
-                                     {:license/id 2}]}
+              :application/resources [{:catalogue-item/id cat-1 :resource/ext-id "res1"}
+                                      {:catalogue-item/id cat-2-other-license :resource/ext-id "res2"}]
+              :application/licenses [{:license/id license-1}
+                                     {:license/id license-2}]}
              (ok-command approved-application
                          {:type :application.command/change-resources
                           :actor handler-user-id
                           :comment "Changed these for you"
-                          :catalogue-item-ids [1 2]}
+                          :catalogue-item-ids [cat-1 cat-2-other-license]}
                          injections)))
 
-      (is (= {:event/type :application.event/resources-changed
-              :event/time test-time
-              :event/actor handler-user-id
-              :application/id 123
-              :application/comment "Changed these for you"
-              :application/resources [{:catalogue-item/id 1 :resource/ext-id "abc"}
-                                      {:catalogue-item/id 2 :resource/ext-id "efg"}
-                                      {:catalogue-item/id 3 :resource/ext-id "hij"}
-                                      {:catalogue-item/id 4 :resource/ext-id "klm"}]
-              :application/licenses [{:license/id 1}
-                                     {:license/id 2}
-                                     {:license/id 3}]}
-             (ok-command approved-application
-                         {:type :application.command/change-resources
-                          :actor handler-user-id
-                          :comment "Changed these for you"
-                          :catalogue-item-ids [1 2 3 4]}
-                         injections))))
+      (testing "- even with different workflow or form"
+        (is (= {:event/type :application.event/resources-changed
+                :event/time test-time
+                :event/actor handler-user-id
+                :application/id 123
+                :application/comment "Changed these for you"
+                :application/resources [{:catalogue-item/id cat-1 :resource/ext-id "res1"}
+                                        {:catalogue-item/id cat-3-other-workflow :resource/ext-id "res3"}
+                                        {:catalogue-item/id cat-4-other-form :resource/ext-id "res4"}]
+                :application/licenses [{:license/id license-1}]}
+               (ok-command approved-application
+                           {:type :application.command/change-resources
+                            :actor handler-user-id
+                            :comment "Changed these for you"
+                            :catalogue-item-ids [cat-1 cat-3-other-workflow cat-4-other-form]}
+                           injections)))))
 
-    (is (= {:errors [{:type :invalid-catalogue-item :catalogue-item-id 42}]}
-           (fail-command application
-                         {:type :application.command/change-resources
-                          :actor applicant-user-id
-                          :catalogue-item-ids [42]}
-                         injections)))
-    (is (= {:errors [{:type :must-not-be-empty :key :catalogue-item-ids}]}
-           (fail-command application
-                         {:type :application.command/change-resources
-                          :actor applicant-user-id
-                          :catalogue-item-ids []})))))
+    (testing "the catalogue item must exist"
+      (is (= {:errors [{:type :invalid-catalogue-item :catalogue-item-id 42}]}
+             (fail-command application
+                           {:type :application.command/change-resources
+                            :actor applicant-user-id
+                            :catalogue-item-ids [42]}
+                           injections))))
+
+    (testing "there must be at least one catalogue item"
+      (is (= {:errors [{:type :must-not-be-empty :key :catalogue-item-ids}]}
+             (fail-command application
+                           {:type :application.command/change-resources
+                            :actor applicant-user-id
+                            :catalogue-item-ids []}))))))
 
 (deftest test-submit
   (let [injections {:validate-fields form-validation/validate-fields}