Repository: CSCfi/rems
Pull request #1574 (2019-09-03 10:05:25)

Change licenses API to have /enabled and /archived instead of /update
Part of #1548

# Definition of Done / Review checklist

## Reviewability
- [ ] link to issue
- [ ] note if PR is on top of other PR
- [ ] note if related change in rems-deploy repo
- [ ] consider adding screenshots for ease of review

## API
- [ ] API is documented and shows up in Swagger UI
- [ ] API is backwards compatible or completely new
- [ ] Events are backwards compatible

## Documentation
- [ ] update changelog if necessary
- [ ] add or update docstrings for namespaces and functions
- [ ] components are added to guide page
- [ ] documentation _at least_ for config options (i.e. docs folder)
- [ ] ADR for major architectural decisions or experiments

## Different installations
- [ ] new configuration options added to rems-deploy repository
- [ ] instance specific translations (i.e. LBR kielivara)

## Testing
- [ ] complex logic is unit tested
- [ ] valuable features are integration / browser / acceptance tested automatically

## Accessibility
- [ ] all icons have the aria-label attribute
- [ ] all fields have a label
- [ ] errors are linked to fields with aria-describedby
- [ ] contrast is checked
- [ ] conscious decision about where to move focus after an action

## Follow-up
- [ ] new tasks are created for pending or remaining tasks
- [ ] no critical TODOs left to implement



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a61cdfc3a..7deedfb29 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,12 +18,12 @@ Changes since v2.4
 - API for creating catalogue item and its localizations has been changed.
   There is now a single API call that is used to create both a catalogue
   item and the localizations, namely, /api/catalogue-items/create.
-- APIs for editing workflow, form and resource have been changed. The API endpoint
-  for editing content of the workflow is now /edit (prefixed with /api/workflows).
-  The endpoint for archiving or unarchiving a workflow, a form, or a resource
-  is /archived and the endpoint for enabling or disabling a workflow, a form, or a
-  resource is /enabled, prefixed with /api/workflows, /api/forms, or api/resources,
-  respectively.
+- APIs for editing workflow, form, resource, and license have been changed. The
+  API endpoint for editing content of the workflow is now /edit (prefixed with
+  /api/workflows). The endpoint for archiving or unarchiving a workflow, a form,
+  a resource, or a license is /archived and the endpoint for enabling or disabling
+  a workflow, a form, a resource, or a license is /enabled, prefixed with
+  /api/workflows, /api/forms, /api/resources, or /api/licenses, respectively.
 - API endpoint for editing forms has been changed from
   /api/forms/[form-id]/edit to /api/forms/edit.
 
diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql
index df379cf82..3a3cf55f4 100644
--- a/resources/sql/queries.sql
+++ b/resources/sql/queries.sql
@@ -265,9 +265,14 @@ INSERT INTO license
 VALUES
 (:owneruserid, :modifieruserid, :type::license_type)
 
--- :name set-license-state! :!
+-- :name set-license-enabled! :!
 UPDATE license
-SET (enabled, archived) = (:enabled, :archived)
+SET (enabled) = (:enabled)
+WHERE id = :id;
+
+-- :name set-license-archived! :!
+UPDATE license
+SET (archived) = (:archived)
 WHERE id = :id;
 
 -- :name create-license-attachment! :insert
diff --git a/src/clj/rems/api/licenses.clj b/src/clj/rems/api/licenses.clj
index 829da377c..1f2efe4ba 100644
--- a/src/clj/rems/api/licenses.clj
+++ b/src/clj/rems/api/licenses.clj
@@ -51,12 +51,19 @@
       :return CreateLicenseResponse
       (ok (licenses/create-license! command (getx-user-id))))
 
-    (PUT "/update" []
-      :summary "Update license"
+    (PUT "/archived" []
+      :summary "Archive or unarchive license"
       :roles #{:owner}
-      :body [command UpdateStateCommand]
+      :body [command ArchivedCommand]
       :return SuccessResponse
-      (ok (licenses/update-license! command)))
+      (ok (licenses/set-license-archived! command)))
+
+    (PUT "/enabled" []
+      :summary "Enable or disable license"
+      :roles #{:owner}
+      :body [command EnabledCommand]
+      :return SuccessResponse
+      (ok (licenses/set-license-enabled! command)))
 
     (POST "/add_attachment" []
       :summary "Add an attachment file that will be used in a license"
diff --git a/src/clj/rems/api/services/licenses.clj b/src/clj/rems/api/services/licenses.clj
index c06ae1f82..fe74f216a 100644
--- a/src/clj/rems/api/services/licenses.clj
+++ b/src/clj/rems/api/services/licenses.clj
@@ -53,14 +53,19 @@
       {:resources resources
        :workflows workflows})))
 
