simpler archive/unarchive errors

resources/translations/en.edn

@@ -73,14 +73,8 @@
                    :edit-workflow "Edit workflow"
                    :enable "Enable"
                    :end "End"
-                   :errors {:form-archived "Form has been archived:"
-                            :form-in-use "Form in use:"
-                            :license-archived "License has been archived:"
-                            :license-in-use "License in use:"
-                            :resource-archived "Resource has been archived:"
-                            :resource-in-use "Resource in use:"
-                            :workflow-archived "Workflow has been archived:"
-                            :workflow-in-use "Workflow in use:"}
+                   :errors {:dependencies-archived "Archived dependencies:"
+                            :in-use-by "It is in use by:"}
                    :export-applications "Export applications"
                    :form "Form"
                    :forms "Forms"

resources/translations/fi.edn

@@ -73,14 +73,8 @@
                    :edit-workflow "Muokkaa työvuota"
                    :enable "Ota käyttöön"
                    :end "Loppu"
-                   :errors {:form-archived "Lomake on arkistoitu:"
-                            :form-in-use "Lomake on käytössä:"
-                            :license-archived "Lisenssi on arkistoitu:"
-                            :license-in-use "Lisenssi on käytössä:"
-                            :resource-archived "Resurssi on arkistoitu:"
-                            :resource-in-use "Resurssi on käytössä:"
-                            :workflow-archived "Työvuo on arkistoitu:"
-                            :workflow-in-use "Työvuo on käytössä:"}
+                   :errors {:dependencies-archived "Arkistoidut riippuvuudet:"
+                            :in-use-by "Se on käytössä täällä:"}
                    :export-applications "Vie hakemuksia"
                    :form "Lomake"
                    :forms "Lomakkeet"

resources/translations/sv.edn

@@ -73,14 +73,8 @@
                    :edit-workflow "Ändra arbetsflöde"
                    :enable "Ta i bruk"
                    :end "Slut"
-                   :errors {:form-archived "Blanketten är arkiverad:"
-                            :form-in-use "Blanketten är aktiv:"
-                            :license-archived "Licensen är arkiverad:"
-                            :license-in-use "Lisencen är aktiv:"
-                            :resource-archived "Resursen är arkiverad:"
-                            :resource-in-use "Resursen är aktiv:"
-                            :workflow-archived "Arbetsflödet är arkiverat:"
-                            :workflow-in-use "Arbetsflödet är aktivt:"}
+                   :errors {:dependencies-archived "Arkiverad beroenden:"
+                            :in-use-by "Den är aktiv i:"}
                    :export-applications "exportera ansökningar"
                    :form "Blankett"
                    :forms "Blanketter"

src/clj/rems/api/services/catalogue.clj

@@ -70,13 +70,10 @@
 
 (defn set-catalogue-item-archived! [{:keys [id archived]}]
   (check-allowed-to-edit! id)
-  (if-let [errors (when-not archived
-                    (dependencies/unarchive-errors {:catalogue-item/id id}))]
-    {:success false
-     :errors errors}
-    (do (db/set-catalogue-item-archived! {:id id
-                                          :archived archived})
-        {:success true})))
+  (or (dependencies/change-archive-status-error archived {:catalogue-item/id id})
+      (do (db/set-catalogue-item-archived! {:id id
+                                            :archived archived})
+          {:success true})))
 
 (defn change-form!
   "Changes the form of a catalogue item.

src/clj/rems/api/services/dependencies.clj

@@ -7,7 +7,7 @@
             [rems.db.resource :as resource]
             [rems.db.workflow :as workflow]))
 
