Merge pull request #3327 from CSCfi/small-fixes

Small fixes

CHANGELOG.md

@@ -19,6 +19,11 @@ Changes since v2.37
   REMS will not try sending it again, but gives up immediately. This should
   avoid having many hopeless retries, if there is a typo in an email address.
 - Form editor performance has been significantly improved. To give rough numbers, the editor now works smoothly with 200 form fields in test data. (#3105)
+- The wording about experimental is removed from GA4GH API. It is already used in production. (#3299)
+- Licenses are ordered consistently (alphabetically) wherever they appear. (#3302)
+
+### Fixes
+- Empty the license attachment field if the upload fails. (#3292)
 
 ## v2.37 "Laivapojankatu" 2024-05-16
 

resources/translations/da.edn

@@ -26,6 +26,7 @@
             :decide "Beslut"
             :delete "Slet kladde"
             :delete-intro [:div.intro [:p "Slet denne kladde permanent?"]]
+            :download-pdf "Download PDF"
             :errors {:handling-user-cannot-join nil
                      :duplicate-id "Id er allerede i brug."
                      ;; %1 - invitation token

resources/translations/en.edn

@@ -26,6 +26,7 @@
             :decide "Decide"
             :delete "Delete draft"
             :delete-intro [:div.intro [:p "Permanently delete this draft?"]]
+            :download-pdf "Download PDF"
             :errors {:handling-user-cannot-join "A handling user cannot join application as member."
                      :duplicate-id "Id is already in use."
                      ;; %1 - invitation token

resources/translations/fi.edn

@@ -26,6 +26,7 @@
             :decide "Päätä"
             :delete "Poista luonnos"
             :delete-intro [:div.intro [:p "Haluatko pysyvästi poistaa tämän hakemusluonnoksen?"]]
+            :download-pdf "Lataa PDF"
             :errors {:handling-user-cannot-join "Hakemuksen käsittelyyn osallistuva ei voi liittyä hakemuksen jäseneksi."
                      :duplicate-id "Tunnus on jo käytössä."
                      :invalid-token [:div [:p "Hakemukseen liittyminen epäonnistui."] [:p "Tarkista kutsukoodi " [:code "%1"]]]

resources/translations/sv.edn

@@ -26,6 +26,7 @@
             :decide "Avgör"
             :delete "Radera utkast"
             :delete-intro [:div.intro [:p "Radera detta utkast permanent?"]]
+            :download-pdf "Ladda ner PDF"
             :errors {:handling-user-cannot-join "Den som redan deltar i ansökans behandling kan inte ansluta till ansökningen."
                      :duplicate-id "Id:et är redan upptaget."
                      :invalid-token [:div [:p "Det gick inte att ansluta till ansökningen."] [:p "Vänligen kontrollera koden för inbjudan." [:code "%1"]]]

src/clj/rems/api/permissions.clj

@@ -23,13 +23,13 @@
      :tags ["permissions"]
      :responses {501 {:description "Permissions API is not enabled. #{:enable-permissions-api}"}}
      (GET "/jwk" []
-       :summary "Experimental. Get JSON Web Key Set (JWKS) (RFC 7517) containing the keys used for signing GA4GH Visas."
+       :summary "Get JSON Web Key Set (JWKS) (RFC 7517) containing the keys used for signing GA4GH Visas."
        :return GetJWKSResponse
        (or (permissions-api-not-enabled-error)
            (ok (rems.service.permissions/get-jwks))))
      (GET "/permissions/:user" []
        ;; We're trying to replicate https://github.com/CSCfi/elixir-rems-proxy/#get-permissionsusername here
-       :summary (str "Experimental. Returns user's permissions in ga4gh visa format. "
+       :summary (str "Returns user's permissions in ga4gh visa format. "
                      "Handlers, owners and reporters can query anybody's permissions. Other users can query their own permissions. "
                      "See also https://github.com/CSCfi/rems/blob/master/docs/ga4gh-visas.md")
        :roles #{:logged-in}

src/cljc/rems/common/atoms.cljc

@@ -0,0 +1,5 @@
+(ns rems.common.atoms)
+
+(def nbsp "\u00A0") ; non breaking space
+(def nbhy "\u2011") ; non breaking hyphen
+(def hamburger "\u2630") ; the hamburger menu icon

src/cljs/rems/actions/add_licenses.cljs

@@ -86,7 +86,8 @@
      [dropdown/dropdown
       {:id dropdown-id
        :items (->> potential-licenses
-                   (mapv #(assoc % ::label (get-localized-title %))))
+                   (mapv #(assoc % ::label (get-localized-title %)))
+                   (sort-by ::label))
        :item-key :id
        :item-label ::label
        :item-selected? #(contains? (set selected-licenses) %)

src/cljs/rems/administration/create_catalogue_item.cljs

@@ -199,6 +199,7 @@
 (defn- localize-licenses [resource]
   (when-some [licenses (->> (:licenses resource)
                             (map #(get-localized-title %))
+                            sort
                             seq)]
     (text-format :t.label/default
                  (text :t.administration/licenses)

src/cljs/rems/administration/create_license.cljs

@@ -3,8 +3,9 @@
             [re-frame.core :as rf]
             [rems.administration.administration :as administration]
             [rems.administration.components :refer [localized-text-field localized-textarea-autosize organization-field radio-button-group]]
-            [rems.atoms :as atoms :refer [failure-symbol file-download document-title]]
+            [rems.atoms :as atoms :refer [document-title failure-symbol file-download]]
             [rems.collapsible :as collapsible]
+            [rems.common.atoms :refer [nbsp]]
             [rems.common.attachment-util :as attachment-util]
             [rems.config]
             [rems.globals]
@@ -77,7 +78,9 @@
          {:body form-data
           :handler (fn [response]
                      (rf/dispatch [::attachment-saved language (:id response)]))
-          :error-handler (flash-message/default-error-handler :top "Save attachment")}))
+          :error-handler (fn [response]
+                           (rf/dispatch [::set-form-field [:localizations language :attachment-filename] nil])
+                           ((flash-message/default-error-handler :top "Save attachment") response))}))
 
 (rf/reg-event-db
  ::attachment-saved
@@ -185,7 +188,7 @@
                [:a.attachment-link.btn.btn-secondary.mr-2
                 {:href (str "/api/licenses/attachments/" attachment-id)
                  :target :_blank}
-                filename " " [file-download]]
+                [file-download] nbsp filename]
                [:button.btn.btn-secondary.mr-2 {:type :button
                                                 :on-click (remove-attachment-event language attachment-id)}
                 (text :t.form/attachment-remove)]])

src/cljs/rems/administration/create_resource.cljs

@@ -120,11 +120,13 @@
      [:label.administration-field-label {:for licenses-dropdown-id} (text :t.create-resource/licenses-selection)]
      [dropdown/dropdown
       {:id licenses-dropdown-id
-       :items (vec (for [license licenses
+       :items (->> (for [license licenses
                          :let [title (get-localized-title license)
                                org-short (localize-org-short license)]]
                      (assoc license
-                            ::label (text-format :t.label/default title org-short))))
+                            ::label (text-format :t.label/default title org-short)))
+                   (sort-by ::label)
+                   vec)
        :item-key :id
        :item-label ::label
        :item-selected? #(contains? (set selected-licenses) %)

src/cljs/rems/administration/create_workflow.cljs

@@ -18,7 +18,7 @@
             [rems.flash-message :as flash-message]
             [rems.focus :as focus]
             [rems.spinner :as spinner]
-            [rems.text :refer [localized localize-command localize-role localize-state text text-format text-format-map]]
+            [rems.text :refer [get-localized-title localized localize-command localize-role localize-state text text-format text-format-map]]
             [rems.util :refer [navigate! post! put! trim-when-string]]))
 
 (rf/reg-event-fx ::enter-page
@@ -265,9 +265,11 @@
 (rf/reg-event-db ::set-licenses (fn [db [_ licenses]] (assoc-in db [::form :licenses] licenses)))
 
 (defn- render-readonly-licenses [id licenses]
-  (let [licenses (for [license licenses
-                       :let [uri (str "/administration/licenses/" (:license/id license))
-                             title (:title (localized (:localizations license)))]]
+  (let [licenses (for [license (->> licenses
+                                    (mapv #(assoc % ::title (get-localized-title %)))
+                                    (sort-by ::title))
+                       :let [uri (str "/administration/licenses/" (:id license))
+                             title (::title license)]]
                    [atoms/link {} uri title])]
     [fields/readonly-field-raw {:id id
                                 :value (if (seq licenses)
@@ -298,7 +300,8 @@
          :items (->> @(rf/subscribe [::licenses])
                      (mapv #(assoc % ::label (text-format :t.label/parens
                                                           (:title (localized (:localizations %)))
-                                                          (localize-org-short %)))))
+                                                          (localize-org-short %))))
+                     (sort-by ::label))
          :item-key :id
          :item-label ::label
          :item-selected? #(contains? selected-ids (:id %))

src/cljs/rems/administration/license.cljs

@@ -122,7 +122,9 @@
     :open? (<= (count licenses) 5)
     :collapse (if (seq licenses)
                 (into [:div]
-                      (for [license licenses]
+                      (for [license (->> licenses
+                                         (mapv #(assoc % ::title (get-localized-title %)))
+                                         (sort-by ::title))]
                         [license-view-compact license]))
                 [:p (text :t.administration/no-licenses)])}])
 

src/cljs/rems/administration/workflow.cljs

@@ -12,7 +12,7 @@
             [rems.common.roles :as roles]
             [rems.spinner :as spinner]
             [rems.table :as table]
-            [rems.text :refer [localized localize-command localize-role localize-state text text-format]]
+            [rems.text :refer [get-localized-title localized localize-command localize-role localize-state text text-format]]
             [rems.util :refer [fetch]]))
 
 (rf/reg-event-fx
@@ -120,9 +120,11 @@
 (defn- render-licenses [licenses]
   (into [:<>]
         (interpose ", ")
-        (for [license licenses
+        (for [license (->> licenses
+                           (mapv #(assoc % ::title (get-localized-title %)))
+                           (sort-by ::title))
               :let [uri (str "/administration/licenses/" (:license/id license))
-                    title (:title (localized (:localizations license)))]]
+                    title (::title license)]]
           [atoms/link {} uri title])))
 
 (defn- localize-workflow [workflow-type]

src/cljs/rems/application.cljs

@@ -35,7 +35,8 @@
             [rems.administration.duo :refer [duo-field duo-info-field]]
             [rems.common.application-util :refer [accepted-licenses? can-see-everything? form-fields-editable? get-member-name is-handler? is-handling-user?]]
             [rems.common.attachment-util :as attachment-util]
-            [rems.atoms :as atoms :refer [external-link file-download info-field readonly-checkbox document-title success-symbol make-empty-symbol]]
+            [rems.atoms :as atoms :refer [document-title external-link file-download info-field make-empty-symbol readonly-checkbox success-symbol]]
+            [rems.common.atoms :refer [nbsp]]
             [rems.common.catalogue-util :refer [catalogue-item-more-info-url]]
             [rems.collapsible :as collapsible]
             [rems.common.form :as form]
@@ -476,14 +477,14 @@
     [:a.btn.btn-secondary
      {:href (str "/api/applications/" app-id "/pdf")
       :target :_blank}
-     [external-link] " PDF"]))
+     [external-link] " " (text :t.actions/download-pdf)]))
 
 (defn- attachment-zip-button [application]
   (when-not (empty? (:application/attachments application))
     [:a.btn.btn-secondary
      {:href (str "/api/applications/" (:application/id application) "/attachments?all=false")
       :target :_blank}
-     [file-download] " " (text :t.form/attachments-as-zip)]))
+     [file-download] nbsp (text :t.form/attachments-as-zip)]))
 
 (defn- link-license [license]
   (let [title (localized (:license/title license))
@@ -507,7 +508,7 @@
                   "/license-attachment/" (:license/id license)
                   "/" (name @rems.config/current-language))]
     [:a.license-title {:href link :target :_blank}
-     title " " [file-download]]))
+     [file-download] nbsp title]))
 
 (defn license-field [application license show-accepted-licenses?]
   [:div.license.flex-row.d-flex

src/cljs/rems/navbar.cljs

@@ -3,6 +3,7 @@
             [re-frame.core :as rf]
             [rems.ajax]
             [rems.atoms :as atoms]
+            [rems.common.atoms :refer [hamburger]]
             [rems.common.util :refer [getx]]
             [rems.common.roles :as roles]
             [rems.config]
@@ -105,7 +106,7 @@
    [:div.navbar.navbar-expand-sm.flex-fill
     [:button.navbar-toggler
      {:type :button :data-toggle "collapse" :data-target "#small-navbar"}
-     "\u2630"]
+     hamburger]
     (when (rems.theme/use-navbar-logo?)
       [:div.navbar-brand.logo-menu
        [:div.img]])

src/cljs/rems/phase.cljs

@@ -1,5 +1,6 @@
 (ns rems.phase
-  (:require [rems.guide-util :refer [component-info example]]
+  (:require [rems.common.atoms :refer [nbsp]]
+            [rems.guide-util :refer [component-info example]]
             [rems.text :refer [text]]))
 
 (defn phases
@@ -33,7 +34,7 @@
                                     [:span.sr-only (text :t.phases/phase-closed)]]
                   :else [:i.fa.fa-chevron-right {:style {:color "rgba(0,0,0,0)"}}
                          [:span.sr-only (text :t.phases/phase-pending)]])
-            "\u00a0"
+            nbsp
             (if (:text phase)
               (text (:text phase))
               (:phase phase))]])))

