Merge pull request #2973 from CSCfi/2158/workflow-licenses

Workflow licenses

CHANGELOG.md

@@ -15,10 +15,15 @@ have notable changes.
 
 Changes since v2.28
 
+**NB: This release contains migrations!**
+**NB: `workflow_licenses` table has been removed and the data is migrated to `workflow` table.**
+
+### Additions
+- Licenses can now be added to workflows through user interface and API. Workflow licenses are included in applications, similar to resource licenses. (#2158)
+
 ## v2.28 "Porkkalankatu" 2022-08-24
 
 **NB: This release contains migrations!**
-
 **NB: One of the migrations fixes organizations, that could be broken by previous features.**
 
 ### Additions

resources/migrations/20220621104646-refactor-workflow-licenses.edn

@@ -0,0 +1,3 @@
+{:ns rems.migrations.refactor-workflow-licenses
+ :up-fn migrate-up
+ :down-fn nil}

resources/migrations/20220711142631-remove-workflow-licenses-table.down.sql

@@ -0,0 +1,7 @@
+CREATE TABLE if not exists workflow_licenses (
+  id serial NOT NULL PRIMARY KEY,
+  wfId integer DEFAULT NULL,
+  licId integer DEFAULT NULL,
+  CONSTRAINT workflow_licenses_ibfk_1 FOREIGN KEY (wfId) REFERENCES workflow (id),
+  CONSTRAINT workflow_licenses_ibfk_2 FOREIGN KEY (licId) REFERENCES license (id)
+);

resources/migrations/20220711142631-remove-workflow-licenses-table.up.sql

@@ -0,0 +1 @@
+DROP TABLE IF EXISTS workflow_licenses;

resources/sql/queries.sql

@@ -405,24 +405,13 @@ SET
   id = id
 WHERE id = :id;
 
--- :name create-workflow-license! :insert
-INSERT INTO workflow_licenses
-(wfid, licid)
-VALUES
-(:wfid, :licid);
-
 -- TODO: consider renaming this to link-resource-license!
 -- :name create-resource-license! :insert
 INSERT INTO resource_licenses
 (resid, licid)
 VALUES
 (:resid, :licid);
 
--- :name get-workflow-licenses :? :*
-SELECT licid
-FROM workflow_licenses
-WHERE wfid = :wfid
-
 -- :name get-workflow :? :1
 SELECT
   wf.id, wf.organization, wf.title,
@@ -448,16 +437,10 @@ FROM workflow wf;
 
 -- :name get-licenses :? :*
 -- :doc
--- - Gets application licenses by workflow and catalogue item ids
--- - :wfid workflow id for workflow licenses
+-- - Gets application licenses by catalogue item ids
 -- - :items vector of catalogue item ids for resource licenses
 SELECT lic.id, lic.type, lic.enabled, lic.archived, lic.organization
 FROM license lic
-INNER JOIN workflow_licenses wl ON lic.id = wl.licid
-WHERE wl.wfid = :wfid
-UNION
-SELECT lic.id, lic.type, lic.enabled, lic.archived, lic.organization
-FROM license lic
 INNER JOIN resource_licenses rl ON lic.id = rl.licid
 INNER JOIN catalogue_item item ON (item.resid = rl.resid)
 WHERE item.id IN (:v*:items)
@@ -485,9 +468,6 @@ FROM license_localization;
 -- :name get-resources-for-license :? :*
 SELECT resid FROM resource_licenses WHERE licid = :id;
 
--- :name get-workflows-for-license :? :*
-SELECT wfid FROM workflow_licenses WHERE licid = :id;
-
 -- :name get-all-roles :? :*
 SELECT userid, role
 FROM roles;

src/clj/rems/api/schema.clj

@@ -107,22 +107,22 @@
    (s/optional-key :resource/duo) {:duo/codes [schema-base/DuoCodeFull]}})
 
 (s/defschema V2License
-  {:license/id s/Int
-   :license/type (s/enum :text :link :attachment)
-   :license/title schema-base/LocalizedString
-   (s/optional-key :license/link) schema-base/LocalizedString
-   (s/optional-key :license/text) schema-base/LocalizedString
-   (s/optional-key :license/attachment-id) schema-base/LocalizedInt
-   (s/optional-key :license/attachment-filename) schema-base/LocalizedString
-   :license/enabled s/Bool
-   :license/archived s/Bool})
+  (merge
+   schema-base/LicenseId
+   {:license/type (s/enum :text :link :attachment)
+    :license/title schema-base/LocalizedString
+    (s/optional-key :license/link) schema-base/LocalizedString
+    (s/optional-key :license/text) schema-base/LocalizedString
+    (s/optional-key :license/attachment-id) schema-base/LocalizedInt
+    (s/optional-key :license/attachment-filename) schema-base/LocalizedString
+    :license/enabled s/Bool
+    :license/archived s/Bool}))
 
 (s/defschema Workflow
   {:id s/Int
    :organization schema-base/OrganizationOverview
    :title s/Str
    :workflow s/Any
-   :licenses [License]
    :enabled s/Bool
    :archived s/Bool})
 

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

