Prevent change-resources when it would violate catalogue item hierarchy

docs/development.md

@@ -286,3 +286,14 @@ This will set local REMS instance as the target and start 3 concurrent browser i
 ## CircleCI
 
 CircleCI offers a [command-line tool](https://circleci.com/docs/guides/toolkit/local-cli/) for debugging and validating the [config file](/.circleci/config.yml) (and much more)
+
+
+## Lucene
+
+The binary release of Lucene includes a GUI tool (Luke) that can be useful for viewing and searching the index. Download the binary from the [Lucene website](https://lucene.apache.org/core/downloads.html), extract it somewhere, `cd` to that directory and launch the tool with
+```sh
+bin/luke.sh
+```
+Then choose the index you want to inspect (`rems/target/search-index-dev`, `...-test`) and pick `NIOFSDirectory` for "Directory implementation". 
+
+The tool is somewhat klunky: any fields indexed as Long need to be set as such (from the default int) under Search > Query Parser > Point range query

docs/search.md

@@ -11,7 +11,7 @@ syntax it's possible to build more specific queries. Wildcards `?` and `*` are
 supported, plus many more. See the [query syntax documentation][query-syntax]
 for more details.
 
-[query-syntax]: https://lucene.apache.org/core/8_2_0/queryparser/org/apache/lucene/queryparser/classic/package-summary.html#package.description
+[query-syntax]: https://lucene.apache.org/core/9_12_3/queryparser/org/apache/lucene/queryparser/classic/package-summary.html
 
 ### Examples
 
@@ -51,4 +51,4 @@ for more details.
 
 The catalogue page and admin pages use a simpler search mechanism which
 only does a substring search of the values shown in the table.
-The search tems are joined with the "AND" operation.
+The search tems are joined with the "AND" operation.

src/clj/rems/application/commands.clj

@@ -114,7 +114,8 @@
                               (remove (set catalogue-item-ids))
                               (map get-catalogue-item)
                               (filter (complement (comp entitled-to-resids :resource-id)))
-                              (map :id))
+                              (map :id)
+                              (distinct))
                         catalogue-item-ids)]
       (when (seq missing)
         {:errors [{:type :t.applications.errors/missing-top-level-item
@@ -450,21 +451,21 @@
                                     :application/licenses (mapv (fn [id] {:license/id id}) (:licenses cmd))})))
 
 (defmethod command-handler :application.command/change-resources
-  [cmd application {:keys [get-catalogue-item get-workflow] :as injections}]
-  (let [cat-ids (:catalogue-item-ids cmd)
-        workflow (when (seq cat-ids)
-                   (get-workflow (-> (first cat-ids)
+  [{:keys [catalogue-item-ids actor] :as cmd} application {:keys [get-catalogue-item get-workflow] :as injections}]
+  (let [workflow (when (seq catalogue-item-ids)
+                   (get-workflow (-> (first catalogue-item-ids)
                                      get-catalogue-item
                                      :wfid)))]
     (or (must-not-be-empty cmd :catalogue-item-ids)
-        (invalid-catalogue-items cat-ids injections)
-        (unbundlable-catalogue-items-for-actor application cat-ids (:actor cmd) injections)
-        (changes-original-workflow application cat-ids (:actor cmd) injections)
+        (invalid-catalogue-items catalogue-item-ids injections)
+        (unbundlable-catalogue-items-for-actor application catalogue-item-ids actor injections)
+        (changes-original-workflow application catalogue-item-ids actor injections)
+        (invalid-catalogue-item-hierarchy-error catalogue-item-ids actor injections)
         (add-comment-and-attachments cmd application injections
                                      {:event/type :application.event/resources-changed
-                                      :application/forms (build-forms-list workflow cat-ids injections)
-                                      :application/resources (build-resources-list cat-ids injections)
-                                      :application/licenses (build-licenses-list cat-ids injections)}))))
+                                      :application/forms (build-forms-list workflow catalogue-item-ids injections)
+                                      :application/resources (build-resources-list catalogue-item-ids injections)
+                                      :application/licenses (build-licenses-list catalogue-item-ids injections)}))))
 
 (defmethod command-handler :application.command/add-member
   [cmd application injections]

src/cljs/rems/actions/change_resources.cljs

@@ -1,12 +1,16 @@
 (ns rems.actions.change-resources
-  (:require [re-frame.core :as rf]
-            [rems.actions.components :refer [action-button action-form-view comment-field collapse-action-form perform-action-button]]
-            [rems.globals]
+  (:require [clojure.set :as set]
+            [medley.core :refer [distinct-by]]
+            [re-frame.core :as rf]
+            [rems.actions.components :refer [action-button action-form-view
+                                             collapse-action-form
+                                             comment-field
+                                             perform-action-button]]
             [rems.dropdown :as dropdown]
             [rems.flash-message :as flash-message]
-            [medley.core :refer [distinct-by]]
+            [rems.globals]
             [rems.spinner :as spinner]
-            [rems.text :refer [text get-localized-title]]
+            [rems.text :refer [get-localized-title text]]
             [rems.util :refer [post!]]))
 
 (def ^:private action-form-id "change-resources")
@@ -18,10 +22,14 @@
    (merge
     {:db (assoc db
                 ::initial-resources (into #{} (map :catalogue-item/id initial-resources))
-                ::selected-resources (into #{} (map :catalogue-item/id initial-resources)))
-     :dispatch-n (concat [[:rems.actions.components/set-comment action-form-id ""]]
-                         (when-not (:rems.catalogue/catalogue db)
-                           [[:rems.catalogue/full-catalogue]]))})))
+                ::selected-resources (into #{} (map :catalogue-item/id initial-resources))
+                ::error nil)
+     :dispatch-n (cond-> [[:rems.actions.components/set-comment action-form-id ""]]
+                   (not (:rems.catalogue/catalogue db))
+                   (conj [:rems.catalogue/full-catalogue])
+
+                   (:enable-catalogue-hierarchy @rems.globals/config)
+                   (conj [:rems.catalogue/entitlements->catalogue-item-ids]))})))
 
 (rf/reg-sub
  ::catalogue
@@ -44,6 +52,9 @@
 (rf/reg-sub ::selected-resources (fn [db _] (::selected-resources db)))
 (rf/reg-event-db ::set-selected-resources (fn [db [_ resources]] (assoc db ::selected-resources (set (map :id resources)))))
 
+(rf/reg-sub ::command-error (fn [db _] some? (::command-error db)))
+(rf/reg-event-db ::set-command-error (fn [db [_ error]] (assoc db ::command-error error)))
+
 (def ^:private dropdown-id "change-resources-dropdown")
 
 ;; The API allows us to add attachments to this command
@@ -57,12 +68,25 @@
                              :catalogue-item-ids (vec resources)}
                             (when comment
                               {:comment comment}))
