one workaround is to have a column formatted like this for example:

ro/column-formatter (fn [report-instance value row-props attribute]
                         (case value
                           "active" (tr "active")
                           "invited" (tr "invited")))

but it feels like the right way would be to have ?! here https://github.com/yenda/fulcro-rad/blob/develop/src/main/com/fulcrologic/rad/report.cljc#L753

So, for string extraction to work right, you always want the former, right?

using tr on database values doesn’t make any sense, since it isn’t an AI

so you always need a fn to do the explicit value-to-value translation, which means column-formatter is the right place to internationalize. You could use tr-unsafe in the column formatter

the problem is that colum-formatter is report specific

and there is already an ao/enunerated-labels for enums

which is its purpose: assert you know you need translations that come into vars

I added the example in the PR:

(def student-statuses  {"active" #(tr "active")
                        "invited" #(tr "invited")})

(defattr status :student/status :enum
  {ao/label #(tr "Status")
   ao/enumerated-values (set (keys student-statuses))
   ao/enumerated-labels student-statuses
   ...})})

but enumerated labels supports a fn.

sorry, I didn’t look at your code link

👀

you’re suggesting this isn’t sufficient:

(if-let [labels (::attr/enumerated-labels column-attribute)]
                            (?! (labels value))
                            (str value))

the value from the database itself will never be a fn, so how would another ?! help here?

OH

I see…yes, you are right. I was looking at your PR

and thinking you were saying that was my code and it needed fixed 😄

but your fix is correct.

well actually I don't know if the fix is needed I'm exploring possible solutions

should we also pass in “this” though?

it looks like enumerated-labels doc is missing the [value] param

because I guess this would work:

ao/enumerated-values #{"active" "invited"}
   ao/enumerated-labels (fn [value]
                          (case value
                            "active" (tr "active")
                            "invited" (tr "invited")))

(testing now)

The docstring says:

(def enumerated-labels
  "RECOMMENDED For data type :enum. A map from enumeration keyword to string (or a `(fn [] string?)` that defines
   the label that should be used for the given enumeration (for example in dropdowns). See `enumerated-values`.

   Labels default to a capitalized version of their keyword name."
  :com.fulcrologic.rad.attributes/enumerated-labels)

and thus the code isn’t right as released

but would be with your patch

(fn [value] string?)

um…yeah, that would probably be nice

we could update the docstring. Won’t hurt to pass a param that no one is expecting

it does seem to work without my patch,

that’s a compatible change

oh?

it’s possible I did something in another layer.

my patch except the value to be a potential fn

the current code expect the whole enumerated-label thing to be a fn

so in the snippet I sent above it works because the whole enumerated labels thing is a fn and not just the values for each label

well, but that would be a breaking change for this default formatter at least

and the docstring does not say enumerated-labels itself is meant to be a fn

not saying it was the perfect choice, just that I can’t change it now.

well, I guess it could support being a fn, since that would be a non-breaking change

Sorry I'm not sure I follow you, to recap if we ignore my PR completely this is the situation: the default formatter does this:

{:default (fn [_ value _ column-attribute]
                          (if-let [labels (::attr/enumerated-labels column-attribute)]
                            (labels value)
                            (str value)))}

A map from enumeration keyword to string (or a `(fn [] string?)

A map from enumeration keyword to string (or a `(fn [value] string?)

no, you’re misunderstanding the code

enumerated labels is a map. Maps are ifn

so (labels value) is just (get labels value)

The docstring is saying that this should be wrapped how your patch does it

(let [value "active"] (if-let [labels (fn [value] (case value
                                                                               "active" (tr "active")
                                                                               "invited" (tr "invited")))]
                                                   (labels value)
                                                   (str value)))
"active"

(labels value) is (get labels value) or ((fn [value] string?) value)

the code could be slightly more clear but it does work as expected if you consider that the docstring was meant to say (fn [value] string?)

again my patch would have been a completely different semantic: currently enumerated-value can be a map (ifn) or a function (taking one arg) and returning a string. with my patch it would have still be the case but on top of that the result of (enumerated-values value) could have been a fn as well. It seems unecessary . however I might not have the whole context and vision on rad so if you think it's still a good idea to add that feel free to merge the PR, but locally I reverted my change and things work great now that I know that enumerated-value can be a fn that takes value as argument

Ah…you’re right, I may have intended it that way. The docstring is ambiguous

I patched the docstring

I got a bit too hopeful it does work with reports but then it breaks forms

probably because of enumerated-field:

(defn enumerated-options [{::form/keys [form-instance] :as env} {::attr/keys [qualified-key] :as attribute}]
  (let [{::attr/keys [enumerated-values]} attribute
        enumeration-labels (merge
                            (::attr/enumerated-labels attribute)
                            (comp/component-options form-instance ::form/enumerated-labels qualified-key))]
    ;; TODO: Sorting should be something users control
    (mapv (fn [k]
            {:text  (?! (get enumeration-labels k (name k)))
             :value k}) enumerated-values)))

But it is fixed by changing it to:

(defn enumerated-options [{::form/keys [form-instance] :as env} {::attr/keys [qualified-key] :as attribute}]
  (let [{::attr/keys [enumerated-values]} attribute
        enumerated-labels (or
                           (::attr/enumerated-labels attribute)
                           (comp/component-options form-instance ::form/enumerated-labels qualified-key))
        options (sort-by :text
                         (mapv (fn [k]
                                 {:text  (or (enumerated-labels k) (name k))
                                  :value k})
                               enumerated-values))]
    options))

ok, so obviously I did mean (fn [] string) as the value in the enumerated labels map. {:k (fn [] (tr "k")) …}

The ability to make the entire option a (fn [value] ) is also workable, esp since a map is exactly that.

I think we can change it to the following and it will not break anything (this is the new docstring for enumerate-labels)…also requires improving the docstring for ::form/enumerated-labels

The labels for the various values an enumeration can take. Can be one of the following:

1. A map from value to label. E.g. {:k "k" ...}
2. A map from value to a `(fn [] string?)`. E.g. {:k #(tr "k") ...}
3. A `(fn [value] string)` E.g. (fn [k] (get my-labels k))

and then of course code changes in all relevant places to make sure all three are supported

so a global search of the SUI plugin and RAD itself for “enumerated-labels” to catch things like the ::form/enumerated-labels override.

they actually don’t appear in very many places

Yeah having the 3 options sounds nice