@@ -35,7 +35,7 @@
     (-> item
         organizations/join-organization
         (update :categories category/enrich-categories)
-         ;; not used at the moment
+        ;; XXX: not used at the moment
         #_licenses/join-catalogue-item-licenses
         #_(transform [:licenses ALL] organizations/join-organization))))
 

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

@@ -50,8 +50,9 @@
 
    (for [workflow (workflow/get-workflows {})
          dep (concat
-              (mapv (fn [lic] {:license/id (:id lic)}) (:licenses workflow))
-              (:forms (:workflow workflow))
+              (->> (get-in workflow [:workflow :licenses])
+                   (mapv #(select-keys % [:license/id])))
+              (get-in workflow [:workflow :forms])
               [(:organization workflow)])]
      {:from {:workflow/id (:id workflow)} :to dep})
 

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

@@ -1,58 +1,52 @@
 (ns rems.api.services.workflow
-  (:require [com.rpl.specter :refer [ALL transform]]
-            [rems.api.services.dependencies :as dependencies]
+  (:require [rems.api.services.dependencies :as dependencies]
             [rems.api.services.util :as util]
             [rems.db.applications :as applications]
             [rems.db.core :as db]
             [rems.db.form :as form]
+            [rems.db.licenses :as licenses]
             [rems.db.organizations :as organizations]
             [rems.db.users :as users]
-            [rems.db.workflow :as workflow]
-            [rems.json :as json]))
+            [rems.db.workflow :as workflow]))
 
 (defn invalid-forms-error [forms]
-  (let [invalid (seq (remove (comp form/get-form-template :form/id) forms))]
-    (when invalid
-      {:success false
-       :errors [{:type :invalid-form
-                 :forms invalid}]})))
+  (when-some [invalid (seq (remove (comp form/get-form-template :form/id) forms))]
+    {:success false
+     :errors [{:type :invalid-form
+               :forms invalid}]}))
 
 (defn invalid-users-error [userids]
-  (let [invalid (seq (remove users/user-exists? userids))]
-    (when invalid
-      {:success false
-       :errors [{:type :invalid-user
-                 :users invalid}]})))
+  (when-some [invalid (seq (remove users/user-exists? userids))]
+    {:success false
+     :errors [{:type :invalid-user
+               :users invalid}]}))
 
