diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj index 313df9940f3567c0927925d0d60d3cea19687da1..1bd37786a10a6e98ef86f98cb1035d1c4925fc4b 100644 --- a/src/metabase/api/field.clj +++ b/src/metabase/api/field.clj @@ -39,13 +39,13 @@ (defendpoint PUT "/:id" "Update `Field` with ID." - [id :as {{:keys [special_type visibility_type description display_name]} :body}] + [id :as {{:keys [special_type visibility_type description display_name], :as body} :body}] {special_type FieldSpecialType visibility_type FieldVisibilityType display_name NonEmptyString} (let-404 [field (Field id)] (write-check field) - (let [special_type (or special_type (:special_type field)) + (let [special_type (if (contains? body :special_type) special_type (:special_type field)) visibility_type (or visibility_type (:visibility_type field))] (check-400 (field/valid-metadata? (:base_type field) (:field_type field) special_type visibility_type)) ;; update the Field. start with keys that may be set to NULL then conditionally add other keys if they have values diff --git a/test/metabase/api/field_test.clj b/test/metabase/api/field_test.clj index 0573be30f6230000e7ce9141c8f4e258b1b6cb4b..67d0262e793539ba86b2d37e7fde99daaedf135e 100644 --- a/test/metabase/api/field_test.clj +++ b/test/metabase/api/field_test.clj @@ -68,31 +68,57 @@ ;; ## 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 -(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 - (db/upd Field (id :venues :latitude) :special_type "latitude") - ;; match against the modified Field - field) - {:description nil - :table_id (id :venues) - :special_type "fk" - :name "LATITUDE" - :display_name "Latitude" - :updated_at $ - :active true - :id $ - :field_type "info" - :visibility_type "normal" - :position 0 - :preview_display true - :created_at $ - :base_type "FloatField" - :parent_id nil}) - ((user->client :crowberto) :put 200 (format "field/%d" (id :venues :latitude)) {:special_type :fk})) +(defn simple-field-details [field] + (select-keys field [:name :display_name :description :visibility_type :special_type])) +;; test that we can do basic field update work, including unsetting some fields such as special-type +(expect + [{:name "Field Test" + :display_name "Field test" + :description nil + :special_type nil + :visibility_type :normal} + {:name "Field Test" + :display_name "yay" + :description "foobar" + :special_type :name + :visibility_type :sensitive} + {:name "Field Test" + :display_name "yay" + :description nil + :special_type nil + :visibility_type :sensitive}] + (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 + :special_type nil + :active true + :preview_display true + :position 1}] + (let [original-val (simple-field-details (db/sel :one Field :id field-id))] + ;; set it + ((user->client :crowberto) :put 200 (format "field/%d" field-id) {:name "something else" + :display_name "yay" + :description "foobar" + :special_type :name + :visibility_type :sensitive}) + (let [updated-val (simple-field-details (db/sel :one Field :id field-id))] + ;; unset it + ((user->client :crowberto) :put 200 (format "field/%d" field-id) {:description nil + :special_type nil}) + [original-val + updated-val + (simple-field-details (db/sel :one Field :id field-id))])))))) + +;; check that you can't set a field to :timestamp_seconds if it's not of a proper base_type (expect ["Invalid Request." nil]