diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj
index 5518d2b48c11acd8d755b7617921349d75ea3bf4..3f5bcfd03e1884b292420586103e0e0520084dcf 100644
--- a/src/metabase/api/field.clj
+++ b/src/metabase/api/field.clj
@@ -39,14 +39,18 @@
{field_type FieldType
special_type FieldSpecialType
display_name NonEmptyString}
- (write-check Field id)
- ;; update the Field. start with keys that may be set to NULL then conditionally add other keys if they have values
- (check-500 (m/mapply upd Field id (merge {:description description
- :special_type special_type}
- (when display_name {:display_name display_name})
- (when field_type {:field_type field_type})
- (when-not (nil? preview_display) {:preview_display preview_display}))))
- (Field id))
+ (let-404 [field (Field id)]
+ (write-check field)
+ (let [field_type (or field_type (:field_type field))
+ special_type (or special_type (:special_type field))]
+ (check-400 (field/valid-metadata? (:base_type field) field_type special_type))
+ ;; update the Field. start with keys that may be set to NULL then conditionally add other keys if they have values
+ (check-500 (m/mapply upd Field id (merge {:description description
+ :field_type field_type
+ :special_type special_type}
+ (when display_name {:display_name display_name})
+ (when-not (nil? preview_display) {:preview_display preview_display}))))
+ (Field id))))
(defendpoint GET "/:id/summary"
"Get the count and distinct count of `Field` with ID."
diff --git a/test/metabase/api/field_test.clj b/test/metabase/api/field_test.clj
index 2791be1ff9181e27e48439f3235e3b04cf3f42a5..bd1c93e085590b9634a07ecf41a67938b2c96357 100644
--- a/test/metabase/api/field_test.clj
+++ b/test/metabase/api/field_test.clj
@@ -1,17 +1,18 @@
(ns metabase.api.field-test
(:require [expectations :refer :all]
- [metabase.db :refer :all]
+ [metabase.db :as db]
+ [metabase.models.database :refer [Database]]
(metabase.models [field :refer [Field]]
[field-values :refer [FieldValues]]
[table :refer [Table]])
[metabase.test.data :refer :all]
[metabase.test.data.users :refer :all]
- [metabase.test.util :refer [match-$ expect-eval-actual-first]]))
+ [metabase.test.util :as tu]))
;; Helper Fns
(defn- db-details []
- (match-$ (db)
+ (tu/match-$ (db)
{:created_at $
:engine "h2"
:id $
@@ -26,10 +27,10 @@
;; ## GET /api/field/:id
(expect
- (match-$ (Field (id :users :name))
+ (tu/match-$ (Field (id :users :name))
{:description nil
:table_id (id :users)
- :table (match-$ (Table (id :users))
+ :table (tu/match-$ (Table (id :users))
{:description nil
:entity_type nil
:visibility_type nil
@@ -68,10 +69,10 @@
;; ## PUT /api/field/:id
;; Check that we can update a Field
;; TODO - this should NOT be modifying a field from our test data, we should create new data to mess with
-(expect-eval-actual-first
- (match-$ (let [field (Field (id :venues :latitude))]
+(tu/expect-eval-actual-first
+ (tu/match-$ (let [field (Field (id :venues :latitude))]
;; this is sketchy. But return the Field back to its unmodified state so it won't affect other unit tests
- (upd Field (id :venues :latitude) :special_type "latitude")
+ (db/upd Field (id :venues :latitude) :special_type "latitude")
;; match against the modified Field
field)
{:description nil
@@ -90,22 +91,43 @@
:parent_id nil})
((user->client :crowberto) :put 200 (format "field/%d" (id :venues :latitude)) {:special_type :fk}))
+(expect
+ ["Invalid Request."
+ nil]
+ (tu/with-temp Database [{database-id :id} {:name "Field Test"
+ :engine :yeehaw
+ :details {}
+ :is_sample false}]
+ (tu/with-temp Table [{table-id :id} {:name "Field Test"
+ :db_id database-id
+ :active true}]
+ (tu/with-temp Field [{field-id :id} {:table_id table-id
+ :name "Field Test"
+ :base_type :TextField
+ :field_type :info
+ :active true
+ :preview_display true
+ :position 1}]
+ [((user->client :crowberto) :put 400 (format "field/%d" field-id) {:special_type :timestamp_seconds})
+ (db/sel :one :field [Field :special_type] :id field-id)]))))
+
+
(defn- field->field-values
"Fetch the `FieldValues` object that corresponds to a given `Field`."
[table-kw field-kw]
- (sel :one FieldValues :field_id (id table-kw field-kw)))
+ (db/sel :one FieldValues :field_id (id table-kw field-kw)))
;; ## GET /api/field/:id/values
;; Should return something useful for a field that has special_type :category
-(expect-eval-actual-first
- (match-$ (field->field-values :venues :price)
+(tu/expect-eval-actual-first
+ (tu/match-$ (field->field-values :venues :price)
{:field_id (id :venues :price)
:human_readable_values {}
:values [1 2 3 4]
:updated_at $
:created_at $
:id $})
- (do (upd FieldValues (:id (field->field-values :venues :price)) :human_readable_values nil) ; clear out existing human_readable_values in case they're set
+ (do (db/upd FieldValues (:id (field->field-values :venues :price)) :human_readable_values nil) ; clear out existing human_readable_values in case they're set
((user->client :rasta) :get 200 (format "field/%d/values" (id :venues :price)))))
;; Should return nothing for a field whose special_type is *not* :category
@@ -118,9 +140,9 @@
;; ## POST /api/field/:id/value_map_update
;; Check that we can set values
-(expect-eval-actual-first
+(tu/expect-eval-actual-first
[{:status "success"}
- (match-$ (sel :one FieldValues :field_id (id :venues :price))
+ (tu/match-$ (db/sel :one FieldValues :field_id (id :venues :price))
{:field_id (id :venues :price)
:human_readable_values {:1 "$"
:2 "$$"
@@ -137,16 +159,16 @@
((user->client :rasta) :get 200 (format "field/%d/values" (id :venues :price)))])
;; Check that we can unset values
-(expect-eval-actual-first
+(tu/expect-eval-actual-first
[{:status "success"}
- (match-$ (sel :one FieldValues :field_id (id :venues :price))
+ (tu/match-$ (db/sel :one FieldValues :field_id (id :venues :price))
{:field_id (id :venues :price)
:human_readable_values {}
:values [1 2 3 4]
:updated_at $
:created_at $
:id $})]
- [(do (upd FieldValues (:id (field->field-values :venues :price)) :human_readable_values {:1 "$" ; make sure they're set
+ [(do (db/upd FieldValues (:id (field->field-values :venues :price)) :human_readable_values {:1 "$" ; make sure they're set
:2 "$$"
:3 "$$$"
:4 "$$$$"})