diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj
index 9bc4e8da5cc91b6d28d876f3fd0e601d4d692e0b..4217b075573922dbcecc3e1e2c0b4fdf9c178384 100644
--- a/src/metabase/models/field_values.clj
+++ b/src/metabase/models/field_values.clj
@@ -79,6 +79,16 @@
(when (values-less-than-total-max-length? values)
values))))
+(defn- fixup-human-readable-values
+ "Field values and human readable values are lists that are zipped
+ together. If the field values have changes, the human readable
+ values will need to change too. This function reconstructs the
+ human_readable_values to reflect `NEW-VALUES`. If a new field value
+ is found, a string version of that is used"
+ [{old-values :values, old-hrv :human_readable_values} new-values]
+ (when (seq old-hrv)
+ (let [orig-remappings (zipmap old-values old-hrv)]
+ (map #(get orig-remappings % (str %)) new-values))))
(defn create-or-update-field-values!
"Create or update the FieldValues object for FIELD. If the FieldValues object already exists, then update values for
@@ -92,8 +102,9 @@
(and field-values values)
(do
(log/debug (format "Storing updated FieldValues for Field %s..." field-name))
- (db/update! FieldValues (u/get-id field-values)
- :values values))
+ (db/update-non-nil-keys! FieldValues (u/get-id field-values)
+ :values values
+ :human_readable_values (fixup-human-readable-values field-values values)))
;; if FieldValues object doesn't exist create one
values
(do
diff --git a/test/metabase/models/field_values_test.clj b/test/metabase/models/field_values_test.clj
index 917b1c016445ce31237b32263bb373e03a4a7bdb..2b93f6ef1416d61dfba5134894bfab72abf68389 100644
--- a/test/metabase/models/field_values_test.clj
+++ b/test/metabase/models/field_values_test.clj
@@ -1,5 +1,10 @@
(ns metabase.models.field-values-test
- (:require [expectations :refer :all]
+ (:require [clojure.java.jdbc :as jdbc]
+ [expectations :refer :all]
+ [metabase
+ [db :as mdb]
+ [sync :as sync]
+ [util :as u]]
[metabase.models
[database :refer [Database]]
[field :refer [Field]]
@@ -50,3 +55,57 @@
(do
(clear-field-values! field-id)
(db/select-one-field :values FieldValues, :field_id field-id))]))
+
+(defn- find-values [field-values-id]
+ (-> (db/select-one FieldValues :id field-values-id)
+ (select-keys [:values :human_readable_values])))
+
+(defn- sync-and-find-values [db field-values-id]
+ (sync/sync-database! db)
+ (find-values field-values-id))
+
+;; Test "fixing" of human readable values when field values change
+(expect
+ (concat (repeat 2 {:values [1 2 3] :human_readable_values ["a" "b" "c"]})
+ (repeat 2 {:values [-2 -1 0 1 2 3] :human_readable_values ["-2" "-1" "0" "a" "b" "c"]})
+ [{:values [-2 -1 0] :human_readable_values ["-2" "-1" "0"]}])
+
+ (binding [mdb/*allow-potentailly-unsafe-connections* true]
+ ;; Create a temp warehouse database that can have it's field values change
+ (jdbc/with-db-connection [conn {:classname "org.h2.Driver", :subprotocol "h2", :subname "mem:temp"}]
+ (jdbc/execute! conn ["drop table foo if exists"])
+ (jdbc/execute! conn ["create table foo (id integer primary key, category_id integer not null, desc text)"])
+ (jdbc/insert-multi! conn :foo [{:id 1 :category_id 1 :desc "foo"}
+ {:id 2 :category_id 2 :desc "bar"}
+ {:id 3 :category_id 3 :desc "baz"}])
+
+ ;; Create a new in the Database table for this newly created temp database
+ (tt/with-temp Database [db {:engine :h2
+ :name "foo"
+ :is_full_sync true
+ :details "{\"db\": \"mem:temp\"}"}]
+
+ ;; Sync the database so we have the new table and it's fields
+ (do (sync/sync-database! db)
+ (let [table-id (db/select-one-field :id Table :db_id (u/get-id db) :name "FOO")
+ field-id (db/select-one-field :id Field :table_id table-id :name "CATEGORY_ID")
+ field-values-id (db/select-one-field :id FieldValues :field_id field-id)]
+ ;; Add in human readable values for remapping
+ (db/update! FieldValues field-values-id {:human_readable_values "[\"a\",\"b\",\"c\"]"})
+
+ ;; This is the starting point, the original catgory ids and their remapped values
+ [(find-values field-values-id)
+ ;; There should be no changes to human_readable_values when resync'd
+ (sync-and-find-values db field-values-id)
+ (do
+ ;; Add new rows that will have new field values
+ (jdbc/insert-multi! conn :foo [{:id 4 :category_id -2 :desc "foo"}
+ {:id 5 :category_id -1 :desc "bar"}
+ {:id 6 :category_id 0 :desc "baz"}])
+ ;; Sync to pickup the new field values and rebuild the human_readable_values
+ (sync-and-find-values db field-values-id))
+ ;; Resyncing this (with the new field values) should result in the same human_readable_values
+ (sync-and-find-values db field-values-id)
+ ;; Test that field values can be removed and the corresponding human_readable_values are removed as well
+ (do (jdbc/delete! conn :foo ["id in (?,?,?)" 1 2 3])
+ (sync-and-find-values db field-values-id))]))))))