Repository: CSCfi/rems
Pull request #1395 (2019-07-10 10:28:02)

Change focus to modal content when opened
I'm making here a new pull request out of this same patch as previously. Meanwhile, I looked into using rems.focus for this, but as I understand, rems.focus shouldn't be needed because there is no need to asynchronously create the element and set focus to it.

However, I had a problem using screen reader with this patch, described below:

Switching the focus to content seemed to work correctly, but the screen reader I'm using (ChromeVox) did not always start reading the content. Not sure if it is a problem with the screen reader or the code. It probably needs to be tested with another screen reader eventually. But at least switching the focus is a step to the right direction.


Patch:
diff --git a/project.clj b/project.clj
index 6b9d1c133..a2a3460f9 100644
--- a/project.clj
+++ b/project.clj
@@ -69,7 +69,6 @@
   :min-lein-version "2.0.0"
 
   :source-paths ["src/clj" "src/cljc"]
-  :java-source-paths ["src/java"]
   :test-paths ["src/clj" "src/cljc" "test/clj" "test/cljc"] ;; also run tests from src files
   :resource-paths ["resources" "target/cljsbuild"]
   :target-path "target/%s/"
@@ -115,7 +114,6 @@
              :prep-tasks [["shell" "sh" "-c" "mkdir -p target/uberjar/resources"]
                           ["shell" "sh" "-c" "git describe --tags --long --always --dirty=-custom > target/uberjar/resources/git-describe.txt"]
                           ["shell" "sh" "-c" "git rev-parse HEAD > target/uberjar/resources/git-revision.txt"]
-                          "javac"
                           "compile"
                           ["cljsbuild" "once" "min"]]
              :cljsbuild {:builds {:min {:source-paths ["src/cljc" "src/cljs"]
@@ -153,6 +151,7 @@
                  :plugins [[lein-ancient "0.6.15"]
                            [lein-doo "0.1.11"]
                            [lein-figwheel "0.5.18"]]
+                 :aot [rems.InvalidRequestException rems.auth.NotAuthorizedException rems.auth.ForbiddenException]
 
                  :jvm-opts ["-Drems.config=dev-config.edn"
                             "-Djdk.attach.allowAttachSelf"] ; needed by clj-memory-meter on Java 9+
diff --git a/src/clj/rems/InvalidRequestException.clj b/src/clj/rems/InvalidRequestException.clj
new file mode 100644
index 000000000..7ef0f68cc
--- /dev/null
+++ b/src/clj/rems/InvalidRequestException.clj
@@ -0,0 +1,3 @@
+(ns ^{:doc "REMS custom exception thrown when a request is invalid"}
+    rems.InvalidRequestException
+  (:gen-class :extends java.lang.Exception))
diff --git a/src/clj/rems/api.clj b/src/clj/rems/api.clj
index 115d9b031..c259df103 100644
--- a/src/clj/rems/api.clj
+++ b/src/clj/rems/api.clj
@@ -15,6 +15,8 @@
             [rems.api.resources :refer [resources-api]]
             [rems.api.users :refer [users-api]]
             [rems.api.workflows :refer [workflows-api]]
+            [rems.auth.ForbiddenException]
+            [rems.auth.NotAuthorizedException]
             [rems.json :refer [muuntaja]]
             [ring.middleware.cors :refer [wrap-cors]]
             [ring.util.http-response :refer :all]
diff --git a/src/clj/rems/auth/ForbiddenException.clj b/src/clj/rems/auth/ForbiddenException.clj
new file mode 100644
index 000000000..bd14fa44c
--- /dev/null
+++ b/src/clj/rems/auth/ForbiddenException.clj
@@ -0,0 +1,3 @@
+(ns ^{:doc "REMS custom exception thrown when user is not authenticated."}
+    rems.auth.ForbiddenException
+  (:gen-class :extends java.lang.Exception))
diff --git a/src/clj/rems/auth/NotAuthorizedException.clj b/src/clj/rems/auth/NotAuthorizedException.clj
new file mode 100644
index 000000000..6d4c91c63
--- /dev/null
+++ b/src/clj/rems/auth/NotAuthorizedException.clj
@@ -0,0 +1,3 @@
+(ns ^{:doc "REMS custom exception thrown when an operation is not authorized."}
+    rems.auth.NotAuthorizedException
+  (:gen-class :extends java.lang.Exception))
diff --git a/src/clj/rems/auth/util.clj b/src/clj/rems/auth/util.clj
index b4842db12..6337e39f7 100644
--- a/src/clj/rems/auth/util.clj
+++ b/src/clj/rems/auth/util.clj
@@ -1,12 +1,13 @@
 (ns rems.auth.util
-  (:import [rems.auth NotAuthorizedException ForbiddenException]))
+  (:require [rems.auth.ForbiddenException]
+            [rems.auth.NotAuthorizedException]))
 
 (defn throw-unauthorized
   "Helper for throwing `NotAuthorizedException`."
   []
-  (throw (NotAuthorizedException.)))
+  (throw (rems.auth.NotAuthorizedException.)))
 
 (defn throw-forbidden
   "Helper for throwing `ForbiddenException`."
   []
-  (throw (ForbiddenException.)))
+  (throw (rems.auth.ForbiddenException.)))
diff --git a/src/clj/rems/db/applications/legacy.clj b/src/clj/rems/db/applications/legacy.clj
index 0688874ad..3b9c78dd8 100644
--- a/src/clj/rems/db/applications/legacy.clj
+++ b/src/clj/rems/db/applications/legacy.clj
@@ -1,23 +1,23 @@
 (ns rems.db.applications.legacy
   "Functions for dealing with old round-based applications."
-  (:require [clj-time.core :as time]
-            [clojure.test :refer :all]
+  (:require [clojure.test :refer :all]
+            [clj-time.core :as time]
             [cprop.tools :refer [merge-maps]]
-            [rems.application-util :refer [form-fields-editable?]]
             [rems.application.commands :as commands]
             [rems.application.model :as model]
+            [rems.application-util :refer [form-fields-editable?]]
             [rems.auth.util :refer [throw-forbidden]]
             [rems.db.applications :as applications]
-            [rems.db.catalogue :as catalogue]
             [rems.db.core :as db]
+            [rems.db.catalogue :as catalogue]
             [rems.db.entitlements :as entitlements]
             [rems.db.events :as events]
+            [rems.db.form :as form]
             [rems.db.licenses :as licenses]
             [rems.db.users :as users]
             [rems.db.workflow-actors :as actors]
             [rems.json :as json]
-            [rems.permissions :as permissions])
-  (:import [rems InvalidRequestException]))
+            [rems.permissions :as permissions]))
 
 (declare get-application-state)
 