-             :handler (flash-message/default-success-handler
-                       :change-resources
-                       description
-                       (fn [_]
-                         (collapse-action-form action-form-id)
-                         (on-finished)))
+             :handler (fn [{:keys [success errors] :as response}]
+                        (cond success
+                              (do
+                                ((flash-message/default-success-handler
+                                  :change-resources
+                                  description
+                                  (fn [_]
+                                    (collapse-action-form action-form-id)
+                                    (on-finished)))
+                                 response)
+                                (rf/dispatch [::set-command-error nil]))
+                              errors
+                              (do
+                                (flash-message/show-error!
+                                 :change-resources
+                                 (->> errors
+                                      (mapv (flash-message/argumentize-some-key :catalogue-item-id :catalogue-item-ids))
+                                      flash-message/format-errors))
+                                (rf/dispatch [::set-command-error errors]))))
              :error-handler (flash-message/default-error-handler :change-resources description)}))
    {}))
 
@@ -74,21 +98,31 @@
 (defn compatible-item? [item original-workflow-id]
   (= original-workflow-id (:wfid item)))
 
+(defn compatible-hierarchy? [catalogue-item selected-resources entitlements]
+  (if-let [top-level-id (-> catalogue-item :part-of :catalogue-item/id)]
+    (contains? (set/union entitlements selected-resources) top-level-id)
+    true))
+
 (defn change-resources-view
-  [{:keys [application initial-resources selected-resources catalogue can-comment? on-set-resources on-send]}]
+  [{:keys [application initial-resources selected-resources catalogue entitlements can-comment? on-set-resources on-send]}]
   (let [original-workflow-id (get-in application [:application/workflow :workflow/id])
         compatible-first-sort-fn #(if (compatible-item? % original-workflow-id) -1 1)
         sorted-selected-catalogue (->> catalogue
                                        (sort-by #(get-localized-title %))
                                        (sort-by compatible-first-sort-fn))
-        enable-cart? (:enable-cart @rems.globals/config)]
+        enable-cart? (:enable-cart @rems.globals/config)
+        enable-hierarchy? (:enable-catalogue-hierarchy @rems.globals/config)
+        item-disabled? #(or (not (compatible-item? % original-workflow-id))
+                            (and enable-hierarchy?
+                                 (not (compatible-hierarchy? % selected-resources entitlements))))]
     [action-form-view action-form-id
      (text :t.actions/change-resources)
      [[perform-action-button {:id "change-resources"
                               :text (text :t.actions/change-resources)
                               :class "btn-primary"
                               :disabled (or (empty? selected-resources)
-                                            (= selected-resources initial-resources))
+                                            (and (not @(rf/subscribe [::command-error]))
+                                                 (= selected-resources initial-resources)))
                               :on-click on-send}]]
      (if (empty? catalogue)
        [spinner/big]
@@ -106,7 +140,7 @@
           {:id dropdown-id
            :items (->> sorted-selected-catalogue
                        (mapv #(assoc % ::label (get-localized-title %))))
-           :item-disabled? #(not (compatible-item? % original-workflow-id))
+           :item-disabled? item-disabled?
            :item-key :id
            :item-label ::label
            :item-selected? #(contains? (set selected-resources) (% :id))
@@ -121,11 +155,13 @@
   (let [initial-resources @(rf/subscribe [::initial-resources])
         selected-resources @(rf/subscribe [::selected-resources])
         catalogue @(rf/subscribe [::catalogue])
+        entitlements @(rf/subscribe [:rems.catalogue/entitlements->catalogue-item-ids])
         comment @(rf/subscribe [:rems.actions.components/comment action-form-id])]
     [change-resources-view {:application application
                             :initial-resources initial-resources
                             :selected-resources selected-resources
                             :catalogue catalogue
+                            :entitlements entitlements
                             :can-comment? can-comment?
                             :on-set-resources #(rf/dispatch [::set-selected-resources %])
                             :on-send #(rf/dispatch [::send-change-resources {:application-id (:application/id application)

src/cljs/rems/catalogue.cljs

@@ -22,14 +22,18 @@
  ::enter-page
  (fn [{:keys [db]} _]
    {:db (dissoc db ::catalogue ::draft-applications)
-    :dispatch-n [[:rems.table/reset]
-                 [::entitlements]
-                 (when @roles/logged-in?
-                   [::draft-applications])
-                 (when (:enable-catalogue-tree @rems.globals/config)
-                   [::full-catalogue-tree])
-                 (when (:enable-catalogue-table @rems.globals/config)
-                   [::full-catalogue])]}))
+    :dispatch-n (cond-> [[:rems.table/reset]]
+                  @roles/logged-in?
+                  (conj [::draft-applications])
+
+                  (:enable-catalogue-tree @rems.globals/config)
+                  (conj [::full-catalogue-tree])
+
+                  (:enable-catalogue-table @rems.globals/config)
+                  (conj [::full-catalogue])
+
+                  (:enable-catalogue-hierarchy @rems.globals/config)
+                  (conj [::entitlements]))}))
 
 (fetcher/reg-fetcher ::full-catalogue "/api/catalogue?join-organization=false")
 (fetcher/reg-fetcher ::full-catalogue-tree "/api/catalogue/tree?join-organization=false" {:result :roots})
@@ -51,7 +55,7 @@
 (rf/reg-sub
  ::entitlements->catalogue-item-ids
  :<- [::entitlements]
- :<- [::catalogue]
+ :<- [::full-catalogue]
  (fn [[entitlements catalogue] _]
    (let [entitled-to-resources (into #{} (map :resource) entitlements)]
      (into #{}

src/cljs/rems/flash_message.cljs

@@ -128,8 +128,7 @@
            :args
            [(->> error
                  ((apply some-fn id-keys))
-                 vector
-                 flatten
+                 ((fn [x] (cond-> x (not (seqable? x)) list)))
                  (str/join ", "))])))
 
 (deftest test-argumentize-some-key

src/cljs/rems/new_application.cljs

@@ -28,7 +28,6 @@
                         errors
                         (do
                           (replace-url! "/catalogue")
-                          (js/console.log errors)
                           (flash-message/show-error! :top (->> errors
                                                                (mapv (flash-message/argumentize-some-key :catalogue-item-id :catalogue-item-ids))
                                                                flash-message/format-errors)))))

test/clj/rems/application/test_commands.clj

@@ -789,6 +789,30 @@
                           :catalogue-item-ids [4]}
                          (build-application-view [dummy-created-event])))))
 
+  (testing "applicant can add hierarchical catalogue items"
+    (is (= {:event/type :application.event/resources-changed
+            :event/time test-time
+            :event/actor applicant-user-id
+            :application/id app-id
+            :application/forms [{:form/id 1}]
+            :application/resources [{:catalogue-item/id 2, :resource/ext-id "res2"}
+                                    {:catalogue-item/id 8 :resource/ext-id "res-top-level"}
+                                    {:catalogue-item/id 9 :resource/ext-id "res-complementary"}]
+            :application/licenses [{:license/id 2} {:license/id 1}]}
+           (ok-command {:type :application.command/change-resources
+                        :actor applicant-user-id
+                        :catalogue-item-ids [2 8 9]}
+                       (build-application-view [dummy-created-event])
+                       (assoc command-injections :get-config (constantly {:enable-catalogue-hierarchy true}))))))
+
+  (testing "applicant cannot change resources in a way that would violate catalogue item hierarchy"
+    (is (= {:errors [{:type :t.applications.errors/missing-top-level-item :catalogue-item-ids [8]}]}
+           (fail-command {:type :application.command/change-resources
+                          :actor applicant-user-id
+                          :catalogue-item-ids [2 9]}
+                         (build-application-view [dummy-created-event])
+                         (assoc command-injections :get-config (constantly {:enable-catalogue-hierarchy true}))))))
+
   (testing "applicant can replace resources with different form"
     (is (= {:event/type :application.event/resources-changed
             :event/time test-time

test/clj/rems/test_browser.clj

@@ -2721,6 +2721,7 @@
       (btu/screenshot "test-update-catalogue-item-before-update-2")
       (btu/scroll-and-click {:tag :button :fn/text "Update catalogue item"})
       (is (btu/eventually-visible? {:css ".alert-success"}))
+      (btu/wait-for-idle)
       (btu/wait-disabled {:tag :button :fn/text "Update catalogue item"})
       (is (= [{"name" "test-update-catalogue-item 1 EN"
                "form" "test-update-catalogue-item form 1"