Hydrate :human_readable_field in :dimensions for public/embedded Fields

src/metabase/models/field.clj

@@ -92,7 +92,7 @@
 (u/strict-extend (class Field)
   models/IModel
   (merge models/IModelDefaults
-         {:hydration-keys (constantly [:destination :field :origin])
+         {:hydration-keys (constantly [:destination :field :origin :human_readable_field])
           :types          (constantly {:base_type       :keyword
                                        :special_type    :keyword
                                        :visibility_type :keyword
@@ -124,9 +124,12 @@
   [{:keys [id]}]
   (db/select [FieldValues :field_id :values], :field_id id))
 
-(defn- keyed-by-field-ids
-  "Queries for `MODEL` instances related by `FIELDS`, returns a map
-  keyed by :field_id"
+(defn- select-field-id->instance
+  "Select instances of `model` related by `field_id` FK to a Field in `fields`, and return a map of Field ID -> model
+  instance. This only returns a single instance for each Field! Duplicates are discarded!
+
+    (select-field-id->instance [(Field 1) (Field 2)] FieldValues)
+    ;; -> {1 #FieldValues{...}, 2 #FieldValues{...}}"
   [fields model]
   (let [field-ids (set (map :id fields))]
     (u/key-by :field_id (when (seq field-ids)
@@ -136,7 +139,7 @@
   "Efficiently hydrate the `FieldValues` for a collection of FIELDS."
   {:batched-hydrate :values}
   [fields]
-  (let [id->field-values (keyed-by-field-ids fields FieldValues)]
+  (let [id->field-values (select-field-id->instance fields FieldValues)]
     (for [field fields]
       (assoc field :values (get id->field-values (:id field) [])))))
 
@@ -144,7 +147,7 @@
   "Efficiently hydrate the `FieldValues` for visibility_type normal FIELDS."
   {:batched-hydrate :normal_values}
   [fields]
-  (let [id->field-values (keyed-by-field-ids (filter fv/field-should-have-field-values? fields)
+  (let [id->field-values (select-field-id->instance (filter fv/field-should-have-field-values? fields)
                                              [FieldValues :id :human_readable_values :values :field_id])]
     (for [field fields]
       (assoc field :values (get id->field-values (:id field) [])))))
@@ -153,7 +156,12 @@
   "Efficiently hydrate the `Dimension` for a collection of FIELDS."
   {:batched-hydrate :dimensions}
   [fields]
-  (let [id->dimensions (keyed-by-field-ids fields Dimension)]
+  ;; TODO - it looks like we obviously thought this code would return *all* of the Dimensions for a Field, not just
+  ;; one! This code is obviously wrong! It will either assoc a single Dimension or an empty vector under the
+  ;; `:dimensions` key!!!!
+  ;; TODO - consult with tom and see if fixing this will break any hacks that surely must exist in the frontend to deal
+  ;; with this
+  (let [id->dimensions (select-field-id->instance fields Dimension)]
     (for [field fields]
       (assoc field :dimensions (get id->dimensions (:id field) [])))))
 

src/metabase/models/params.clj

@@ -54,12 +54,21 @@
   [fields]
   (filter #(isa? (:special_type %) :type/PK) fields))
 
+(def ^:private Field:params-columns-only
+  "Form for use in Toucan `db/select` expressions (as a drop-in replacement for using `Field`) that returns Fields with
+  only the columns that are appropriate for returning in public/embedded API endpoints, which make heavy use of the
+  functions in this namespace. Use `conj` to add additional Fields beyond the ones already here. Use `rest` to get
+  just the column identifiers, perhaps for use with something like `select-keys`. Clutch!
+
+    (db/select Field:params-columns-only)"
+  ['Field :id :table_id :display_name :base_type :special_type])
+
 (defn- fields->table-id->name-field
   "Given a sequence of `fields,` return a map of Table ID -> to a `:type/Name` Field in that Table, if one exists. In
   cases where more than one name Field exists for a Table, this just adds the first one it finds."
   [fields]
   (when-let [table-ids (seq (map :table_id fields))]
-    (u/key-by :table_id (db/select ['Field :id :table_id :display_name :base_type :special_type]
+    (u/key-by :table_id (db/select (conj Field:params-columns-only :table_id)
                           :table_id     [:in table-ids]
                           :special_type (mdb/isa :type/Name)))))
 
@@ -71,7 +80,6 @@
   {:batched-hydrate :name_field}
   [fields]
   (let [table-id->name-field (fields->table-id->name-field (pk-fields fields))]
-    (println "table-id->name-field:" table-id->name-field) ; NOCOMMIT
     (for [field fields]
       (-> field
           ;; add matching `:name_field` if it's a PK
@@ -83,15 +91,45 @@
           (dissoc :table_id)))))
 
 
+;; We hydrate the `:human_readable_field` for each Dimension using the usual hydration logic, so it contains columns we
+;; don't want to return. The two functions below work to remove the unneeded ones.
+
+(defn- remove-dimension-nonpublic-columns
+  "Strip nonpublic columns from a `dimension` and from its hydrated human-readable Field."
+  [dimension]
+  (-> dimension
+      (update :human_readable_field #(select-keys % (rest Field:params-columns-only)))
+      ;; these aren't exactly secret but you the frontend doesn't need them either so while we're at it let's go ahead
+      ;; and strip them out
+      (dissoc :created_at :updated_at)))
+
+(defn- remove-dimensions-nonpublic-columns
+  "Strip nonpublic columns from the hydrated human-readable Field in the hydrated Dimensions in `fields`."
+  [fields]
+  (for [field fields]
+    (update field :dimensions
+            (fn [dimension-or-dimensions]
+              ;; as disucssed in `metabase.models.field` the hydration code for `:dimensions` is
+              ;; WRONG and the value ends up either being a single Dimension or an empty vector.
+              ;; However at some point we will fix this so deal with either a map or a sequence of
+              ;; maps
+              (cond
+                (map? dimension-or-dimensions)
+                (remove-dimension-nonpublic-columns dimension-or-dimensions)
+
+                (sequential? dimension-or-dimensions)
+                (map remove-dimension-nonpublic-columns dimension-or-dimensions))))))
+
+
 (defn- param-field-ids->fields
   "Get the Fields (as a map of Field ID -> Field) that shoudl be returned for hydrated `:param_fields` for a Card or
   Dashboard. These only contain the minimal amount of information neccesary needed to power public or embedded
   parameter widgets."
   [field-ids]
   (when (seq field-ids)
-    (u/key-by :id (-> (db/select ['Field :id :table_id :display_name :base_type :special_type]
-                        :id [:in field-ids])
-                      (hydrate :has_field_values :name_field :dimensions)))))
+    (u/key-by :id (-> (db/select (conj Field:params-columns-only :table_id) :id [:in field-ids])
+                      (hydrate :has_field_values :name_field [:dimensions :human_readable_field])
+                      remove-dimensions-nonpublic-columns))))
 
 (defmulti ^:private ^{:hydrate :param_values} param-values
   "Add a `:param_values` map (Field ID -> FieldValues) containing FieldValues for the Fields referenced by the