Repository: CSCfi/rems
Pull request #2713 (2021-09-01 14:27:07)

feat: allow only future dates in entitlement expiration date field
implements #2674

* add `min` value to `approve-reject-view` entitlement expiration date field as **today + 1 day**
  * this adds only date picker support, it is still possible to manually type e.g. `01.01.1989` into the field
* add check to `:application.command/approve` command handler that, if provided, `:entitlement-end` is in future (calculated from `today at 23:59:59`)

## Reviewability
- [x] Link to issue
- [x] Consider adding screenshots for ease of review

## Documentation
- [x] Update changelog if necessary

## Backwards compatibility
- [x] API is **backwards compatible** or completely new

## Testing
- [x] Valuable features are integration / browser / acceptance tested automatically

---
![Screenshot 2021-09-01 at 17 24 19](https://user-images.githubusercontent.com/794087/131688789-57f7f4cf-3597-482d-ab22-b5fdf5ae28f2.png)


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index dc077b621..1c436db2a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ Changes since v2.20
 - "More info" support for DOI style resources (when `:enable-doi` flag is set) (#2701)
 - There is a new v2 push API to sync entitlements outside of REMS. So far the only supported type is `:ega`. See [configuration.md](docs/configuration.md) for more details. (#2466)
 - Handlers can be invited to a workflow by email using the API (#2650)
+- Entitlement expiration field in application actions will now accept only future dates (#2674)
 
 ## v2.20 "Vattuniemenkatu" 2021-08-24
 
diff --git a/resources/translations/en.edn b/resources/translations/en.edn
index 58bd958de..bc6bd11fa 100644
--- a/resources/translations/en.edn
+++ b/resources/translations/en.edn
@@ -35,7 +35,8 @@
                      :licenses-not-accepted "Terms of use not accepted."
                      ;; %1 - organization name
                      :organization-does-not-exist "Organization \"%1\" does not exist"
-                     :validation-errors "Check the following errors:"}
+                     :validation-errors "Check the following errors:"
+                     :entitlement-end-not-in-future "The expiration date for access rights must be in the future."}
             :export-entitlements "Export entitlements as CSV"
             :generate-ega-api-key-in-profile "Generate one at the Profile page."
             :handled-applications "Processed applications"
diff --git a/resources/translations/fi.edn b/resources/translations/fi.edn
index 559b86036..aa1ce1f4c 100644
--- a/resources/translations/fi.edn
+++ b/resources/translations/fi.edn
@@ -33,7 +33,8 @@
                      :invalid-token [:div [:p "Hakemukseen liittyminen epäonnistui."] [:p "Tarkista kutsukoodi " [:code "%1"]]]
                      :licenses-not-accepted "Käyttöehtoja ei ole hyväksytty."
                      :organization-does-not-exist "Organisaatiota \"%1\" ei ole olemassa"
-                     :validation-errors "Tarkasta seuraavat virheet:"}
+                     :validation-errors "Tarkasta seuraavat virheet:"
+                     :entitlement-end-not-in-future "Pääsyoikeuksien vanhenemispäivän täytyy olla tulevaisuudessa."}
             :export-entitlements "Lataa käyttöoikeudet CSV-tiedostona"
             :generate-ega-api-key-in-profile "Luo avain Profiili-sivulla."
             :handled-applications "Käsitellyt hakemukset"
diff --git a/resources/translations/sv.edn b/resources/translations/sv.edn
index 577f8bcb9..bf0111c73 100644
--- a/resources/translations/sv.edn
+++ b/resources/translations/sv.edn
@@ -33,7 +33,8 @@
                      :invalid-token [:div [:p "Det gick inte att ansluta till ansökningen."] [:p "Vänligen kontrollera koden för inbjudan." [:code "%1"]]]
                      :licenses-not-accepted "Du har inte godkänt användarvillkoren."
                      :organization-does-not-exist "Organisationen \"%1\" hittades inte."
-                     :validation-errors "Var vänlig kontrollera följande:"}
+                     :validation-errors "Var vänlig kontrollera följande:"
+                     :entitlement-end-not-in-future "Utgångsdatumet för rättigheter måste vara i framtiden."}
             :export-entitlements "Ladda ner rättigheterna som csv-fil"
             :generate-ega-api-key-in-profile "Hämta den på Profil-sidan."
             :handled-applications "Behandlade ansökningar"
diff --git a/src/clj/rems/application/commands.clj b/src/clj/rems/application/commands.clj
index a38ecd98e..d2d78f908 100644
--- a/src/clj/rems/application/commands.clj
+++ b/src/clj/rems/application/commands.clj
@@ -10,7 +10,8 @@
             [rems.schema-base :as schema-base]
             [rems.util :refer [assert-ex getx getx-in try-catch-ex update-present]]
             [schema-refined.core :as r]
-            [schema.core :as s])
+            [schema.core :as s]
+            [clj-time.core :as time])
   (:import [java.util UUID]
            [org.joda.time DateTime]))
 