-(defn enrich-dependency [dep]
+(defn- enrich-dependency [dep]
   (cond
     (:license/id dep) (licenses/get-license (:license/id dep))
     (:resource/id dep) (resource/get-resource (:resource/id dep))
@@ -44,7 +44,7 @@
   {:dependencies (build-index {:keys [:from] :value-fn :to :collect-fn set} lst)
    :reverse-dependencies (build-index {:keys [:to] :value-fn :from :collect-fn set} lst)})
 
-(defn compute-dependencies []
+(defn- compute-dependencies []
   (-> (list-dependencies)
       list-to-maps))
 
@@ -58,7 +58,8 @@
 ;; 500ms with performance test data.
 (def ^:private dependencies-cache (atom nil))
 
-(defn dependencies []
+;; For now, all public uses are via the error helpers below
+(defn- dependencies []
   (when-not @dependencies-cache
     (reset! dependencies-cache (compute-dependencies)))
   @dependencies-cache)
@@ -85,45 +86,41 @@
                  (:form/id dep)
                  {:forms [(select-keys (enrich-dependency dep) [:form/id :form/title])]}))))
 
-(defn archive-errors
+(defn- archive-errors
   "Return errors if given item is depended on by non-archived items"
-  [error-key item]
+  [item]
   (when-let [users (->> (get-in (dependencies) [:reverse-dependencies item])
                         (mapv add-status-bits)
                         (remove :archived)
                         seq)]
-    [(merge {:type error-key}
-            (format-deps-for-errors users))]))
-
-(defn in-use-errors
-  "Returns errors if given item is depended on at all"
-  [error-key item]
-  (when-let [users (seq (get-in (dependencies) [:reverse-dependencies item]))]
-    [(merge {:type error-key}
-            (format-deps-for-errors users))]))
+    {:success false
+     :errors [(merge {:type :t.administration.errors/in-use-by}
+                     (format-deps-for-errors users))]}))
 
-(defn unarchive-errors
+(defn- unarchive-errors
   "Return errors if given item depends on archived items"
   [item]
   (when-let [used (->> (get-in (dependencies) [:dependencies item])
                        (mapv add-status-bits)
                        (filter :archived)
                        seq)]
-    (let [{:keys [licenses resources workflows catalogue-items forms]} (format-deps-for-errors used)]
-      (concat
-       (when licenses
-         [{:type :t.administration.errors/license-archived
-           :licenses licenses}])
-       (when resources
-         [{:type :t.administration.errors/resource-archived
-           :resources resources}])
-       (when workflows
-         [{:type :t.administration.errors/workflow-archived
-           :workflows workflows}])
-       (when forms
-         [{:type :t.administration.errors/form-archived
-           :forms forms}])
-       ;; case not possible:
-       (when catalogue-items
-         [{:type :catalogue-items-archived
-           :catalogue-items catalogue-items}])))))
+    {:success false
+     :errors [(merge {:type :t.administration.errors/dependencies-archived}
+                     (format-deps-for-errors used))]}))
+
+(defn change-archive-status-error
+  "Returns an error structure {:success false :errors [...]} if
+    - archived is true and item is used by non-archived items
+    - archived is false and item uses archived items"
+  [archived item]
+  (if archived
+    (archive-errors item)
+    (unarchive-errors item)))
+
+(defn in-use-error
+  "Returns an error structure {:success false :errors [...]} if given item is depended on by anything"
+  [item]
+  (when-let [users (seq (get-in (dependencies) [:reverse-dependencies item]))]
+    {:success false
+     :errors [(merge {:type :t.administration.errors/in-use-by}
+                     (format-deps-for-errors users))]}))

src/clj/rems/api/services/form.clj

@@ -16,13 +16,8 @@
             [schema.core :as s])
   (:import rems.InvalidRequestException))
 