-(defn update-license! [command]
-  (let [usage (get-license-usage (:id command))]
-    (if (and (:archived command) usage)
+(defn set-license-enabled! [command]
+  (db/set-license-enabled! (select-keys command [:id :enabled]))
+  {:success true})
+
+(defn set-license-archived! [{:keys [id archived]}]
+  (let [usage (get-license-usage id)]
+    (if (and archived usage)
       {:success false
        :errors [(merge {:type :t.administration.errors/license-in-use}
                        usage)]}
       (do
-        (db/set-license-state! command)
+        (db/set-license-archived! {:id id
+                                   :archived archived})
         {:success true}))))
 
 (defn get-license
diff --git a/src/cljs/rems/administration/license.cljs b/src/cljs/rems/administration/license.cljs
index 4db206137..638d8f06f 100644
--- a/src/cljs/rems/administration/license.cljs
+++ b/src/cljs/rems/administration/license.cljs
@@ -77,8 +77,8 @@
    (let [id (:id license)]
      [:div.col.commands
       [back-button]
-      [status-flags/enabled-toggle license #(rf/dispatch [:rems.administration.licenses/update-license %1 %2 [::enter-page id]])]
-      [status-flags/archived-toggle license #(rf/dispatch [:rems.administration.licenses/update-license %1 %2 [::enter-page id]])]])])
+      [status-flags/enabled-toggle license #(rf/dispatch [:rems.administration.licenses/set-license-enabled %1 %2 [::enter-page id]])]
+      [status-flags/archived-toggle license #(rf/dispatch [:rems.administration.licenses/set-license-archived %1 %2 [::enter-page id]])]])])
 
 ;; XXX: Duplicates much of license-view. One notable difference is that
 ;;      here the license text is only shown in the current language.
diff --git a/src/cljs/rems/administration/licenses.cljs b/src/cljs/rems/administration/licenses.cljs
index 47c3d1411..2fa7587e1 100644
--- a/src/cljs/rems/administration/licenses.cljs
+++ b/src/cljs/rems/administration/licenses.cljs
@@ -36,10 +36,19 @@
 (rf/reg-sub ::loading? (fn [db _] (::loading? db)))
 
 (rf/reg-event-fx
- ::update-license
+ ::set-license-archived
  (fn [_ [_ item description dispatch-on-finished]]
-   (put! "/api/licenses/update"
-         {:params (select-keys item [:id :enabled :archived])
+   (put! "/api/licenses/archived"
+         {:params (select-keys item [:id :archived])
+          :handler (flash-message/status-update-handler description #(rf/dispatch dispatch-on-finished))
+          :error-handler (flash-message/default-error-handler description)})
+   {}))
+
+(rf/reg-event-fx
+ ::set-license-enabled
+ (fn [_ [_ item description dispatch-on-finished]]
+   (put! "/api/licenses/enabled"
+         {:params (select-keys item [:id :enabled])
           :handler (flash-message/status-update-handler description #(rf/dispatch dispatch-on-finished))
           :error-handler (flash-message/default-error-handler description)})
    {}))
@@ -84,8 +93,8 @@
                       :sort-value (if checked? 1 2)})
            :commands {:td [:td.commands
                            [to-view-license (:id license)]
-                           [status-flags/enabled-toggle license #(rf/dispatch [::update-license %1 %2 [::fetch-licenses]])]
-                           [status-flags/archived-toggle license #(rf/dispatch [::update-license %1 %2 [::fetch-licenses]])]]}})
+                           [status-flags/enabled-toggle license #(rf/dispatch [::set-license-enabled %1 %2 [::fetch-licenses]])]
+                           [status-flags/archived-toggle license #(rf/dispatch [::set-license-archived %1 %2 [::fetch-licenses]])]]}})
         licenses)))
 
 (defn- licenses-list []


Test patch:
diff --git a/src/clj/rems/db/test_data.clj b/src/clj/rems/db/test_data.clj
index 25b779d0c..f6b0940ce 100644
--- a/src/clj/rems/db/test_data.clj
+++ b/src/clj/rems/db/test_data.clj
@@ -217,7 +217,7 @@
                                              :fi "Käytöstä poistettu lisenssi"}
                              :license/link {:en "http://disabled"
                                             :fi "http://disabled"}})]
-    (db/set-license-state! {:id id :enabled false :archived false})))
+    (db/set-license-enabled! {:id id :enabled false})))
 
 (defn- create-basic-form!
   "Creates a bilingual form with all supported field types. Returns id of the form meta."
diff --git a/test/clj/rems/api/services/test_catalogue.clj b/test/clj/rems/api/services/test_catalogue.clj
index abe0c9284..1e3ee306b 100644
--- a/test/clj/rems/api/services/test_catalogue.clj
+++ b/test/clj/rems/api/services/test_catalogue.clj
@@ -32,9 +32,8 @@
                                             :archived %})
         archive-form! #(form/set-form-archived! {:id form-id
                                                  :archived %})
-        archive-license! #(licenses/update-license! {:id lic-id
-                                                     :enabled true
-                                                     :archived %})
+        archive-license! #(licenses/set-license-archived! {:id lic-id
+                                                           :archived %})
         archive-resource! #(resource/set-resource-archived! {:id res-id
                                                              :archived %})
         archive-workflow! #(workflow/set-workflow-archived! {:id workflow-id
