Repository: CSCfi/rems
Pull request #3181 (2023-08-29 17:18:10)

feat: support named arguments in localization
relates #3183 

- add tempura support for map arguments with custom resource compiler `rems.tempura/resource-compiler`
- use `rems.tempura/tr` as "base" function for `rems.text` (`rems.tempura` is shared code namespace)
- removed `rems.context/*tempura*` with goal to share more code between clj/cljs

Localization strings may use either map or vector argument syntax, but not both. `rems.tempura/resource-compiler` can switch between map argument and vector arguments when both are provided (e.g. from `rems.text/text-format`):

```clojure
(text-format :k {:arg1 :val1 :arg 2 :val2} :val1 :val2)
;;=> works for {:k "%1 %2"}
;;=> and {:k "%:arg1% %:arg2%"}
```

Custom resource compiler works as so:
1. When translation fn is called, check provided arguments (vector).
2. If arguments contain map as first element, check if resource (localization string) contains vector indexes ("%1" etc).
2.1. If yes, drop first argument `(rest arguments)` and use resource as-is.
2.2. If not, walk through resource and replace any keyword arguments `"%:kw%"` with equivalent vector index argument `"%1"`. Use replaced resource string, and map argument map to ordered argument indexes (creating vector argument list): `(mapv {:kw "%1"} [:kw]) ;=> ["%1"]`
3. Call tempura default resource compiler with returned resource and argument vector.

# Checklist for author

Remove items that aren't applicable, check items that are done.

## Reviewability
- [x] Link to issue

## Documentation
- [x] Update changelog if necessary
- [x] Update docs/ (if applicable)
- [ ] ADR for major architectural decisions or experiments (experiment for sure, but maybe not architectural decision?)

## Testing
- [x] Complex logic is unit tested

