Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: factor out user-selection widget for actions #2426

Merged
merged 2 commits into from Nov 5, 2020
Merged

refactor: factor out user-selection widget for actions #2426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d1e1f73
refactor: factor out user-selection from rems.actions.request-decision
opqdonut Nov 4, 2020
826d5c2
refactor: use user-selection in rems.actions.request-review
opqdonut Nov 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion resources/translations/en.edn
View file
Open in desktop
Expand Up @@ -40,7 +40,6 @@
:remove-member "Remove member"
:request-decision "Request decision"
:request-review "Request review"
:request-selection "Send request to"
Macroz marked this conversation as resolved.
Show resolved Hide resolved
:request-selections "Send requests to"
:resources-selection "Resources included in the application:"
:return "Return to applicant"
Expand Down
1 change: 0 additions & 1 deletion resources/translations/fi.edn
View file
Open in desktop
Expand Up @@ -40,7 +40,6 @@
:remove-member "Poista jäsen"
:request-decision "Pyydä päätöstä"
:request-review "Pyydä katselmointia"
:request-selection "Lähetä pyyntö henkilölle"
:request-selections "Lähetä pyynnöt henkilöille"
:resources-selection "Hakemukseen liitettävät resurssit:"
:return "Palauta hakijalle"
Expand Down
1 change: 0 additions & 1 deletion resources/translations/sv.edn
View file
Open in desktop
Expand Up @@ -40,7 +40,6 @@
:remove-member "Ta bort deltagare"
:request-decision "Begär beslut"
:request-review "Begär granskning"
:request-selection "Skicka begäran till"
:request-selections "Skicka begäranden till"
:resources-selection "Välj resurser som ansökan gäller:"
:return "Sänd tillbaka till sökande"
Expand Down
23 changes: 22 additions & 1 deletion src/cljs/rems/actions/components.cljs
View file
Open in desktop
@@ -1,7 +1,9 @@
(ns rems.actions.components
(:require [re-frame.core :as rf]
[rems.atoms :refer [attachment-link close-symbol success-symbol textarea]]
[rems.atoms :refer [enrich-user textarea]]
[rems.common.attachment-types :as attachment-types]
[rems.dropdown :as dropdown]
[rems.fetcher :as fetcher]
[rems.fields :as fields]
[rems.flash-message :as flash-message]
[rems.text :refer [text]]
Expand Down Expand Up @@ -115,6 +117,25 @@
:on-attach #(rf/dispatch [::save-attachment application-id field-key %])
:on-remove-attachment #(rf/dispatch [::remove-attachment field-key %])}])

(fetcher/reg-fetcher ::reviewers "/api/applications/reviewers" {:result (partial map enrich-user)})
(fetcher/reg-fetcher ::deciders "/api/applications/deciders" {:result (partial map enrich-user)})

(rf/reg-sub ::users (fn [db [_ field-key]] (get-in db [::users field-key])))
(rf/reg-event-db ::set-users (fn [db [_ field-key value]] (assoc-in db [::users field-key] value)))

(defn user-selection [{:keys [subscription field-key]}]
(let [id (str field-key "-users")]
[:div.form-group
[:label {:for id} (text :t.actions/request-selections)]
[dropdown/dropdown
{:id id
:items @(rf/subscribe subscription)
:item-key :userid
:item-label :display
:item-selected? (set @(rf/subscribe [::users field-key]))
:multi? true
:on-change #(rf/dispatch [::set-users field-key %])}]]))

(defn action-form-view
"Renders an action form that is collapsible.

Expand Down
62 changes: 13 additions & 49 deletions src/cljs/rems/actions/request_decision.cljs
View file
Open in desktop
@@ -1,42 +1,17 @@
(ns rems.actions.request-decision
(:require [re-frame.core :as rf]
[rems.actions.components :refer [action-attachment action-button comment-field action-form-view button-wrapper command!]]
[rems.atoms :refer [enrich-user]]
[rems.dropdown :as dropdown]
[rems.flash-message :as flash-message]
[rems.text :refer [text]]
[rems.util :refer [fetch]]))
[rems.actions.components :refer [action-attachment action-button comment-field action-form-view button-wrapper command! user-selection]]
[rems.text :refer [text]]))

(def ^:private action-form-id "request-decision")
(def ^:private dropdown-id "request-decision-dropdown")

(rf/reg-fx
::fetch-potential-deciders
(fn [on-success]
(fetch "/api/applications/deciders"
{:handler on-success
:error-handler (flash-message/default-error-handler :top "Fetch potential deciders")})))

(rf/reg-event-fx
::open-form
(fn [{:keys [db]} _]
{:db (assoc db
::potential-deciders #{}
::selected-deciders #{})
:dispatch-n [[:rems.actions.components/set-comment action-form-id nil]
[:rems.actions.components/set-attachments action-form-id []]]
::fetch-potential-deciders #(rf/dispatch [::set-potential-deciders %])}))

(rf/reg-sub ::potential-deciders (fn [db _] (::potential-deciders db)))
(rf/reg-event-db
::set-potential-deciders
(fn [db [_ deciders]]
(assoc db
::potential-deciders (set (map enrich-user deciders))
::selected-deciders #{})))

(rf/reg-sub ::selected-deciders (fn [db _] (::selected-deciders db)))
(rf/reg-event-db ::set-selected-deciders (fn [db [_ deciders]] (assoc db ::selected-deciders deciders)))
(fn [_ _]
{:dispatch-n [[:rems.actions.components/deciders]
[:rems.actions.components/set-users action-form-id nil]
[:rems.actions.components/set-comment action-form-id nil]
[:rems.actions.components/set-attachments action-form-id []]]}))

(rf/reg-event-fx
::send-request-decision
Expand All @@ -57,39 +32,28 @@
:on-click #(rf/dispatch [::open-form])}])

(defn request-decision-view
[{:keys [application-id selected-deciders potential-deciders on-set-deciders on-send]}]
[{:keys [application-id disabled on-send]}]
[action-form-view action-form-id
(text :t.actions/request-decision)
[[button-wrapper {:id "request-decision"
:text (text :t.actions/request-decision)
:class "btn-primary"
:on-click on-send
:disabled (empty? selected-deciders)}]]
:disabled disabled}]]
[:div
[comment-field {:field-key action-form-id
:label (text :t.form/add-comments-not-shown-to-applicant)}]
[action-attachment {:field-key action-form-id
:application-id application-id}]
[:div.form-group
[:label {:for dropdown-id} (text :t.actions/request-selection)]
[dropdown/dropdown
{:id dropdown-id
:items potential-deciders
:item-key :userid
:item-label :display
:item-selected? #(contains? (set selected-deciders) %)
:multi? true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here, decider selection has had multi? true. The event & command also support multiple users.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it could potentially be wrong. We don't have it as configurable and I guess that's ok for now, but the assumption we had in an earlier discussion couple weeks ago (when doing the wording change) was that there would only ever be one decider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which wording change? Looking at the history, the label has been :t.actions/request-selection and multi? has been true at least since 2019-08-01, commit a6c15aa.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2397

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! What do you propose? I find that the UI is clear enough even with multiselection, and it doesn't really do any harm to support multiple deciders.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I would just like to confirm from @jaakkocsc that it's so far ok to have multiple deciders. The changes look fine otherwise.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say the decision on having only one decider or multiple deciders is organisational, by the "organisation-owner", not a technical limitation set by REMS.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It simplifies REMS to have all the invites/requests to be available for multiple people.

:on-change on-set-deciders}]]]])
[user-selection {:field-key action-form-id
:subscription [:rems.actions.components/deciders]}]]])

(defn request-decision-form [application-id on-finished]
(let [selected-deciders @(rf/subscribe [::selected-deciders])
potential-deciders @(rf/subscribe [::potential-deciders])
(let [selected-deciders @(rf/subscribe [:rems.actions.components/users action-form-id])
comment @(rf/subscribe [:rems.actions.components/comment action-form-id])
attachments @(rf/subscribe [:rems.actions.components/attachments action-form-id])]
[request-decision-view {:application-id application-id
:selected-deciders selected-deciders
:potential-deciders potential-deciders
:on-set-deciders #(rf/dispatch [::set-selected-deciders %])
:disabled (empty? selected-deciders)
:on-send #(rf/dispatch [::send-request-decision {:application-id application-id
:deciders selected-deciders
:comment comment
Expand Down
62 changes: 13 additions & 49 deletions src/cljs/rems/actions/request_review.cljs
View file
Open in desktop
@@ -1,43 +1,18 @@
(ns rems.actions.request-review
(:require [re-frame.core :as rf]
[rems.actions.components :refer [action-attachment action-button comment-field action-form-view button-wrapper command!]]
[rems.atoms :refer [enrich-user]]
[rems.dropdown :as dropdown]
[rems.flash-message :as flash-message]
[rems.text :refer [text]]
[rems.util :refer [fetch]]))
[rems.actions.components :refer [action-attachment action-button comment-field action-form-view button-wrapper command! user-selection]]
[rems.text :refer [text]]))

(def ^:private action-form-id "request-review")
(def ^:private dropdown-id "request-review-dropdown")

(rf/reg-fx
::fetch-potential-reviewers
(fn [on-success]
(fetch "/api/applications/reviewers"
{:handler on-success
:error-handler (flash-message/default-error-handler :top "Fetch potential reviewers")})))

(rf/reg-event-fx
::open-form
(fn
[{:keys [db]} _]
{:db (assoc db
::potential-reviewers #{}
::selected-reviewers #{})
:dispatch-n [[:rems.actions.components/set-comment action-form-id nil]
[:rems.actions.components/set-attachments action-form-id []]]
::fetch-potential-reviewers #(rf/dispatch [::set-potential-reviewers %])}))

(rf/reg-sub ::potential-reviewers (fn [db _] (::potential-reviewers db)))
(rf/reg-event-db
::set-potential-reviewers
(fn [db [_ reviewers]]
(assoc db
::potential-reviewers (set (map enrich-user reviewers))
::selected-reviewers #{})))

(rf/reg-sub ::selected-reviewers (fn [db _] (::selected-reviewers db)))
(rf/reg-event-db ::set-selected-reviewers (fn [db [_ reviewers]] (assoc db ::selected-reviewers reviewers)))
[_ _]
{:dispatch-n [[:rems.actions.components/reviewers]
[:rems.actions.components/set-users action-form-id nil]
[:rems.actions.components/set-comment action-form-id nil]
[:rems.actions.components/set-attachments action-form-id []]]}))

(rf/reg-event-fx
::send-request-review
Expand All @@ -58,39 +33,28 @@
:on-click #(rf/dispatch [::open-form])}])

(defn request-review-view
[{:keys [application-id selected-reviewers potential-reviewers on-set-reviewers on-send]}]
[{:keys [application-id disabled on-send]}]
[action-form-view action-form-id
(text :t.actions/request-review)
[[button-wrapper {:id "request-review-button"
:text (text :t.actions/request-review)
:class "btn-primary"
:on-click on-send
:disabled (empty? selected-reviewers)}]]
:disabled disabled}]]
[:div
[comment-field {:field-key action-form-id
:label (text :t.form/add-comments-not-shown-to-applicant)}]
[action-attachment {:field-key action-form-id
:application-id application-id}]
[:div.form-group
[:label {:for dropdown-id} (text :t.actions/request-selections)]
[dropdown/dropdown
{:id dropdown-id
:items potential-reviewers
:item-key :userid
:item-label :display
:item-selected? #(contains? (set selected-reviewers) %)
:multi? true
:on-change on-set-reviewers}]]]])
[user-selection {:field-key action-form-id
:subscription [:rems.actions.components/reviewers]}]]])

(defn request-review-form [application-id on-finished]
(let [selected-reviewers @(rf/subscribe [::selected-reviewers])
potential-reviewers @(rf/subscribe [::potential-reviewers])
(let [selected-reviewers @(rf/subscribe [:rems.actions.components/users action-form-id])
comment @(rf/subscribe [:rems.actions.components/comment action-form-id])
attachments @(rf/subscribe [:rems.actions.components/attachments action-form-id])]
[request-review-view {:application-id application-id
:selected-reviewers selected-reviewers
:potential-reviewers potential-reviewers
:on-set-reviewers #(rf/dispatch [::set-selected-reviewers %])
:disabled (empty? selected-reviewers)
:on-send #(rf/dispatch [::send-request-review {:application-id application-id
:reviewers selected-reviewers
:comment comment
Expand Down