Allow remapping fields with null values (#13033)

frontend/src/metabase/admin/datamodel/components/FieldRemapping.jsx

@@ -66,7 +66,9 @@ export default class FieldRemapping extends React.Component {
     // (for a field without user-defined remappings, every key of `field.remappings` has value `undefined`)
     const hasMappableNumeralValues =
       field.remapping.size > 0 &&
-      [...field.remapping.keys()].every(key => typeof key === "number");
+      [...field.remapping.keys()].every(
+        key => typeof key === "number" || key === null,
+      );
 
     return [
       MAP_OPTIONS.original,

src/metabase/api/field.clj

@@ -214,7 +214,7 @@
   "Update the fields values and human-readable values for a `Field` whose special type is
   `category`/`city`/`state`/`country` or whose base type is `type/Boolean`. The human-readable values are optional."
   [id :as {{value-pairs :values} :body}]
-  {value-pairs [[(s/one s/Num "value") (s/optional su/NonBlankString "human readable value")]]}
+  {value-pairs [[(s/one (s/maybe s/Num) "value") (s/optional su/NonBlankString "human readable value")]]}
   (let [field (api/write-check Field id)]
     (api/check (field-values/field-should-have-field-values? field)
       [400 (str "You can only update the human readable values of a mapped values of a Field whose value of "

src/metabase/query_processor/middleware/add_dimension_projections.clj

@@ -155,14 +155,14 @@
   "Converts `values` to a type compatible with the base_type found for `col`. These values should be directly comparable
   with the values returned from the database for the given `col`."
   [{:keys [base_type] :as col} values]
-  (map (condp #(isa? %2 %1) base_type
-         :type/Decimal    bigdec
-         :type/Float      double
-         :type/BigInteger bigint
-         :type/Integer    int
-         :type/Text       str
-         identity)
-       values))
+  (let [transform (condp #(isa? %2 %1) base_type
+                    :type/Decimal    bigdec
+                    :type/Float      double
+                    :type/BigInteger bigint
+                    :type/Integer    int
+                    :type/Text       str
+                    identity)]
+    (map #(some-> % transform) values)))
 
 (def ^:private InternalDimensionInfo
   {;; index of original column

test/metabase/api/field_test.clj

@@ -192,18 +192,18 @@
   (testing "POST /api/field/:id/values"
     (testing "Existing field values can be updated (with their human readable values)"
       (mt/with-temp* [Field [{field-id :id} list-field]
-                      FieldValues [{field-value-id :id} {:values (range 1 5), :field_id field-id}]]
+                      FieldValues [{field-value-id :id} {:values (conj (range 1 5) nil), :field_id field-id}]]
         (testing "fetch initial values"
-          (is (= {:values [[1] [2] [3] [4]], :field_id true}
+          (is (= {:values [[nil] [1] [2] [3] [4]], :field_id true}
                  (mt/boolean-ids-and-timestamps
                   ((mt/user->client :crowberto) :get 200 (format "field/%d/values" field-id))))))
         (testing "update values"
           (is (= {:status "success"}
                  (mt/boolean-ids-and-timestamps
                   ((mt/user->client :crowberto) :post 200 (format "field/%d/values" field-id)
-                   {:values [[1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]]})))))
+                   {:values [[nil "no $"] [1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]]})))))
         (testing "fetch updated values"
-          (is (= {:values [[1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]], :field_id true}
+          (is (= {:values [[nil "no $"] [1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]], :field_id true}
                  (mt/boolean-ids-and-timestamps
                   ((mt/user->client :crowberto) :get 200 (format "field/%d/values" field-id))))))))))
 

test/metabase/query_processor/middleware/add_dimension_projections_test.clj

@@ -143,15 +143,16 @@
                                       (cond-> field
                                         (= field-name "CATEGORY_ID")
                                         (assoc :dimensions {:type :internal, :name "Foo", :field_id 10}
-                                               :values     {:human_readable_values ["Foo" "Bar" "Baz" "Qux"]
-                                                            :values                [4 11 29 20]}))))]
+                                               :values     {:human_readable_values ["Foo" "Bar" "Baz" "Qux" "Quux"]
+                                                            :values                [4 11 29 20 nil]}))))]
       (is (= {:status    :completed
-              :row_count 5
-              :data      {:rows [[1 "Red Medicine"                  4 3 "Foo"]
-                                 [2 "Stout Burgers & Beers"        11 2 "Bar"]
-                                 [3 "The Apple Pan"                11 2 "Bar"]
-                                 [4 "Wurstküche"                   29 2 "Baz"]
-                                 [5 "Brite Spot Family Restaurant" 20 2 "Qux"]]
+              :row_count 6
+              :data      {:rows [[1 "Red Medicine"                   4 3 "Foo"]
+                                 [2 "Stout Burgers & Beers"         11 2 "Bar"]
+                                 [3 "The Apple Pan"                 11 2 "Bar"]
+                                 [4 "Wurstküche"                    29 2 "Baz"]
+                                 [5 "Brite Spot Family Restaurant"  20 2 "Qux"]
+                                 [6 "Spaghetti Warehouse"          nil 2 "Quux"]]
                           :cols [example-result-cols-id
                                  example-result-cols-name
                                  (assoc example-result-cols-category-id
@@ -166,11 +167,50 @@
                         example-result-cols-name
                         example-result-cols-category-id
                         example-result-cols-price]}
-                [[1 "Red Medicine"                  4 3]
-                 [2 "Stout Burgers & Beers"        11 2]
-                 [3 "The Apple Pan"                11 2]
-                 [4 "Wurstküche"                   29 2]
-                 [5 "Brite Spot Family Restaurant" 20 2]]))))))
+                [[1 "Red Medicine"                   4 3]
+                 [2 "Stout Burgers & Beers"         11 2]
+                 [3 "The Apple Pan"                 11 2]
+                 [4 "Wurstküche"                    29 2]
+                 [5 "Brite Spot Family Restaurant"  20 2]
+                 [6 "Spaghetti Warehouse"          nil 2]]))))))
+
+  (testing "remapping string columns with `human_readable_values`"
+    ;; swap out `hydrate` with one that will add some fake dimensions and values for CATEGORY_ID.
+    (with-redefs [hydrate/hydrate (fn [fields & _]
+                                    (for [{field-name :name, :as field} fields]
+                                      (cond-> field
+                                        (= field-name "NAME")
+                                        (assoc :dimensions {:type :internal, :name "Foo", :field_id 10}
+                                               :values     {:human_readable_values ["Appletini" "Bananasplit" "Kiwi-flavored Thing"]
+                                                            :values                ["apple" "banana" "kiwi"]}))))]
+      (is (= {:status    :completed
+              :row_count 3
+              :data      {:rows [[1 "apple"   4 3 "Appletini"]
+                                 [2 "banana" 11 2 "Bananasplit"]
+                                 [3 "kiwi"   11 2 "Kiwi-flavored Thing"]]
+                          :cols [example-result-cols-id
+                                 (assoc example-result-cols-name
+                                        :remapped_to "Foo")
+                                 example-result-cols-category-id
+                                 example-result-cols-price
+                                 (assoc example-result-cols-foo
+                                        :remapped_from "NAME")]}}
+             (with-redefs [add-dim-projections/add-fk-remaps (fn [query]
+                                                               [nil query])]
+               (add-remapping
+                {}
+                {:cols [example-result-cols-id
+                        example-result-cols-name
+                        example-result-cols-category-id
+                        example-result-cols-price]}
+                [[1 "apple"   4 3]
+                 [2 "banana" 11 2]
+                 [3 "kiwi"   11 2]]))))))
+
+  (testing "test that different columns types are transformed"
+    (is (= (map list [123M 123.0 123N 123 "123"])
+           (map #(#'add-dim-projections/transform-values-for-col {:base_type %} [123])
+                [:type/Decimal :type/Float :type/BigInteger :type/Integer :type/Text]))))
 
   (testing "test that external remappings get the appropriate `:remapped_from`/`:remapped_to` info"
     (is (= {:status    :completed