## Follow-up
- [ ] New tasks are created for pending or remaining tasks (tasks maybe for updating email templates to use new translation keys?)



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0f145fc68..9860746e0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ Changes since v2.33
 ### Additions
 - (Experimental) Workflow can be configured to enable voting for the approval. Currently all handlers can vote (including bots). Use `:enable-voting`. (#3174)
 - There is now a Danish language translation (#3176). We are considering supporting a limited set of languages officially, and improving support for community maintained translations (see #3179).
+- Added experimental support for named format parameters in translations. (#3183)
 
 ### Fixes
 - Email template parameters for `:application-expiration-notification` event are now documented. The parameters are different from standard event email parameters, which may have caused confusion.
diff --git a/docs/configuration.md b/docs/configuration.md
index ea2c56e9d..0f709ffee 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -144,6 +144,12 @@ for a list of all translation keys and their format parameters. Format
 parameters are pieces of text like `%3` that get replaced with certain
 information.
 
+### Localization format parameters
+
+Localizations may use format parameters for dynamic translations, e.g. emails. Format parameters are pieces of text that get replaced with certain information. Vector format parameters are default, which typically look like `"Application %1"`, where `"%1"` refers to specific position in the translation arguments.
+
+Experimental support exists for named format parameters, which can be used alternatively. Named format parameters are included in text like `"Application %:application/id%"`, and unlike vector format parameters, do not rely on specific ordering of translation arguments.
+
 ## Themes
 
 Custom themes can be used by creating a file, for example `my-custom-theme.edn`, and specifying its location in the `:theme-path` configuration parameter. The theme file can override some or all of the theme attributes (see `:theme` in [config-defaults.edn](https://github.com/CSCfi/rems/blob/master/resources/config-defaults.edn)). Static resources can be placed in a `public` directory next to the theme configuration file. See [example-theme/theme.edn](https://github.com/CSCfi/rems/blob/master/example-theme/theme.edn) for an example.
diff --git a/example-theme/extra-translations/en.edn b/example-theme/extra-translations/en.edn
index 4c21b4e3b..054de43b7 100644
--- a/example-theme/extra-translations/en.edn
+++ b/example-theme/extra-translations/en.edn
@@ -3,4 +3,7 @@
   ;; the extra %1 and the :unused-key are for rems.test-locales
   :create-license {:license-text "Text %1"}
   :unused-key "Unused"
-  :login {:errors {:group "group"}}}}
+  :login {:errors {:group "group"}}
+  ;; example of using named parameters instead of vector index params
+  :applications {:view-application-without-description "View application %:application/id% for resources: %:catalogue-item-names%"
+                 :view-application-with-description "View application %:application/id%: %:application/description%"}}}
diff --git a/src/clj/rems/context.clj b/src/clj/rems/context.clj
index 9087c4f1b..83588cec2 100644
--- a/src/clj/rems/context.clj
+++ b/src/clj/rems/context.clj
@@ -15,8 +15,6 @@
 
 (def ^:dynamic ^{:doc "Set of roles for user (or nil)"} *roles*)
 
-(def ^:dynamic ^{:doc "Tempura object initialized with user's preferred language."} *tempura*)
-
 (def ^:dynamic ^{:doc "User's preferred language."} *lang*)
 
 (def ^:dynamic ^{:doc "Ongoing HTTP request if any."} *request*)
diff --git a/src/cljc/rems/tempura.cljc b/src/cljc/rems/tempura.cljc
new file mode 100644
index 000000000..67f608b5f
--- /dev/null
+++ b/src/cljc/rems/tempura.cljc
@@ -0,0 +1,96 @@
+(ns rems.tempura
+  (:require [clojure.string]
+            [clojure.test :refer [deftest is testing]]
+            [clojure.walk]
+            #?(:cljs [re-frame.core :as rf])
+            #?@(:clj [[rems.context :as context]
+                      [rems.locales]])
+            [taoensso.tempura]))
+
+;; backtick (`) is used to escape vector argument (%)
+(defonce ^:private +map-args+ #"(?<!`)%:([^\s`%]+)%")
+(defonce ^:private +vector-args+ #"(?<!`)%\d")
+
+(deftest test-map-args
+  (is (= ["key" "my.special/namespace?"]
+         (->> "%:key% for (%:my.special/namespace?%) `%:ignored% %:also-ignored`%"
+              (re-seq +map-args+)
+              (mapv last)))))
+
+(defn- replace-map-args [resource]
+  (let [res-keys (atom {})
+        idx (atom 0)
+        upsert! (fn [k]
+                  (when-not (contains? @res-keys k)
+                    (swap! res-keys assoc k (swap! idx inc)))
+                  (get @res-keys k))
+        resource (->> resource
+                      (clojure.walk/postwalk
+                       (fn [node]
+                         (if (string? node)
+                           (clojure.string/replace node +map-args+ #(let [map-arg (keyword (second %))
+                                                                          vec-arg (upsert! map-arg)]
+                                                                      (str "%" vec-arg)))
+                           node))))]
+    {:resource resource
+     :resource-keys (->> (sort-by val @res-keys)
+                         (mapv key))}))
+
+(def ^:private memoized-replace-map-args (memoize replace-map-args))
+
+(deftest test-replace-map-args
+  (testing "string transformation"
+    (is (= {:resource "{:x %1 :y %2}"
+            :resource-keys [:x :y]}
+           (replace-map-args "{:x %:x% :y %:y%}"))))
+  (testing "memoized"
+    (let [memoized-f (memoize replace-map-args)]
+      (is (= {:resource "{:x %1 :y %2}"
+              :resource-keys [:x :y]}
+             (memoized-f "{:x %:x% :y %:y%}")
+             (memoized-f "{:x %:x% :y %:y%}")))))
+  (testing "hiccup transformation"
+    (is (= {:resource [:div {:aria-label "argument x is %1, argument y is %2"} "{:x %1 :y %2}"]
+            :resource-keys [:x :y]}
+           (replace-map-args [:div {:aria-label "argument x is %:x%, argument y is %:y%"} "{:x %:x% :y %:y%}"])))))
+
+(def ^:private get-resource-compiler (:resource-compiler taoensso.tempura/default-tr-opts))
+
+(defn- compile-vec-args [resource vargs]
+  (let [compile-vargs (get-resource-compiler resource)]
+    (compile-vargs (vec vargs))))
+
+(defn- compile-map-args [resource arg-map]
+  (let [res-map (memoized-replace-map-args resource)]
+    (compile-vec-args (:resource res-map)
+                      (map arg-map (:resource-keys res-map)))))
+
+(defn- tempura-config []
+  {:dict #?(:clj rems.locales/translations
+            :cljs @(rf/subscribe [:translations]))
+   :resource-compiler (fn [resource]
+                        (fn [vargs]
+                          (cond
+                            (map? (first vargs)) (if (re-find +vector-args+ resource)
+                                                   (compile-vec-args resource (rest vargs))
+                                                   (compile-map-args resource (first vargs)))
+                            :else (compile-vec-args resource vargs))))})
+
+(defn get-language []
+  #?(:clj context/*lang*
+     :cljs @(rf/subscribe [:language])))
+
+(defn tr
+  "When translation function is called with both map and vector arguments,
+   custom resource compiler can use either argument format for translation.
+   Argument formats cannot be mixed. When using both argument formats,
+   map argument must be given first followed by vector arguments:
+
+   (tr [:key] [{:k :v} x1 x2])"
+  ([ks args] (taoensso.tempura/tr (tempura-config)
+                                  [(get-language)]
+                                  (vec ks)
+                                  (vec args)))
+  ([ks] (taoensso.tempura/tr (tempura-config)
+                             [(get-language)]
+                             (vec ks))))
diff --git a/src/cljc/rems/text.cljc b/src/cljc/rems/text.cljc
index c6175f0ea..3d433013e 100644
--- a/src/cljc/rems/text.cljc
+++ b/src/cljc/rems/text.cljc
@@ -6,16 +6,13 @@
                :cljs [cljs-time.format :as time-format])
             #?(:cljs [cljs-time.coerce :as time-coerce])
             [clojure.string :as str]
