Skip to content

Commit

Permalink
Merge pull request #549 from CSCfi/filter-event-comments
Browse files Browse the repository at this point in the history
Filter event comments
  • Loading branch information
Macroz authored May 23, 2018
2 parents da6678b + 15fac4d commit 877b664
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 17 deletions.
18 changes: 17 additions & 1 deletion src/clj/rems/api/application.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
[rems.db.core :as db]
[rems.db.users :as users]
[rems.form :as form]
[rems.util :refer [get-user-id]]
[ring.util.http-response :refer :all]
[schema.core :as s]))

Expand Down Expand Up @@ -84,9 +85,24 @@
(update-in [:items] longify-keys)
(update-in [:licenses] longify-keys)))

(defn- hide-sensitive-comments [events]
(map (fn [event]
(if (contains? #{"third-party-review" "review-request"} (:event event))
(assoc event :comment nil) ; remove sensitive comment
event))
events))

(defn hide-event-comments [application user]
(let [events (get-in application [:application :events])
can-see-comments? (contains? (set (applications/get-handlers application)) (get-user-id))]
(if can-see-comments?
application
(update-in application [:application :events] hide-sensitive-comments))))

(defn api-get-application [application-id]
(when (not (empty? (db/get-applications {:id application-id})))
(applications/get-form-for application-id)))
(-> (applications/get-form-for application-id)
(hide-event-comments (get-user-id)))))

(def application-api
(context "/application" []
Expand Down
20 changes: 17 additions & 3 deletions src/clj/rems/db/applications.clj
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,24 @@
(and (= "applied" (:state application))
(is-third-party-reviewer? (get-user-id) (:curround application) application)))

(defn get-approvers [application]
(actors/get-by-role (:id application) "approver"))

(defn get-reviewers [application]
(actors/get-by-role (:id application) "reviewer"))

(defn get-third-party-reviewers
"Takes as an argument a structure containing application information and a workflow round. Then returns userids for all users that have been requested to review for the given round."
[application round]
(set (map :userid (get-events-of-type application round "review-request"))))
"Takes as an argument a structure containing application information and a optionally the workflow round. Then returns userids for all users that have been requested to review for the given round or all rounds if not given."
([application]
(set (map :userid (get-events-of-type application "review-request"))))
([application round]
(set (map :userid (get-events-of-type application round "review-request")))))

(defn get-handlers [application]
(let [approvers (get-approvers application)
reviewers (get-reviewers application)
third-party-reviewers (get-third-party-reviewers application)]
(union approvers reviewers third-party-reviewers)))

(defn is-applicant? [application]
(= (:applicantuserid application) (get-user-id)))
Expand Down
50 changes: 37 additions & 13 deletions test/clj/rems/test/api/application.clj
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,12 @@

(deftest application-api-third-party-review-test
(let [api-key "42"
user "developer"
reviewer "alice"
applicant "alice"
approver "developer"
reviewer "bob"
catid 2
app-id (-> (request :put (str "/api/application/save"))
(authenticate api-key user)
(authenticate api-key applicant)
(json-body {:command "submit"
:catalogue-items [catid]
:items {1 "x" 2 "y" 3 "z"}
Expand All @@ -351,7 +352,7 @@
(testing "send review request"
(is (= 200
(-> (request :put (str "/api/application/review_request"))
(authenticate api-key user)
(authenticate api-key approver)
(json-body {:application-id app-id
:round 0
:comment "pls revu"
Expand All @@ -360,13 +361,13 @@
:status))))
(testing "check review event"
(let [events (-> (request :get (str "/api/application/" app-id))
(authenticate api-key user)
(authenticate api-key reviewer)
app
read-body
:application
:events)]
(is (= [{:userid "developer" :comment nil :event "apply"}
{:userid "alice" :comment "pls revu" :event "review-request"}]
(is (= [{:userid applicant :comment nil :event "apply"}
{:userid reviewer :comment "pls revu" :event "review-request"}]
(map #(select-keys % [:userid :comment :event]) events)))))
(testing "send review"
(is (= 200
Expand All @@ -378,17 +379,40 @@
:comment "is ok"})
app
:status))))
(testing "check events"
(testing "events of approver"
(let [events (-> (request :get (str "/api/application/" app-id))
(authenticate api-key user)
(authenticate api-key approver)
app
read-body
:application
:events)]
(is (= [{:userid applicant :comment nil :event "apply"}
{:userid reviewer :comment "pls revu" :event "review-request"}
{:userid reviewer :comment "is ok" :event "third-party-review"}]
(map #(select-keys % [:userid :comment :event]) events)))))
(testing "events of reviewer"
(let [events (-> (request :get (str "/api/application/" app-id))
(authenticate api-key reviewer)
app
read-body
:application
:events)]
(is (= [{:userid applicant :comment nil :event "apply"}
{:userid reviewer :comment "pls revu" :event "review-request"}
{:userid reviewer :comment "is ok" :event "third-party-review"}]
(map #(select-keys % [:userid :comment :event]) events)))))
(testing "events of applicant"
(let [events (-> (request :get (str "/api/application/" app-id))
(authenticate api-key applicant)
app
read-body
:application
:events)]
(is (= [{:userid "developer" :comment nil :event "apply"}
{:userid "alice" :comment "pls revu" :event "review-request"}
{:userid "alice" :comment "is ok" :event "third-party-review"}]
(map #(select-keys % [:userid :comment :event]) events)))))))
(is (= [{:userid applicant :comment nil :event "apply"}
{:userid reviewer :comment nil :event "review-request"}
{:userid reviewer :comment nil :event "third-party-review"}]
(map #(select-keys % [:userid :comment :event]) events))
"does not see review event comments")))))
;; TODO non-happy path tests for review?

;; TODO test for event filtering when it gets implemented
Expand Down
1 change: 1 addition & 0 deletions test/clj/rems/test/api/applications.clj
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
app
read-body)]
(is (= [1 2 3 4 5 6 7] (map :id (sort-by :id data)))))))

(deftest applications-api-security-test
(testing "listing without authentication"
(let [response (-> (request :get (str "/api/applications"))
Expand Down

0 comments on commit 877b664

Please sign in to comment.