diff --git a/test/clj/rems/api/services/test_resources.clj b/test/clj/rems/api/services/test_resources.clj
index e76833be2..da7020a06 100644
--- a/test/clj/rems/api/services/test_resources.clj
+++ b/test/clj/rems/api/services/test_resources.clj
@@ -17,9 +17,8 @@
         res-id (test-data/create-resource! {:license-ids [lic-id]})
         res-id2 (test-data/create-resource! {})
 
-        archive-license! #(licenses/update-license! {:id lic-id
-                                                     :enabled true
-                                                     :archived %})]
+        archive-license! #(licenses/set-license-archived! {:id lic-id
+                                                           :archived %})]
 
     (testing "new resources are enabled and not archived"
       (is (= {:enabled true
diff --git a/test/clj/rems/api/test_administration.clj b/test/clj/rems/api/test_administration.clj
index be1ec0d93..c94e49cca 100644
--- a/test/clj/rems/api/test_administration.clj
+++ b/test/clj/rems/api/test_administration.clj
@@ -60,12 +60,15 @@
                                   :enabled %}
                                  api-key user-id)
 
-        update-license! (fn [{:keys [enabled archived]}]
-                          (api-call :put "/api/licenses/update"
+        license-archived! #(api-call :put "/api/licenses/archived"
+                                     {:id license-id
+                                      :archived %}
+                                     api-key user-id)
+
+        license-enabled! #(api-call :put "/api/licenses/enabled"
                                     {:id license-id
-                                     :enabled enabled
-                                     :archived archived}
-                                    api-key user-id))
+                                     :enabled %}
+                                    api-key user-id)
 
         workflow-archived! #(api-call :put "/api/workflows/archived"
                                       {:id workflow-id
@@ -116,10 +119,10 @@
                (:errors resp)))))
 
     (testing "can disable a license"
-      (is (:success (update-license! {:enabled false :archived false}))))
+      (is (:success (license-enabled! false))))
 
     (testing "can't archive a license that's in use"
-      (let [resp (update-license! {:enabled true :archived true})]
+      (let [resp (license-archived! true)]
         (is (false? (:success resp)))
         (is (= [{:type "t.administration.errors/license-in-use"
                  :resources [{:id resource-id :resid "test"}]
@@ -139,7 +142,7 @@
       (is (:success (workflow-archived! true))))
 
     (testing "can archive a license that's not in use"
-      (is (= {:success true} (update-license! {:enabled true :archived true}))))
+      (is (= {:success true} (license-archived! true))))
 
     (testing "cannot unarchive a resource with an archived license"
       (let [resp (resource-archived! false)]
diff --git a/test/clj/rems/api/test_workflows.clj b/test/clj/rems/api/test_workflows.clj
index cc8af54a9..a8fafefb4 100644
--- a/test/clj/rems/api/test_workflows.clj
+++ b/test/clj/rems/api/test_workflows.clj
@@ -95,9 +95,8 @@
         _ (db/create-workflow-license! {:wfid wfid :licid lic-id})
 
         fetch #(fetch api-key user-id wfid)
-        archive-license! #(licenses/update-license! {:id lic-id
-                                                     :enabled true
-                                                     :archived %})
+        archive-license! #(licenses/set-license-archived! {:id lic-id
+                                                           :archived %})
         set-enabled! #(-> (request :put "/api/workflows/enabled")
                           (json-body {:id wfid
                                       :enabled %})