diff --git a/backend/src/gpml/db/stakeholder.sql b/backend/src/gpml/db/stakeholder.sql index 55bb580b4..f6ee7149f 100644 --- a/backend/src/gpml/db/stakeholder.sql +++ b/backend/src/gpml/db/stakeholder.sql @@ -174,6 +174,7 @@ update stakeholder set --~ (when (contains? params :idp_usernames) "idp_usernames= :idp_usernames::jsonb,") --~ (when (contains? params :picture_id) "picture_id= :picture_id,") --~ (when (contains? params :cv_id) "cv_id= :cv_id, ") +--~ (when (contains? params :review_status) "review_status= :review_status::review_status,") modified = now() where id = :id; diff --git a/backend/src/gpml/handler/stakeholder.clj b/backend/src/gpml/handler/stakeholder.clj index a5f138d12..292f7ae39 100644 --- a/backend/src/gpml/handler/stakeholder.clj +++ b/backend/src/gpml/handler/stakeholder.clj @@ -201,11 +201,12 @@ (fn [{:keys [body-params headers user]}] (try (let [tags (handler.stakeholder.tag/api-stakeholder-tags->stakeholder-tags body-params)] - (update-stakeholder config (assoc body-params - :id (:id user) - :picture {:payload (:picture body-params) - :user-agent (get headers "user-agent")} - :tags tags)) + (update-stakeholder config (-> body-params + (assoc :id (:id user) + :picture {:payload (:picture body-params) + :user-agent (get headers "user-agent")} + :tags tags) + (dissoc :review_status))) (resp/status {:success? true} 204)) (catch Exception e (log logger :error ::failed-to-update-stakeholder {:exception-message (.getMessage e)}) diff --git a/backend/src/gpml/handler/stakeholder/expert.clj b/backend/src/gpml/handler/stakeholder/expert.clj index 67eac4688..4105f54cb 100644 --- a/backend/src/gpml/handler/stakeholder/expert.clj +++ b/backend/src/gpml/handler/stakeholder/expert.clj @@ -8,6 +8,7 @@ [gpml.handler.resource.permission :as h.r.permission] [gpml.handler.responses :as r] [gpml.handler.stakeholder.tag :as handler.stakeholder.tag] + [gpml.service.permissions :as srv.permissions] [gpml.util :as util] [gpml.util.email :as email] [gpml.util.postgresql :as pg-util] @@ -179,6 +180,8 @@ (log logger :error ::send-invitation-emails-failed {:exception-message (.getMessage e) :context-data {:invitations invitations}})))) +;; TODO: Improve how we deal with errors here, since we should rollback invitation processes one by one, as otherwise +;; we might rollback all of them while the notifications or some of them have been sent already. (defn- invite-experts [{:keys [db mailjet-config logger] :as config} {{:keys [body]} :parameters}] @@ -202,21 +205,48 @@ logger mailjet-config {:tags (handler.stakeholder.tag/api-stakeholder-tags->stakeholder-tags {:expertise expertise}) - :stakeholder-id stakeholder-id})) + :stakeholder-id stakeholder-id}) + (let [{:keys [success?]} (srv.permissions/create-resource-context + {:conn conn + :logger logger} + {:context-type :stakeholder + :resource-id stakeholder-id})] + (if-not success? + (throw (ex-info + "Failing creating stakeholder rbac context" + {:reason :failing-creating-stakeholder-rbac-context + :stakeholder-id stakeholder-id})) + (let [role-assignments [{:role-name :unapproved-user + :context-type :application + :resource-id srv.permissions/root-app-resource-id + :user-id stakeholder-id}] + result (first (srv.permissions/assign-roles-to-users + {:conn conn + :logger logger} + role-assignments))] + (when-not (:success? result) + (throw (ex-info + "Failing assigning roles to stakeholder" + {:reason :failing-assigning-roles-to-stakeholder + :role-assignments role-assignments}))))))) (future (send-invitation-emails config invitations)) - (resp/response {:success? true - :invited-experts (map #(update % :id str) invitations)}))) - (catch Exception e - (log logger :error ::invite-experts-error {:exception-message (.getMessage e)}) - (if (instance? SQLException e) - {:status 500 - :body {:success? false - :reason (if (= :unique-constraint-violation (pg-util/get-sql-state e)) - :stakeholder-email-already-exists - (pg-util/get-sql-state e))}} - {:status 500 - :body {:success? false - :reason :could-not-create-expert}})))) + (r/ok {:success? true + :invited-experts (map #(update % :id str) invitations)}))) + (catch Throwable t + (log logger :error ::invite-experts-error {:exception-message (ex-message t) + :exception-data (ex-data t) + :stack-trace (.getStackTrace t) + :context-data body}) + (if (instance? SQLException t) + (r/server-error + {:success? false + :reason (if (= :unique-constraint-violation (pg-util/get-sql-state t)) + :stakeholder-email-already-exists + (pg-util/get-sql-state t))}) + (let [{:keys [reason]} (ex-data t)] + (r/server-error + {:success? false + :reason reason})))))) (defn- generate-admins-expert-suggestion-text [{:keys [email expertise suggested_expertise] :as expert} diff --git a/backend/src/gpml/service/stakeholder.clj b/backend/src/gpml/service/stakeholder.clj index 43c85ef4b..9dc0a4b62 100644 --- a/backend/src/gpml/service/stakeholder.clj +++ b/backend/src/gpml/service/stakeholder.clj @@ -1,5 +1,6 @@ (ns gpml.service.stakeholder - (:require [gpml.db.organisation :as db.organisation] + (:require [duct.logger :refer [log]] + [gpml.db.organisation :as db.organisation] [gpml.db.resource.tag :as db.resource.tag] [gpml.db.stakeholder :as db.stakeholder] [gpml.domain.file :as dom.file] @@ -318,12 +319,89 @@ (fn get-experts [{:keys [old-stakeholder] :as context}] (let [expert? (seq (db.stakeholder/get-experts conn {:filters {:ids [(:id old-stakeholder)]} - :page-size 0 + :page-size 1 :offset 0}))] - (assoc context :expert? expert?)))} + (assoc context :expert? expert?))) + :rollback-fn + (fn rollback-unassign-unapproved-user-role + [{:keys [old-stakeholder invited-expert?] :as context}] + (if invited-expert? + (let [role-assignments [{:role-name :unapproved-user + :context-type :application + :resource-id srv.permissions/root-app-resource-id + :user-id (:id old-stakeholder)}] + result (first (srv.permissions/assign-roles-to-users + {:conn conn + :logger logger} + role-assignments))] + (when-not (:success? result) + (log logger :error ::rollback-unassign-unapproved-user-role {:reason result}))) + (dissoc context :invited-expert?)))} + {:txn-fn + (fn unassign-unapproved-user-role + [{:keys [old-stakeholder expert?] :as context}] + (if (and expert? + (= (:review-status old-stakeholder) "INVITED")) + (let [role-unassignments [{:role-name :unapproved-user + :context-type :application + :resource-id srv.permissions/root-app-resource-id + :user-id (:id old-stakeholder)}] + result (first (srv.permissions/unassign-roles-from-users + {:conn conn + :logger logger} + role-unassignments))] + (if (:success? result) + (assoc context :invited-expert? true) + (assoc context + :success? false + :reason :failed-to-remove-unapproved-user-role + :error-details {:result result}))) + context)) + :rollback-fn + (fn rollback-assign-approved-user-role + [{:keys [old-stakeholder invited-expert?] :as context}] + (if invited-expert? + (let [role-unassignments [{:role-name :approved-user + :context-type :application + :resource-id srv.permissions/root-app-resource-id + :user-id (:id old-stakeholder)}] + result (first (srv.permissions/unassign-roles-from-users + {:conn conn + :logger logger} + role-unassignments))] + (when-not (:success? result) + (log logger :error ::rollback-assign-approved-user-role {:reason result})) + context) + context))} + {:txn-fn + (fn assign-approved-user-role + [{:keys [old-stakeholder invited-expert?] :as context}] + (if invited-expert? + (let [role-assignments [{:role-name :approved-user + :context-type :application + :resource-id srv.permissions/root-app-resource-id + :user-id (:id old-stakeholder)}] + result (first (srv.permissions/assign-roles-to-users + {:conn conn + :logger logger} + role-assignments))] + (if (:success? result) + context + (assoc context + :success? false + :reason :failed-to-add-approved-user-role + :error-details {:result result}))) + context)) + :rollback-fn + (fn rollback-update-stakeholder + [{:keys [old-stakeholder] :as context}] + (let [affected (db.stakeholder/update-stakeholder conn old-stakeholder)] + (when-not (= 1 affected) + (log logger :error ::rollback-update-stakeholder {:id (:id old-stakeholder)}))) + context)} {:txn-fn (fn update-stakeholder - [{:keys [stakeholder old-stakeholder expert? picture-file cv-file] :as context}] + [{:keys [stakeholder old-stakeholder invited-expert? picture-file cv-file] :as context}] (let [idp-usernames (-> (concat (:idp-usernames old-stakeholder) (:idp_usernames stakeholder)) distinct @@ -342,8 +420,7 @@ cv-id (assoc :cv_id cv-id) - (and expert? - (= (:review-status old-stakeholder) "INVITED")) + invited-expert? (assoc :review_status "APPROVED")))] (if (= 1 affected) context