-(defn- form-in-use-error [form-id]
-  (when-let [errors (dependencies/in-use-errors :t.administration.errors/form-in-use {:form/id form-id})]
-    {:success false
-     :errors errors}))
-
 (defn form-editable [form-id]
-  (or (form-in-use-error form-id)
+  (or (dependencies/in-use-error {:form/id form-id})
       {:success true}))
 
 (defn validate-given-ids
@@ -119,7 +114,7 @@
     ;; need to check both previous and new organization
     (util/check-allowed-organization! (:form/organization (get-form-template form-id)))
     (util/check-allowed-organization! organization)
-    (or (form-in-use-error form-id)
+    (or (dependencies/in-use-error {:form/id form-id})
         (validation-error form)
         (do (db/edit-form-template! {:id form-id
                                      :organization (:organization/id organization)
@@ -135,11 +130,8 @@
 
 (defn set-form-archived! [{:keys [id archived]}]
   (util/check-allowed-organization! (:form/organization (get-form-template id)))
-  (if-let [errors (when archived
-                    (dependencies/archive-errors :t.administration.errors/form-in-use {:form/id id}))]
-    {:success false
-     :errors errors}
-    (do
-      (db/set-form-template-archived! {:id id
-                                       :archived archived})
-      {:success true})))
+  (or (dependencies/change-archive-status-error archived {:form/id id})
+      (do
+        (db/set-form-template-archived! {:id id
+                                         :archived archived})
+        {:success true})))

src/clj/rems/api/services/licenses.clj

@@ -74,14 +74,11 @@
 
 (defn set-license-archived! [{:keys [id archived]}]
   (util/check-allowed-organization! (:organization (get-license id)))
-  (if-let [errors (and archived
-                       (dependencies/archive-errors :t.administration.errors/license-in-use {:license/id id}))]
-    {:success false
-     :errors errors}
-    (do
-      (db/set-license-archived! {:id id
-                                 :archived archived})
-      {:success true})))
+  (or (dependencies/change-archive-status-error archived  {:license/id id})
+      (do
+        (db/set-license-archived! {:id id
+                                   :archived archived})
+        {:success true})))
 
 (defn get-all-licenses
   "Get all licenses.

src/clj/rems/api/services/resource.clj

@@ -45,12 +45,8 @@
 
 (defn set-resource-archived! [{:keys [id archived]}]
   (util/check-allowed-organization! (:organization (get-resource id)))
-  (if-let [errors (if archived
-                    (dependencies/archive-errors :t.administration.errors/resource-in-use {:resource/id id})
-                    (dependencies/unarchive-errors {:resource/id id}))]
-    {:success false
-     :errors errors}
-    (do
-      (db/set-resource-archived! {:id id
-                                  :archived archived})
-      {:success true})))
+  (or (dependencies/change-archive-status-error archived {:resource/id id})
+      (do
+        (db/set-resource-archived! {:id id
+                                    :archived archived})
+        {:success true})))

src/clj/rems/api/services/workflow.clj

@@ -54,15 +54,11 @@
 
 (defn set-workflow-archived! [{:keys [id archived]}]
   (util/check-allowed-organization! (:organization (workflow/get-workflow id)))
-  (if-let [errors (if archived
-                    (dependencies/archive-errors :t.administration.errors/workflow-in-use {:workflow/id id})
-                    (dependencies/unarchive-errors {:workflow/id id}))]
-    {:success false
-     :errors errors}
-    (do
-      (db/set-workflow-archived! {:id id
-                                  :archived archived})
-      {:success true})))
+  (or (dependencies/change-archive-status-error archived {:workflow/id id})
+      (do
+        (db/set-workflow-archived! {:id id
+                                    :archived archived})
+        {:success true})))
 
 (defn- join-dependencies [workflow]
   (when workflow

src/cljs/rems/administration/status_flags.cljs

@@ -3,16 +3,28 @@
             [rems.atoms :refer [checkbox]]
             [rems.text :refer [text get-localized-title]]))
 
+;; TODO this should be in some util namespace
+(defn- get-localized-title-for-anything
+  ([item]
+   (get-localized-title-for-anything item @(rf/subscribe [:language])))
+  ([item language]
+   (or (get-localized-title item language)
+       (:resid item)
+       (:form/title item)
+       (:title item))))
+
 (defn- disable-button [item on-change]
   [:button.btn.btn-secondary.button-min-width
    {:type :button
-    :on-click #(on-change (assoc item :enabled false) [text :t.administration/disable])}
+    :on-click #(on-change (assoc item :enabled false) [:span [text :t.administration/disable]
+                                                       " \"" [get-localized-title-for-anything item] "\""])}
    (text :t.administration/disable)])
 
 (defn- enable-button [item on-change]
   [:button.btn.btn-primary.button-min-width
    {:type :button
-    :on-click #(on-change (assoc item :enabled true) [text :t.administration/enable])}
+    :on-click #(on-change (assoc item :enabled true) [:span [text :t.administration/enable]
+                                                      " \""  [get-localized-title-for-anything item] "\""])}
    (text :t.administration/enable)])
 
 ; TODO consider naming enabled-toggle-button