test/clj/rems/application/test_model.clj

@@ -7,6 +7,7 @@
             [rems.api.schema :as schema]
             [rems.application.events :as events]
             [rems.application.model :as model]
+            [rems.common.atoms :refer [nbsp nbhy]]
             [rems.common.application-util :as application-util]
             [rems.common.util :refer [deep-merge]]
             [rems.permissions :as permissions]
@@ -371,8 +372,8 @@
         nowrap (fn [s]
                  ;; GitHub will strip all CSS from markdown, so we cannot use CSS for nowrap
                  (-> s
-                     (str/replace " " "\u00A0") ;  non-breaking space
-                     (str/replace "-" "\u2011")))] ; non-breaking hyphen
+                     (str/replace " " nbsp) ;  non-breaking space
+                     (str/replace "-" nbhy)))] ; non-breaking hyphen
     (hiccup/html
      [:table {:border 1}
       [:tr

test/clj/rems/test_browser.clj

@@ -412,15 +412,15 @@
     (btu/scroll-and-click :licensetype-attachment)
     (btu/eventually-visible? :attachment-en) ; inputs are hidden
     (btu/upload-file :upload-license-button-en "test-data/test.txt")
-    (btu/wait-predicate #(= (set [(str "test.txt" attachment-load-text)])
+    (btu/wait-predicate #(= (set [(str attachment-load-text " test.txt")])
                             (set (get-attachments {:css ".page-create-license"})))
                         #(do {:attachments (set (get-attachments {:css ".page-create-license"}))}))
     (btu/upload-file :upload-license-button-fi "test-data/test-fi.txt")
-    (btu/wait-predicate #(= (set [(str "test.txt" attachment-load-text) (str "test-fi.txt" attachment-load-text)])
+    (btu/wait-predicate #(= (set [(str attachment-load-text " test.txt") (str attachment-load-text " test-fi.txt")])
                             (set (get-attachments {:css ".page-create-license"})))
                         #(do {:attachments (get-attachments {:css ".page-create-license"})}))
     (btu/upload-file :upload-license-button-sv "test-data/test-sv.txt")
-    (btu/wait-predicate #(= (set [(str "test.txt" attachment-load-text) (str "test-fi.txt" attachment-load-text) (str "test-sv.txt" attachment-load-text)])
+    (btu/wait-predicate #(= (set [(str attachment-load-text " test.txt") (str attachment-load-text " test-fi.txt") (str attachment-load-text " test-sv.txt")])
                             (set (get-attachments {:css ".page-create-license"})))
                         #(do {:attachments (get-attachments {:css ".page-create-license"})}))))
 
@@ -3446,7 +3446,7 @@
           (select-option "Organisation" "NBN")
           (fill-license-fields {:title "E2E license with attachments"
                                 :attachments true
-                                :attachment-load-text "\nLadda ner fil"})
+                                :attachment-load-text "Ladda ner fil\n"})
           (btu/scroll-and-click :save)
           (btu/screenshot "after-saving-attachments-sv")
           (wait-page-title "Licens – REMS")