@@ -202,7 +202,7 @@
 
 (defn- get-dynamic-application-state [application-id]
   (let [application (or (first (db/get-applications {:id application-id}))
-                        (throw (InvalidRequestException.
+                        (throw (rems.InvalidRequestException.
                                 (str "Application " application-id " not found"))))
         events (events/get-application-events application-id)
         application (assoc application
diff --git a/src/clj/rems/db/attachments.clj b/src/clj/rems/db/attachments.clj
index 74e1da03c..f55e4aa66 100644
--- a/src/clj/rems/db/attachments.clj
+++ b/src/clj/rems/db/attachments.clj
@@ -1,9 +1,10 @@
 (ns rems.db.attachments
   (:require [rems.application-util :refer [form-fields-editable?]]
+            [rems.InvalidRequestException]
             [rems.auth.util :refer [throw-forbidden]]
             [rems.db.core :as db])
-  (:import [java.io ByteArrayOutputStream FileInputStream File]
-           [rems InvalidRequestException]))
+  (:import [java.io ByteArrayOutputStream FileInputStream File ByteArrayInputStream]
+           rems.InvalidRequestException))
 
 (defn check-attachment-content-type
   "Checks that content-type matches the allowed ones listed on the UI side:
diff --git a/src/clj/rems/db/form.clj b/src/clj/rems/db/form.clj
index 78397584f..b10beca29 100644
--- a/src/clj/rems/db/form.clj
+++ b/src/clj/rems/db/form.clj
@@ -1,5 +1,6 @@
 (ns rems.db.form
   (:require [clojure.test :refer :all]
+            [rems.InvalidRequestException]
             [rems.db.catalogue :as catalogue]
             [rems.db.core :as db]
             [rems.json :as json]))
@@ -55,8 +56,8 @@
        :errors [{:type :t.administration.errors/form-in-use :catalogue-items catalogue-items}]})))
 
 (defn form-editable [form-id]
-  (or (form-in-use-error form-id)
-      {:success true}))
+    (or (form-in-use-error form-id)
+        {:success true}))
 
 (defn- generate-fields-with-ids! [user-id form-id fields]
   ;; Mirror field ids to form template so that form templates
diff --git a/src/clj/rems/middleware.clj b/src/clj/rems/middleware.clj
index 5f477f36d..fe65dfa15 100644
--- a/src/clj/rems/middleware.clj
+++ b/src/clj/rems/middleware.clj
@@ -24,9 +24,9 @@
             [ring.middleware.format :refer [wrap-restful-format]]
             [ring.middleware.webjars :refer [wrap-webjars]]
             [ring.util.http-response :refer [unauthorized]]
-            [ring.util.response :refer [redirect header]])
-  (:import [javax.servlet ServletContext]
-           [rems.auth ForbiddenException NotAuthorizedException]))
+            [ring.util.response :refer [redirect header]]
+            [taoensso.tempura :as tempura])
+  (:import (javax.servlet ServletContext)))
 
 (defn calculate-root-path [request]
   (if-let [context (:servlet-context request)]
@@ -159,9 +159,9 @@
   (fn [req]
     (try
       (handler req)
-      (catch NotAuthorizedException e
+      (catch rems.auth.NotAuthorizedException e
         (on-unauthorized-error req))
-      (catch ForbiddenException e
+      (catch rems.auth.ForbiddenException e
         (on-forbidden-error req)))))
 
 (defn wrap-logging
diff --git a/src/cljs/rems/modal.cljs b/src/cljs/rems/modal.cljs
index 24c287943..808a48598 100644
--- a/src/cljs/rems/modal.cljs
+++ b/src/cljs/rems/modal.cljs
@@ -48,7 +48,12 @@
                   :title-class title-class
                   :always [:div.full
                            ;; max-height in order to keep header and controls visible
-                           [:div.modal--content {:style {:max-height "70vh" :overflow :auto}} content]
+                           [:div.modal--content
+                            {:style {:max-height "70vh" :overflow :auto}
+                             :ref (fn [elem] (when elem
+                                               (.focus elem)))
+                             :tabIndex "-1"}
+                            content]
                            (into [:div.modal--commands.commands {:style {:padding 0}}]
                                  commands)]
                   :open? true}]]
diff --git a/src/java/rems/InvalidRequestException.java b/src/java/rems/InvalidRequestException.java
deleted file mode 100644
index e3b65bc41..000000000
--- a/src/java/rems/InvalidRequestException.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package rems;
-
-/**
- * Thrown when a request is invalid.
- */
-public class InvalidRequestException extends Exception {
-
-    public InvalidRequestException() {
-    }
-
-    public InvalidRequestException(String message) {
-        super(message);
-    }
-
-    public InvalidRequestException(String message, Throwable cause) {
-        super(message, cause);
-    }
-
-    public InvalidRequestException(Throwable cause) {
-        super(cause);
-    }
-}
diff --git a/src/java/rems/auth/ForbiddenException.java b/src/java/rems/auth/ForbiddenException.java
deleted file mode 100644
index 3917e5844..000000000
--- a/src/java/rems/auth/ForbiddenException.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package rems.auth;
-
-/**
- * Thrown when user is not authenticated.
- */
-public class ForbiddenException extends Exception {
-
-    public ForbiddenException() {
-    }
-
-    public ForbiddenException(String message) {
-        super(message);
-    }
-
-    public ForbiddenException(String message, Throwable cause) {
-        super(message, cause);
-    }
-
-    public ForbiddenException(Throwable cause) {
-        super(cause);
-    }
-}
diff --git a/src/java/rems/auth/NotAuthorizedException.java b/src/java/rems/auth/NotAuthorizedException.java
deleted file mode 100644
index 62598bb18..000000000
--- a/src/java/rems/auth/NotAuthorizedException.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package rems.auth;
-
-/**
- * Thrown when an operation is not authorized.
- */
-public class NotAuthorizedException extends Exception {
-
-    public NotAuthorizedException() {
-    }
-
-    public NotAuthorizedException(String message) {
-        super(message);
-    }
-
-    public NotAuthorizedException(String message, Throwable cause) {
-        super(message, cause);
-    }
-
-    public NotAuthorizedException(Throwable cause) {
-        super(cause);
-    }
-}


Test patch:
diff --git a/test/clj/rems/api/test_util.clj b/test/clj/rems/api/test_util.clj
index fda22c366..87548f680 100644
--- a/test/clj/rems/api/test_util.clj
+++ b/test/clj/rems/api/test_util.clj
@@ -2,6 +2,8 @@
   (:require [clojure.test :refer :all]
             [compojure.api.sweet :refer :all]
             [rems.api.util :refer :all]
+            [rems.auth.ForbiddenException]
+            [rems.auth.NotAuthorizedException]
             [rems.context :as context]
             [ring.util.http-response :refer :all])
   (:import (rems.auth ForbiddenException NotAuthorizedException)))
diff --git a/test/clj/rems/test_db.clj b/test/clj/rems/test_db.clj
index fb1eb1a38..eedb99a2b 100644
--- a/test/clj/rems/test_db.clj
+++ b/test/clj/rems/test_db.clj
@@ -4,6 +4,7 @@
             [clj-time.core :as time]
             [clojure.string :refer [split-lines]]
             [clojure.test :refer :all]
+            [rems.auth.ForbiddenException]
             [rems.config]
             [rems.context :as context]
             [rems.db.applications :as applications]