@@ -320,6 +321,13 @@
        (mapv (fn [license]
                {:license/id (:id license)}))))
 
+(defn- entitlement-end-not-in-future-error
+  "Checks that entitlement date time, if provided, is in the future."
+  [dt]
+  (when dt
+    (when-not (time/after? dt (time/today-at 23 59 59))
+      {:errors [{:type :t.actions.errors/entitlement-end-not-in-future}]})))
+
 (defn- application-created-event! [{:keys [catalogue-item-ids time actor] :as cmd}
                                    {:keys [allocate-application-ids! get-catalogue-item get-workflow]
                                     :as injections}]
@@ -387,10 +395,11 @@
 
 (defmethod command-handler :application.command/approve
   [cmd application injections]
-  (add-comment-and-attachments cmd injections
-                               (merge {:event/type :application.event/approved}
-                                      (when-let [end (:entitlement-end cmd)]
-                                        {:entitlement/end end}))))
+  (or (entitlement-end-not-in-future-error (:entitlement-end cmd))
+      (add-comment-and-attachments cmd injections
+                                   (merge {:event/type :application.event/approved}
+                                          (when-let [end (:entitlement-end cmd)]
+                                            {:entitlement/end end})))))
 
 (defmethod command-handler :application.command/reject
   [cmd application injections]
diff --git a/src/cljs/rems/actions/approve_reject.cljs b/src/cljs/rems/actions/approve_reject.cljs
index dd83614c0..47d2ffe67 100644
--- a/src/cljs/rems/actions/approve_reject.cljs
+++ b/src/cljs/rems/actions/approve_reject.cljs
@@ -5,7 +5,7 @@
             [re-frame.core :as rf]
             [rems.actions.components :refer [action-attachment action-button comment-field action-form-view button-wrapper command!]]
             [rems.atoms :refer [close-symbol]]
-            [rems.text :refer [text]]))
+            [rems.text :refer [text localize-utc-date]]))
 
 (def ^:private action-form-id "approve-reject")
 
@@ -84,6 +84,9 @@
                             :id "approve-end"
                             :name "approve-end"
                             :value end
+                            :min (-> (time/now)
+                                     (time/plus (time/days 1))
+                                     localize-utc-date)
                             :required false
                             :on-change #(on-set-entitlement-end (.. % -target -value))}]
       (when end
@@ -108,4 +111,4 @@
                           :on-reject #(rf/dispatch [::send-reject {:application-id application-id
                                                                    :comment comment
                                                                    :attachments attachments
-                                                                   :on-finished on-finished}])}]))
+                                                                   :on-finished on-finished}])}]))
\ No newline at end of file


Test patch:
diff --git a/test/clj/rems/application/test_commands.clj b/test/clj/rems/application/test_commands.clj
index 26b9dfb41..bab4aa5f7 100644
--- a/test/clj/rems/application/test_commands.clj
+++ b/test/clj/rems/application/test_commands.clj
@@ -5,7 +5,9 @@
             [rems.application.model :as model]
             [rems.form-validation :as form-validation]
             [rems.permissions :as permissions]
-            [rems.util :refer [assert-ex getx]])
+            [rems.util :refer [assert-ex getx]]
+            [rems.testing-util :refer [with-fixed-time]]
+            [clj-time.core :as time])
   (:import [clojure.lang ExceptionInfo]
            [java.util UUID]
            [org.joda.time DateTime]))
@@ -904,14 +906,16 @@
       (is (= {:event/type :application.event/approved
               :event/time test-time
               :event/actor handler-user-id
-              :entitlement/end (DateTime. 1234)
+              :entitlement/end (time/plus (DateTime. 1234) (time/days 1))
               :application/id app-id
               :application/comment "fine"}
-             (ok-command application
-                         {:type :application.command/approve
-                          :actor handler-user-id
-                          :entitlement-end (DateTime. 1234)
-                          :comment "fine"}))))
+             (with-fixed-time (DateTime. 1234)
+               (fn []
+                 (ok-command application
+                             {:type :application.command/approve
+                              :actor handler-user-id
+                              :entitlement-end (time/plus (DateTime. 1234) (time/days 1))
+                              :comment "fine"}))))))
     (testing "rejected successfully"
       (is (= {:event/type :application.event/rejected
               :application/comment "bad"
@@ -921,7 +925,13 @@
              (ok-command application
                          {:type :application.command/reject
                           :actor handler-user-id
-                          :comment "bad"}))))))
+                          :comment "bad"}))))
+    (testing "throws with past entitlement end-date"
+      (is (= {:errors [{:type :t.actions.errors/entitlement-end-not-in-future}]}
+             (fail-command application
+                           {:type :application.command/approve
+                            :actor handler-user-id
+                            :entitlement-end (DateTime. 1234)}))))))
 
 (deftest test-close
   (let [application (apply-events nil