@@ -25,13 +37,15 @@
 (defn- archive-button [item on-change]
   [:button.btn.btn-secondary.button-min-width
    {:type :button
-    :on-click #(on-change (assoc item :archived true) [text :t.administration/archive])}
+    :on-click #(on-change (assoc item :archived true) [:span [text :t.administration/archive]
+                                                       " \"" [get-localized-title-for-anything item] "\""])}
    (text :t.administration/archive)])
 
 (defn- unarchive-button [item on-change]
   [:button.btn.btn-primary.button-min-width
    {:type :button
-    :on-click #(on-change (assoc item :archived false) [text :t.administration/unarchive])}
+    :on-click #(on-change (assoc item :archived false) [:span [text :t.administration/unarchive]
+                                                        " \"" [get-localized-title-for-anything item] "\""])}
    (text :t.administration/unarchive)])
 
 ;; TODO consider naming archived-toggle-button
@@ -82,33 +96,35 @@
               (text :t.administration/catalogue-item) ": "
               [:a {:target :_blank
                    :href (str "/administration/catalogue-items/" (:id ci))}
-               (get-localized-title ci language)]]))
+               (get-localized-title-for-anything ci language)]]))
      (into [:ul]
            (for [f forms]
              [:li
               (text :t.administration/form) ": "
               [:a {:target :_blank
                    :href (str "/administration/forms/" (:id f))}
-               (:form/title f)]]))
+               (get-localized-title-for-anything f language)]]))
      (into [:ul]
            (for [lic licenses]
              [:li
               (text :t.administration/license) ": "
               [:a {:target :_blank
                    :href (str "/administration/licenses/" (:id lic))}
-               (get-localized-title lic language)]]))
+               (get-localized-title-for-anything lic language)]]))
      (into [:ul]
            (for [r resources]
              [:li
               (text :t.administration/resource) ": "
               [:a {:target :_blank
-                   :href (str "/administration/resources/" (:id r))} (:resid r)]]))
+                   :href (str "/administration/resources/" (:id r))}
+               (get-localized-title-for-anything r language)]]))
      (into [:ul]
            (for [w workflows]
              [:li
               (text :t.administration/workflow) ": "
               [:a {:target :_blank
-                   :href (str "/administration/workflows/" (:id w))} (:title w)]]))]))
+                   :href (str "/administration/workflows/" (:id w))}
+               (get-localized-title-for-anything w language)]]))]))
 
 (defn format-update-failure [{:keys [errors]}]
   (into [:div]

test/clj/rems/api/services/test_catalogue.clj

@@ -20,7 +20,7 @@
   (let [owner "owner"
         form-id (test-data/create-form! {})
         lic-id (test-data/create-license! {})
-        res-id (test-data/create-resource! {:license-ids [lic-id]})
+        res-id (test-data/create-resource! {:resource-ext-id "ext" :license-ids [lic-id]})
         workflow-id (test-data/create-workflow! {})
         item-id (test-data/create-catalogue-item! {:resource-id res-id
                                                    :form-id form-id
@@ -116,11 +116,11 @@
       (archive-catalogue-item! true)
       (archive-resource! true)
       (archive-license! true)
-      (let [errors (:errors (archive-catalogue-item! false))]
-        ;; TODO indirect catalogue item -> resource -> license dep not tracked right now
-        (is (= #{:t.administration.errors/resource-archived
-                 #_:t.administration.errors/license-archived}
-               (set (mapv :type errors)))))
+      ;; TODO indirect catalogue item -> resource -> license dep not tracked right now
+      (is (= {:success false
+              :errors [{:type :t.administration.errors/dependencies-archived
+                        :resources [{:id res-id :resid "ext"}]}]}
+             (archive-catalogue-item! false)))
       (archive-license! false)
       (archive-resource! false)
       (is (:success (archive-catalogue-item! true))))))