diff --git a/src/metabase/sync_database/analyze.clj b/src/metabase/sync_database/analyze.clj
index ccb6496bbf71a3cd699b431baf86e8c63b978984..afe2c2206649ea5c337cdbfa2445281c44d62dd5 100644
--- a/src/metabase/sync_database/analyze.clj
+++ b/src/metabase/sync_database/analyze.clj
@@ -123,8 +123,8 @@
"Mark FIELD as `:json` if it's textual, doesn't already have a special type, the majority of it's values are non-nil, and all of its non-nil values
are valid serialized JSON dictionaries or arrays."
[driver field field-stats]
- (if-not (and (not (:special_type field))
- (not (isa? (:base_type field) :type/Text)))
+ (if (or (:special_type field)
+ (not (isa? (:base_type field) :type/Text)))
;; this field isn't suited for this test
field-stats
;; check for json values
@@ -134,14 +134,8 @@
(log/debug (u/format-color 'green "Field '%s' looks like it contains valid JSON objects. Setting special_type to :type/JSON." (field/qualified-name field)))
(assoc field-stats :special-type :type/JSON, :preview-display false)))))
-(defn valid-email?
- [email]
- (let [pattern #"[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?"]
- (and (string? email) (re-matches pattern email))))
-
-
(defn- values-are-valid-emails?
- "`true` if at every item in VALUES is `nil` or a valid string-encoded JSON dictionary or array, and at least one of those is non-nil."
+ "`true` if at every item in VALUES is `nil` or a valid email, and at least one of those is non-nil."
[values]
(try
(loop [at-least-one-non-nil-value? false, [val & more] values]
@@ -151,25 +145,25 @@
(s/blank? val) (recur at-least-one-non-nil-value? more)
;; If val is non-nil, check that it's a JSON dictionary or array. We don't want to mark Fields containing other
;; types of valid JSON values as :json (e.g. a string representation of a number or boolean)
- :else (do (assert (valid-email? val))
+ :else (do (assert (u/is-email? val))
(recur true more))))
(catch Throwable _
false)))
(defn- test:email-special-type
- "Mark FIELD as `:json` if it's textual, doesn't already have a special type, the majority of it's values are non-nil, and all of its non-nil values
- are valid serialized JSON dictionaries or arrays."
+ "Mark FIELD as `:email` if it's textual, doesn't already have a special type, the majority of it's values are non-nil, and all of its non-nil values
+ are valid emails."
[driver field field-stats]
- (if-not (and (not (:special_type field))
- (not (isa? (:base_type field) :type/Text)))
+ (if (or (:special_type field)
+ (not (isa? (:base_type field) :type/Text)))
;; this field isn't suited for this test
field-stats
- ;; check for json values
+ ;; check for emails
(if-not (values-are-valid-emails? (take driver/max-sync-lazy-seq-results (driver/field-values-lazy-seq driver field)))
field-stats
(do
(log/debug (u/format-color 'green "Field '%s' looks like it contains valid email addresses. Setting special_type to :type/Email." (field/qualified-name field)))
- (assoc field-stats :special-type :type/Email, :preview-display false)))))
+ (assoc field-stats :special-type :type/Email, :preview-display true)))))
@@ -180,7 +174,8 @@
(->> field-stats
(test:no-preview-display driver field)
(test:url-special-type driver field)
- (test:json-special-type driver field)))
+ (test:json-special-type driver field)
+ (test:email-special-type driver field)))
(defn make-analyze-table
"Make a generic implementation of `analyze-table`."
diff --git a/test/metabase/sync_database/analyze_test.clj b/test/metabase/sync_database/analyze_test.clj
index 62e602083d1c00adfb55899a86b7607a59a9376a..44613039d526970af4f0bed089fa4da0a63e28e7 100644
--- a/test/metabase/sync_database/analyze_test.clj
+++ b/test/metabase/sync_database/analyze_test.clj
@@ -23,6 +23,7 @@
;;; ## mark-json-field!
(tu/resolve-private-vars metabase.sync-database.analyze values-are-valid-json?)
+(tu/resolve-private-vars metabase.sync-database.analyze values-are-valid-emails?)
(def ^:const ^:private fake-values-seq-json
"A sequence of values that should be marked is valid JSON.")
@@ -62,10 +63,12 @@
(expect false (values-are-valid-json? ["false"]))
;; Check that things that are valid emails are marked as Emails
-(expect true (values-are-valid-email? ["helper@metabase.com"]))
-(expect true (values-are-valid-email? ["helper@metabase.com", "someone@here.com", "help@nope.com"]))
+(expect true (values-are-valid-emails? ["helper@metabase.com"]))
+(expect true (values-are-valid-emails? ["helper@metabase.com", "someone@here.com", "help@nope.com"]))
+(expect true (values-are-valid-emails? ["helper@metabase.com", nil, "help@nope.com"]))
-(expect false (values-are-valid-email? ["\"A string should not cause a Field to be marked as email\""]))
-(expect false (values-are-valid-email? [100]))
-(expect false (values-are-valid-email? ["true"]))
-(expect false (values-are-valid-email? ["false"]))
+(expect false (values-are-valid-emails? ["helper@metabase.com", "1111IsNot!An....email", "help@nope.com"]))
+(expect false (values-are-valid-emails? ["\"A string should not cause a Field to be marked as email\""]))
+(expect false (values-are-valid-emails? [100]))
+(expect false (values-are-valid-emails? ["true"]))
+(expect false (values-are-valid-emails? ["false"]))