Merge pull request #1478 from metabase/no_field_values_on_datetimes

Exclude the creation of FieldValues on temporal column types

src/metabase/models/field_values.clj

@@ -33,6 +33,7 @@
          (contains? field :base_type)
          (contains? field :special_type)]}
   (and (not= (keyword field_type) :sensitive)
+       (not (contains? #{:DateField :DateTimeField :TimeField} (keyword base_type)))
        (or (contains? #{:category :city :state :country} (keyword special_type))
            (= (keyword base_type) :BooleanField))))
 

test/metabase/api/table_test.clj

@@ -292,10 +292,7 @@
        :active          true
        :id              (id :users)
        :db_id           (id)
-       :field_values    {(keyword (str (id :users :last_login)))
-                         user-last-login-date-strs
-
-                         (keyword (str (id :users :name)))
+       :field_values    {(keyword (str (id :users :name)))
                          ["Broen Olujimi"
                           "Conchúr Tihomir"
                           "Dwight Gresham"
@@ -390,10 +387,7 @@
        :active          true
        :id              (id :users)
        :db_id           (id)
-       :field_values    {(keyword (str (id :users :last_login)))
-                         user-last-login-date-strs
-
-                         (keyword (str (id :users :name)))
+       :field_values    {(keyword (str (id :users :name)))
                          ["Broen Olujimi"
                           "Conchúr Tihomir"
                           "Dwight Gresham"

test/metabase/models/field_test.clj

@@ -5,6 +5,60 @@
                              [field-values :refer :all])
             [metabase.test.data :refer :all]))
 
+
+;; field-should-have-field-values?
+
+;; sensitive fields should always be excluded
+(expect false (field-should-have-field-values? {:base_type    :BooleanField
+                                                :special_type :category
+                                                :field_type   :sensitive}))
+;; date/time based fields should always be excluded
+(expect false (field-should-have-field-values? {:base_type    :DateField
+                                                :special_type :category
+                                                :field_type   :dimension}))
+(expect false (field-should-have-field-values? {:base_type    :DateTimeField
+                                                :special_type :category
+                                                :field_type   :dimension}))
+(expect false (field-should-have-field-values? {:base_type    :TimeField
+                                                :special_type :category
+                                                :field_type   :dimension}))
+;; most special types should be excluded
+(expect false (field-should-have-field-values? {:base_type    :CharField
+                                                :special_type :image
+                                                :field_type   :dimension}))
+(expect false (field-should-have-field-values? {:base_type    :CharField
+                                                :special_type :id
+                                                :field_type   :dimension}))
+(expect false (field-should-have-field-values? {:base_type    :CharField
+                                                :special_type :fk
+                                                :field_type   :dimension}))
+(expect false (field-should-have-field-values? {:base_type    :CharField
+                                                :special_type :latitude
+                                                :field_type   :dimension}))
+(expect false (field-should-have-field-values? {:base_type    :CharField
+                                                :special_type :number
+                                                :field_type   :dimension}))
+(expect false (field-should-have-field-values? {:base_type    :CharField
+                                                :special_type :timestamp_milliseconds
+                                                :field_type   :dimension}))
+;; boolean fields + category/city/state/country fields are g2g
+(expect true (field-should-have-field-values? {:base_type    :BooleanField
+                                               :special_type :number
+                                               :field_type   :dimension}))
+(expect true (field-should-have-field-values? {:base_type    :CharField
+                                               :special_type :category
+                                               :field_type   :dimension}))
+(expect true (field-should-have-field-values? {:base_type    :TextField
+                                               :special_type :city
+                                               :field_type   :dimension}))
+(expect true (field-should-have-field-values? {:base_type    :TextField
+                                               :special_type :state
+                                               :field_type   :dimension}))
+(expect true (field-should-have-field-values? {:base_type    :TextField
+                                               :special_type :country
+                                               :field_type   :dimension}))
+
+
 ;; Check that setting a Field's special_type to :category will cause a corresponding FieldValues to be created asynchronously
 (expect
     [nil