-            #?(:cljs [re-frame.core :as rf])
             [rems.common.application-util :as application-util]
             #?(:clj [rems.context :as context])
-            #?(:clj [rems.locales :as locales])
-            [taoensso.tempura :refer [tr]]))
+            [rems.tempura]))
 
 #?(:clj
    (defmacro with-language [lang & body]
-     `(binding [rems.context/*lang* ~lang
-                rems.context/*tempura* (partial tr (rems.locales/tempura-config) [~lang])]
+     `(binding [rems.context/*lang* ~lang]
         (assert (keyword? ~lang) {:lang ~lang})
         ~@body)))
 
@@ -27,53 +24,50 @@
                  (cons k args)))))
 
 (defn text-format
-  "Return the tempura translation for a given key & time arguments"
+  "Return the tempura translation for a given key and arguments.
+   Map can be used for named parameters as `(first args)`,
+   when the localization resource supports them. `(rest args)`
+   are then used as vector arguments if localization resource does not support
+   named parameters. See `rems.tempura/tr`
+
+   (text-format :key {:arg :value} varg-1 varg-2 ...)"
   [k & args]
-  #?(:clj (context/*tempura* [k :t/missing] (vec args))
-     :cljs (let [translations (rf/subscribe [:translations])
-                 language (rf/subscribe [:language])]
-             (tr {:dict @translations}
-                 [@language]
-                 [k :t/missing (failsafe-fallback k args)]
-                 (vec args)))))
+  #?(:clj (rems.tempura/tr [k :t/missing]
+                           args)
+     :cljs (rems.tempura/tr [k :t/missing (failsafe-fallback k args)]
+                            args)))
 
 (defn text-no-fallback
   "Return the tempura translation for a given key. Additional fallback
   keys can be given but there is no default fallback text."
   [& ks]
-  #?(:clj (context/*tempura* (vec ks))
-     :cljs (let [translations (rf/subscribe [:translations])
-                 language (rf/subscribe [:language])]
-             (try
-               (tr {:dict @translations}
-                   [@language]
-                   (vec ks))
-               (catch js/Object e
-                 ;; fail gracefully if the re-frame state is incomplete
-                 (.error js/console e)
-                 (str (vec ks)))))))
+  #?(:clj (rems.tempura/tr ks)
+     :cljs (try
+             (rems.tempura/tr ks)
+             (catch js/Object e
+               ;; fail gracefully if the re-frame state is incomplete
+               (.error js/console e)
+               (str (vec ks))))))
 
 (defn text
   "Return the tempura translation for a given key. Additional fallback
   keys can be given."
   [& ks]
-  #?(:clj (apply text-no-fallback (conj (vec ks) (text-format :t/missing (vec ks))))
+  #?(:clj (apply text-no-fallback
+                 (conj (vec ks)
+                       (text-format :t/missing (vec ks))))
      ;; NB: we can't call the text-no-fallback here as in CLJS
      ;; we can both call this as function or use as a React component
-     :cljs (let [translations (rf/subscribe [:translations])
-                 language (rf/subscribe [:language])]
-             (try
-               (tr {:dict @translations}
-                   [@language]
-                   (conj (vec ks) (text-format :t/missing (vec ks))))
-               (catch js/Object e
-                 ;; fail gracefully if the re-frame state is incomplete
-                 (.error js/console e)
-                 (str (vec ks)))))))
+     :cljs (try
+             (rems.tempura/tr (conj (vec ks)
+                                    (text-format :t/missing (vec ks))))
+             (catch js/Object e
+               ;; fail gracefully if the re-frame state is incomplete
+               (.error js/console e)
+               (str (vec ks))))))
 
 (defn localized [m]
-  (let [lang #?(:clj context/*lang*
-                :cljs @(rf/subscribe [:language]))]
+  (let [lang (rems.tempura/get-language)]
     (or (get m lang)
         (first (vals m)))))
 
@@ -153,7 +147,6 @@
             (is (= "2020-09-29" (localize-utc-date (time/to-time-zone (time/date-time 2020 9 29 1 1)
                                                                       (time/time-zone-for-offset -5))))))))
 
-
 (def ^:private event-types
   {:application.event/applicant-changed :t.applications.events/applicant-changed
    :application.event/approved :t.applications.events/approved
diff --git a/src/cljs/rems/application_list.cljs b/src/cljs/rems/application_list.cljs
index 2adf9008c..4005a7b8b 100644
--- a/src/cljs/rems/application_list.cljs
+++ b/src/cljs/rems/application_list.cljs
@@ -26,14 +26,18 @@
 
 (defn- view-button [app]
   (let [config @(rf/subscribe [:rems.config/config])
-        id (format-application-id config app)]
+        id (format-application-id config app)
+        description (:application/description app)]
     [atoms/link
      {:class "btn btn-primary"
-      :aria-label (if (str/blank? (:application/description app))
-                    (text-format :t.applications/view-application-without-description
-                                 id (format-catalogue-items app))
+      :aria-label (if (str/blank? description)
+                    (let [catalogue-items (format-catalogue-items app)]
+                      (text-format :t.applications/view-application-without-description
+                                   {:application/id id :catalogue-item-names catalogue-items}
+                                   id catalogue-items))
                     (text-format :t.applications/view-application-with-description
-                                 id (:application/description app)))}
+                                 {:application/id id :application/description description}
+                                 id description))}
      (str "/application/" (:application/id app))
      (text :t.applications/view)]))
 
@@ -69,7 +73,6 @@
           threshold (time/plus start seconds)]
       (time/after? (time/now) threshold))))
 
-
 (rf/reg-sub
  ::table-rows
  (fn [[_ apps-sub] _]
@@ -131,7 +134,6 @@
            :view {:display-value [:div.commands.justify-content-end [view-button app]]}})
         apps)))
 
-
 (defn list [{:keys [id applications visible-columns default-sort-column default-sort-order]
              :or {visible-columns (constantly true)}}]
   (let [all-columns [{:key :id


Test patch:
diff --git a/test/clj/rems/test_db.clj b/test/clj/rems/test_db.clj
index 745036421..bbddb1201 100644
--- a/test/clj/rems/test_db.clj
+++ b/test/clj/rems/test_db.clj
@@ -8,11 +8,10 @@
             [rems.db.roles :as roles]
             [rems.service.test-data :as test-data]
             [rems.db.test-data-helpers :as test-helpers]
-            [rems.db.testing :refer [test-db-fixture rollback-db-fixture]]
-            [rems.testing-tempura :refer [fake-tempura-fixture]])
+            [rems.db.testing :refer [test-db-fixture rollback-db-fixture]])
   (:import (rems.auth ForbiddenException)))
 
-(use-fixtures :once fake-tempura-fixture test-db-fixture)
+(use-fixtures :once test-db-fixture)
 (use-fixtures :each rollback-db-fixture)
 
 (deftest test-get-catalogue-items
diff --git a/test/clj/rems/test_locales.clj b/test/clj/rems/test_locales.clj
index 47d6f5f92..e47995ffa 100644
--- a/test/clj/rems/test_locales.clj
+++ b/test/clj/rems/test_locales.clj
@@ -3,10 +3,11 @@
             [clojure.java.shell :as sh]
             [clojure.set :as set]
             [clojure.string :as str]
-            [clojure.test :refer :all]
+            [clojure.test :refer [deftest is testing]]
             [clojure.tools.logging]
             [rems.common.util :refer [recursive-keys]]
             [rems.locales :as locales]
+            [rems.tempura]
             [rems.testing-util :refer [create-temp-dir]]
             [rems.util :refer [getx-in delete-directory-recursively]]
             [taoensso.tempura.impl :refer [compile-dictionary]])
@@ -47,27 +48,32 @@
   (is (= #{"%3" "%5" "%7"} (locales/extract-format-parameters "user %3 has made %7 alterations in %5!"))))
 
 (deftest test-format-parameters-match
-  #_(testing "[:en vs :da]"
-      (let [en (loc-en)
-            da (loc-da)]
-        (doseq [k (recursive-keys en)] ;; we check that keys match separately
-          (testing k
-            (is (= (locales/extract-format-parameters (getx-in en (vec k)))
-                   (locales/extract-format-parameters (getx-in da (vec k)))))))))
-  (testing "[:en vs :fi]"
-    (let [en (loc-en)
-          fi (loc-fi)]
-      (doseq [k (recursive-keys en)] ;; we check that keys match separately
-        (testing k
-          (is (= (locales/extract-format-parameters (getx-in en (vec k)))
-                 (locales/extract-format-parameters (getx-in fi (vec k)))))))))
-  (testing "[:en vs :sv]"
-    (let [en (loc-en)
-          sv (loc-sv)]
-      (doseq [k (recursive-keys en)]
-        (testing k
-          (is (= (locales/extract-format-parameters (getx-in en (vec k)))
-                 (locales/extract-format-parameters (getx-in sv (vec k))))))))))
+  (let [en (loc-en)
+        da (loc-da)
+        fi (loc-fi)
+        sv (loc-sv)
+        translation-keys (recursive-keys en)]
+    #_(testing "[:en vs :da]"
+        (doseq [ks translation-keys]
+          (testing ks
+            (is (= (locales/extract-format-parameters (getx-in en (vec ks)))
+                   (locales/extract-format-parameters (getx-in da (vec ks)))))
+            (is (= (set (:resource-keys (#'rems.tempura/replace-map-args (getx-in en (vec ks)))))
+                   (set (:resource-keys (#'rems.tempura/replace-map-args (getx-in da (vec ks))))))))))
+    (testing "[:en vs :fi]"
+      (doseq [ks translation-keys]
+        (testing ks
+          (is (= (locales/extract-format-parameters (getx-in en (vec ks)))
+                 (locales/extract-format-parameters (getx-in fi (vec ks)))))
+          (is (= (set (:resource-keys (#'rems.tempura/replace-map-args (getx-in en (vec ks)))))
+                 (set (:resource-keys (#'rems.tempura/replace-map-args (getx-in fi (vec ks))))))))))
+    (testing "[:en vs :sv]"
+      (doseq [ks translation-keys]
+        (testing ks
+          (is (= (locales/extract-format-parameters (getx-in en (vec ks)))
+                 (locales/extract-format-parameters (getx-in sv (vec ks)))))
+          (is (= (set (:resource-keys (#'rems.tempura/replace-map-args (getx-in en (vec ks)))))
+                 (set (:resource-keys (#'rems.tempura/replace-map-args (getx-in sv (vec ks))))))))))))
 
 (defn- translation-keywords-in-use []
   ;; git grep would be nice, but circleci's git grep doesn't have -o
diff --git a/test/clj/rems/testing_tempura.clj b/test/clj/rems/testing_tempura.clj
deleted file mode 100644
index 2b41ed67c..000000000
--- a/test/clj/rems/testing_tempura.clj
+++ /dev/null
@@ -1,13 +0,0 @@
-(ns rems.testing-tempura
-  (:require [rems.context :as context]))
-
-(defn fake-tempura [& args]
-  (pr-str args))
-
-(defmacro with-fake-tempura [& body]
-  `(binding [context/*tempura* fake-tempura]
-     ~@body))
-
-(defn fake-tempura-fixture [f]
-  (with-fake-tempura
-    (f)))