-(defn create-workflow! [{:keys [organization handlers forms] :as cmd}]
+(defn invalid-licenses-error [licenses]
+  (when-some [invalid (seq (remove (comp licenses/license-exists? :license/id) licenses))]
+    {:success false
+     :errors [{:type :invalid-license
+               :licenses invalid}]}))
+
+(defn create-workflow! [{:keys [organization handlers licenses forms] :as cmd}]
   (util/check-allowed-organization! organization)
   (or (invalid-users-error handlers)
       (invalid-forms-error forms)
-      (let [id (workflow/create-workflow! cmd)]
+      (invalid-licenses-error licenses)
+      (let [id (workflow/create-workflow! (update cmd :licenses #(map :license/id %)))]
         (dependencies/reset-cache!)
         {:success (not (nil? id))
          :id id})))
 
-(defn- unrich-workflow [workflow]
-  ;; TODO: keep handlers always in the same format, to avoid this conversion (we can ignore extra keys)
-  (if (get-in workflow [:workflow :handlers])
-    (update-in workflow [:workflow :handlers] #(map :userid %))
-    workflow))
-
-;; TODO: use rems.db.workflow/edit-workflow! and don't go directly to db fns, same for other fns
-(defn edit-workflow! [{:keys [id organization title handlers]}]
-  (let [workflow (unrich-workflow (workflow/get-workflow id))
-        workflow-body (cond-> (:workflow workflow)
-                        handlers (assoc :handlers handlers))]
+(defn edit-workflow! [{:keys [id organization handlers] :as cmd}]
+  (let [workflow (workflow/get-workflow id)]
     (util/check-allowed-organization! (:organization workflow))
     (when organization
       (util/check-allowed-organization! organization))
-    (db/edit-workflow! {:id id
-                        :title title
-                        :organization (:organization/id organization)
-                        :workflow (json/generate-string workflow-body)}))
-  (applications/reload-cache!)
-  {:success true})
+    (or (invalid-users-error handlers)
+        (do
+          (workflow/edit-workflow! (update cmd :licenses #(map :license/id %)))
+          (applications/reload-cache!)
+          {:success true}))))
 
 (defn set-workflow-enabled! [{:keys [id enabled]}]
   (util/check-allowed-organization! (:organization (workflow/get-workflow id)))
@@ -69,18 +63,21 @@
 
 ;; TODO more systematic joining for these needed. Now we just add the title for the UI
 (defn- enrich-workflow-form [item]
-  (select-keys (dependencies/enrich-dependency item) [:form/id :form/internal-name :form/external-title]))
+  (-> item
+      dependencies/enrich-dependency
+      (select-keys [:form/id :form/internal-name :form/external-title])))
 
-(defn- join-workflow-forms [workflow]
-  (update-in workflow [:workflow :forms] (partial mapv enrich-workflow-form)))
+(defn- enrich-workflow-license [item]
+  (-> item
+      licenses/join-license
+      organizations/join-organization))
 
 (defn- join-dependencies [workflow]
   (when workflow
-    (->> workflow
-         join-workflow-forms
-         organizations/join-organization
-         workflow/join-workflow-licenses
-         (transform [:licenses ALL] organizations/join-organization))))
+    (-> workflow
+        organizations/join-organization
+        (update-in [:workflow :forms] (partial map enrich-workflow-form))
+        (update-in [:workflow :licenses] (partial map enrich-workflow-license)))))
 
 (defn get-workflow [id]
   (->> (workflow/get-workflow id)
@@ -99,3 +96,4 @@
                            (get-in wf [:workflow :handlers]))
                          workflows)]
     (->> handlers distinct (sort-by :userid))))
+

src/clj/rems/api/workflows.clj

@@ -6,7 +6,6 @@
             [rems.application.events :as events]
             [rems.common.roles :refer [+admin-read-roles+ +admin-write-roles+]]
             [rems.schema-base :as schema-base]
-            [rems.util :refer [getx-user-id]]
             [ring.util.http-response :refer :all]
             [schema.core :as s]))
 
@@ -15,7 +14,8 @@
    :title s/Str
    (s/optional-key :forms) [{:form/id s/Int}]
    :type (apply s/enum events/workflow-types) ; TODO: exclude master workflow?
-   (s/optional-key :handlers) [schema-base/UserId]})
+   (s/optional-key :handlers) [schema-base/UserId]
+   (s/optional-key :licenses) [schema-base/LicenseId]})
 
 (s/defschema EditWorkflowCommand
   {:id s/Int

src/clj/rems/application/commands.clj

@@ -322,9 +322,8 @@
 (defn- build-licenses-list [catalogue-item-ids {:keys [get-catalogue-item-licenses]}]
   (->> catalogue-item-ids
        (mapcat get-catalogue-item-licenses)
-       distinct
-       (mapv (fn [license]
-               {:license/id (:id license)}))))
+       (distinct-by :license/id)
+       (mapv #(select-keys % [:license/id]))))
 
 (defn- entitlement-end-not-in-future-error
   "Checks that entitlement date time, if provided, is in the future."

src/clj/rems/application/model.clj

@@ -2,7 +2,7 @@
   (:require [clojure.set :as set]
             [clojure.test :refer [deftest is testing]]
             [com.rpl.specter :refer [ALL select transform]]
-            [medley.core :refer [find-first map-vals update-existing update-existing-in]]
+            [medley.core :refer [distinct-by find-first map-vals update-existing update-existing-in]]
             [rems.application.events :as events]
             [rems.application.master-workflow :as master-workflow]
             [rems.common.application-util :as application-util]
@@ -507,6 +507,16 @@
       (-> application
           (assoc-in [:application/duo :duo/matches] matches)))))
 
+(defn- enrich-workflow-licenses [application get-workflow]
+  (let [wf (get-workflow (get-in application [:application/workflow :workflow/id]))]
+    (if-some [workflow-licenses (seq (get-in wf [:workflow :licenses]))]
+      (-> application
+          (update :application/licenses (fn [app-licenses]
+                                          (->> workflow-licenses
+                                               (into app-licenses)
+                                               (distinct-by :license/id)))))
+      application)))
+
 (defn- enrich-licenses [app-licenses get-license]
   (let [rich-licenses (->> app-licenses
                            (map :license/id)
@@ -676,6 +686,7 @@
       (update :application/resources enrich-resources get-catalogue-item)
       enrich-application-duo-matches ; uses enriched resources
       (update-existing-in [:application/duo :duo/codes] duo/enrich-duo-codes)
+      (enrich-workflow-licenses get-workflow)
       (update :application/licenses enrich-licenses get-license)
       (update :application/events (partial mapv #(enrich-event % get-user get-catalogue-item)))
       (assoc :application/applicant (get-user (get-in application [:application/applicant :userid])))

src/clj/rems/db/applications.clj

@@ -6,12 +6,12 @@
             [clojure.test :refer [deftest is]]
             [clojure.tools.logging :as log]
             [conman.core :as conman]
-            [medley.core :refer [map-vals]]
+            [medley.core :refer [distinct-by map-vals]]
             [mount.core :as mount]
             [rems.application.events-cache :as events-cache]
             [rems.application.model :as model]
             [rems.auth.util :refer [throw-forbidden]]
-            [rems.common.util :refer [conj-set]]
+            [rems.common.util :refer [conj-set keep-keys]]
             [rems.config :refer [env]]
             [rems.db.attachments :as attachments]
             [rems.db.blacklist :as blacklist]
@@ -53,9 +53,13 @@
 ;;; Running commands
 
 (defn get-catalogue-item-licenses [catalogue-item-id]
-  (db/get-licenses
-   {:wfid (:wfid (catalogue/get-localized-catalogue-item catalogue-item-id {}))
-    :items [catalogue-item-id]}))
+  (let [item (catalogue/get-localized-catalogue-item catalogue-item-id {})
+        workflow-licenses (-> (workflow/get-workflow (:wfid item))
+                              (get-in [:workflow :licenses]))]
+    (->> (licenses/get-licenses {:items [catalogue-item-id]})
+         (keep-keys {:id :license/id})
+         (into workflow-licenses)
+         (distinct-by :license/id))))
 
 (defn get-application-by-invitation-token [invitation-token]
   (:id (db/get-application-by-invitation-token {:token invitation-token})))

src/clj/rems/db/licenses.clj

@@ -1,6 +1,6 @@
 (ns rems.db.licenses
   "querying localized licenses"
-  (:require [rems.common.util :refer [distinct-by]]
+  (:require [medley.core :refer [distinct-by]]
             [rems.db.core :as db]))
 
 (defn- format-license [license]
@@ -55,7 +55,6 @@
 
 (defn get-licenses
   "Get licenses. Params map can contain:
-     :wfid -- workflow to get workflow licenses for
      :items -- sequence of catalogue items to get resource licenses for"
   [params]
   (->> (db/get-licenses params)
@@ -67,7 +66,13 @@
   (assoc x :licenses (get-resource-licenses (:id x))))
 
 (defn join-catalogue-item-licenses [item]
-  (assoc item
-         :licenses
-         (get-licenses {:wfid (:wfid item)
-                        :items [(:id item)]})))
+  (assoc item :licenses (get-licenses {:items [(:id item)]})))
+
+(defn join-license [{:keys [license/id] :as x}]
+  (-> (get-license id)
+      (dissoc :id)
+      (merge x)))
+
+(defn license-exists? [id]
+  (some? (db/get-license {:id id})))
+