Skip to content
Snippets Groups Projects
Unverified Commit 0b469bca authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

[Toucan 2 Prep] Convert `IRevisioned` to multimethods (#25108)

parent 033f47da
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 182 additions and 191 deletions
.dir-locals.el
+ 1
− 1
View file @ 0b469bca
......@@ -55,7 +55,7 @@
(eval . (put-clojure-indent 'u/prog1 1))
(eval . (put-clojure-indent 'u/select-keys-when 1))
(eval . (put-clojure-indent 'u/strict-extend 1))
(eval . (put-clojure-indent 'with-meta '(:defn)))
(eval . (put-clojure-indent 'with-meta '(:form)))
;; these ones have to be done with `define-clojure-indent' for now because of upstream bug
;; https://github.com/clojure-emacs/clojure-mode/issues/600 once that's resolved we should use `put-clojure-indent'
;; instead. Please don't add new entries unless they don't work with `put-clojure-indent'
......
src/metabase/models/card.clj
+ 10
− 15
View file @ 0b469bca
......@@ -70,11 +70,10 @@
;;; --------------------------------------------------- Revisions ----------------------------------------------------
(defn serialize-instance
"Serialize a `Card` for use in a `Revision`."
(defmethod revision/serialize-instance Card
([instance]
(serialize-instance nil nil instance))
([_ _ instance]
(revision/serialize-instance Card nil instance))
([_model _id instance]
(cond-> (dissoc instance :created_at :updated_at)
;; datasets should preserve edits to metadata
(not (:dataset instance))
......@@ -327,23 +326,19 @@
:pre-delete pre-delete
:post-select public-settings/remove-public-uuid-if-public-sharing-is-disabled})
revision/IRevisioned
(assoc revision/IRevisionedDefaults
:serialize-instance serialize-instance)
serdes.hash/IdentityHashable
{:identity-hash-fields (constantly [:name (serdes.hash/hydrated-hash :collection)])})
;;; ------------------------------------------------- Serialization --------------------------------------------------
(defmethod serdes.base/extract-query "Card" [_ {:keys [user]}]
(serdes.base/raw-reducible-query
"Card"
{:select [:card.*]
:from [[:report_card :card]]
:left-join [[:collection :coll] [:= :coll.id :card.collection_id]]
:where (if user
[:or [:= :coll.personal_owner_id user] [:is :coll.personal_owner_id nil]]
[:is :coll.personal_owner_id nil])}))
"Card"
{:select [:card.*]
:from [[:report_card :card]]
:left-join [[:collection :coll] [:= :coll.id :card.collection_id]]
:where (if user
[:or [:= :coll.personal_owner_id user] [:is :coll.personal_owner_id nil]]
[:is :coll.personal_owner_id nil])}))
(defmethod serdes.base/extract-one "Card"
[_model-name _opts card]
......
src/metabase/models/dashboard.clj
+ 6
− 16
View file @ 0b469bca
......@@ -149,18 +149,16 @@
;;; --------------------------------------------------- Revisions ----------------------------------------------------
(defn serialize-dashboard
"Serialize a Dashboard for use in a Revision."
[dashboard]
(defmethod revision/serialize-instance Dashboard
[_model _id dashboard]
(-> dashboard
(select-keys [:description :name :cache_ttl])
(assoc :cards (vec (for [dashboard-card (ordered-cards dashboard)]
(-> (select-keys dashboard-card [:size_x :size_y :row :col :id :card_id])
(assoc :series (mapv :id (dashboard-card/series dashboard-card)))))))))
(defn- revert-dashboard!
"Revert a Dashboard to the state defined by `serialized-dashboard`."
[_ dashboard-id user-id serialized-dashboard]
(defmethod revision/revert-to-revision! Dashboard
[_model dashboard-id user-id serialized-dashboard]
;; Update the dashboard description / name / permissions
(db/update! Dashboard dashboard-id, (dissoc serialized-dashboard :cards))
;; Now update the cards as needed
......@@ -188,9 +186,8 @@
serialized-dashboard)
(defn- diff-dashboards-str
"Describe the difference between two Dashboard instances."
[_ dashboard1 dashboard2]
(defmethod revision/diff-str Dashboard
[_model dashboard1 dashboard2]
(let [[removals changes] (diff dashboard1 dashboard2)
check-series-change (fn [idx card-changes]
(when (and (:series card-changes)
......@@ -231,13 +228,6 @@
(->> (filter identity)
build-sentence))))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Dashboard)
revision/IRevisioned
(merge revision/IRevisionedDefaults
{:serialize-instance (fn [_ _ dashboard] (serialize-dashboard dashboard))
:revert-to-revision! revert-dashboard!
:diff-str diff-dashboards-str}))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | OTHER CRUD FNS |
......
src/metabase/models/interface.clj
+ 5
− 1
View file @ 0b469bca
......@@ -258,7 +258,9 @@
;;; | New Permissions Stuff |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- dispatch-on-model [x & _args]
(defn dispatch-on-model
"Helper dispatch function for multimethods. Dispatches on the first arg, using [[models.dispatch/model]]."
[x & _args]
(models.dispatch/model x))
(defmulti perms-objects-set
......@@ -435,6 +437,8 @@
;;; swap out [[models/defmodel]] with a special magical version that avoids redefining stuff if the definition has not
;;; changed at all. This is important to make the stuff in [[models.dispatch]] work properly, since we're dispatching
;;; off of the model objects themselves e.g. [[metabase.models.user/User]] -- it is important that they do not change
;;;
;;; This code is temporary until the switch to Toucan 2.
(defonce ^:private original-defmodel @(resolve `models/defmodel))
......
src/metabase/models/metric.clj
+ 11
− 14
View file @ 0b469bca
......@@ -52,30 +52,27 @@
;;; --------------------------------------------------- REVISIONS ----------------------------------------------------
(defn- serialize-metric [_ _ instance]
(defmethod revision/serialize-instance Metric
[_model _id instance]
(dissoc instance :created_at :updated_at))
(defn- diff-metrics [this metric1 metric2]
(defmethod revision/diff-map Metric
[model metric1 metric2]
(if-not metric1
;; this is the first version of the metric
;; model is the first version of the metric
(m/map-vals (fn [v] {:after v}) (select-keys metric2 [:name :description :definition]))
;; do our diff logic
(let [base-diff (revision/default-diff-map this
(select-keys metric1 [:name :description :definition])
(select-keys metric2 [:name :description :definition]))]
(let [base-diff ((get-method revision/diff-map :default)
model
(select-keys metric1 [:name :description :definition])
(select-keys metric2 [:name :description :definition]))]
(cond-> (merge-with merge
(m/map-vals (fn [v] {:after v}) (:after base-diff))
(m/map-vals (fn [v] {:before v}) (:before base-diff)))
(m/map-vals (fn [v] {:after v}) (:after base-diff))
(m/map-vals (fn [v] {:before v}) (:before base-diff)))
(or (get-in base-diff [:after :definition])
(get-in base-diff [:before :definition])) (assoc :definition {:before (get-in metric1 [:definition])
:after (get-in metric2 [:definition])})))))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Metric)
revision/IRevisioned
(merge revision/IRevisionedDefaults
{:serialize-instance serialize-metric
:diff-map diff-metrics}))
;;; ------------------------------------------------- SERIALIZATION --------------------------------------------------
......
src/metabase/models/revision.clj
+ 44
− 52
View file @ 0b469bca
(ns metabase.models.revision
(:require [clojure.data :as data]
[metabase.models.interface :as mi]
[metabase.models.revision.diff :refer [diff-string]]
[metabase.models.user :refer [User]]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
[potemkin.types :as p.types]
[toucan.db :as db]
[toucan.hydrate :refer [hydrate]]
[toucan.models :as models]))
......@@ -14,50 +14,43 @@
will be deleted."
15)
;;; # IRevisioned Protocl
(defmulti serialize-instance
"Prepare an instance for serialization in a Revision."
{:arglists '([model id instance])}
mi/dispatch-on-model)
(p.types/defprotocol+ IRevisioned
"Methods an entity may optionally implement to control how revisions of an instance are saved and reverted to.
All of these methods except for `serialize-instance` have a default implementation in `IRevisionedDefaults`."
(serialize-instance [this id instance]
"Prepare an instance for serialization in a Revision.")
(revert-to-revision! [this id user-id serialized-instance]
"Return an object to the state recorded by `serialized-INSTANCE`.")
(diff-map [this object-1 object-2]
"Return a map describing the difference between `object-1` and `object-2`.")
(diff-str [this object-1 object-2]
"Return a string describing the difference between `object-1` and `object-2`."))
;;; no default implementation for [[serialize-instance]]; models need to implement this themselves.
(defmulti revert-to-revision!
"Return an object to the state recorded by `serialized-instance`."
{:arglists '([model id user-id serialized-instance])}
mi/dispatch-on-model)
;;; # Reusable Base Implementations for IRevisioned functions
(defmethod revert-to-revision! :default
[model id _user-id serialized-instance]
(db/update! model id, serialized-instance))
;; NOTE that we do not provide a base implementation for `serialize-instance`, that should be done per entity.
(defmulti diff-map
"Return a map describing the difference between `object-1` and `object-2`."
{:arglists '([model object-1 object-2])}
mi/dispatch-on-model)
(defn default-revert-to-revision!
"Default implementation of `revert-to-revision!` which simply does an update using the values from `serialized-instance`."
[entity id _user-id serialized-instance]
(db/update! entity id, serialized-instance))
(defn default-diff-map
"Default implementation of `diff-map` which simply uses clojures `data/diff` function and sets the keys `:before` and `:after`."
[_ o1 o2]
(defmethod diff-map :default
[_model o1 o2]
(when o1
(let [[before after] (data/diff o1 o2)]
{:before before
:after after})))
(defn default-diff-str
"Default implementation of `diff-str` which simply uses clojures `data/diff` function and passes that on to `diff-string`."
[entity o1 o2]
(when-let [[before after] (data/diff o1 o2)]
(diff-string (:name entity) before after)))
(def IRevisionedDefaults
"Default implementations for `IRevisioned`."
{:revert-to-revision! default-revert-to-revision!
:diff-map default-diff-map
:diff-str default-diff-str})
(defmulti diff-str
"Return a string describing the difference between `object-1` and `object-2`."
{:arglists '([model object-1 object-2])}
mi/dispatch-on-model)
(defmethod diff-str :default
[model o1 o2]
(when-let [[before after] (data/diff o1 o2)]
(diff-string (:name model) before after)))
;;; # Revision Entity
......@@ -90,10 +83,10 @@
(defn add-revision-details
"Add enriched revision data such as `:diff` and `:description` as well as filter out some unnecessary props."
[entity revision prev-revision]
[model revision prev-revision]
(-> revision
(assoc :diff (diff-map entity (:object prev-revision) (:object revision))
:description (diff-str entity (:object prev-revision) (:object revision)))
(assoc :diff (diff-map model (:object prev-revision) (:object revision))
:description (diff-str model (:object prev-revision) (:object revision)))
;; add revision user details
(hydrate :user)
(update :user select-keys [:id :first_name :last_name :common_name])
......@@ -101,34 +94,34 @@
(dissoc :model :model_id :user_id :object)))
(defn revisions
"Get the revisions for `entity` with `id` in reverse chronological order."
[entity id]
{:pre [(models/model? entity) (integer? id)]}
(db/select Revision, :model (:name entity), :model_id id, {:order-by [[:id :desc]]}))
"Get the revisions for `model` with `id` in reverse chronological order."
[model id]
{:pre [(models/model? model) (integer? id)]}
(db/select Revision, :model (:name model), :model_id id, {:order-by [[:id :desc]]}))
(defn revisions+details
"Fetch `revisions` for `entity` with `id` and add details."
[entity id]
(when-let [revisions (revisions entity id)]
"Fetch `revisions` for `model` with `id` and add details."
[model id]
(when-let [revisions (revisions model id)]
(loop [acc [], [r1 r2 & more] revisions]
(if-not r2
(conj acc (add-revision-details entity r1 nil))
(recur (conj acc (add-revision-details entity r1 r2))
(conj acc (add-revision-details model r1 nil))
(recur (conj acc (add-revision-details model r1 r2))
(conj more r2))))))
(defn- delete-old-revisions!
"Delete old revisions of `entity` with `id` when there are more than `max-revisions` in the DB."
[entity id]
{:pre [(models/model? entity) (integer? id)]}
"Delete old revisions of `model` with `id` when there are more than `max-revisions` in the DB."
[model id]
{:pre [(models/model? model) (integer? id)]}
(when-let [old-revisions (seq (drop max-revisions (map :id (db/select [Revision :id]
:model (:name entity)
:model (:name model)
:model_id id
{:order-by [[:timestamp :desc]]}))))]
(db/delete! Revision :id [:in old-revisions])))
(defn push-revision!
"Record a new Revision for `entity` with `id`. Returns `object`."
{:arglists '([& {:keys [object entity id user-id is-creation? message]}]), :style/indent 0}
{:arglists '([& {:keys [object entity id user-id is-creation? message]}])}
[& {object :object,
:keys [entity id user-id is-creation? message],
:or {id (:id object), is-creation? false}}]
......@@ -155,7 +148,6 @@
(defn revert!
"Revert `entity` with `id` to a given Revision."
{:style/indent 0}
[& {:keys [entity id user-id revision-id]}]
{:pre [(models/model? entity)
(integer? id)
......
src/metabase/models/segment.clj
+ 11
− 15
View file @ 0b469bca
......@@ -52,32 +52,28 @@
;;; --------------------------------------------------- Revisions ----------------------------------------------------
(defn- serialize-segment [_ _ instance]
(defmethod revision/serialize-instance Segment
[_model _id instance]
(dissoc instance :created_at :updated_at))
(defn- diff-segments [this segment1 segment2]
(defmethod revision/diff-map Segment
[model segment1 segment2]
(if-not segment1
;; this is the first version of the segment
(m/map-vals (fn [v] {:after v}) (select-keys segment2 [:name :description :definition]))
;; do our diff logic
(let [base-diff (revision/default-diff-map this
(select-keys segment1 [:name :description :definition])
(select-keys segment2 [:name :description :definition]))]
(let [base-diff ((get-method revision/diff-map :default)
model
(select-keys segment1 [:name :description :definition])
(select-keys segment2 [:name :description :definition]))]
(cond-> (merge-with merge
(m/map-vals (fn [v] {:after v}) (:after base-diff))
(m/map-vals (fn [v] {:before v}) (:before base-diff)))
(or (get-in base-diff [:after :definition])
(get-in base-diff [:before :definition])) (assoc :definition {:before (get-in segment1 [:definition])
:after (get-in segment2 [:definition])})))))
(or (get-in base-diff [:after :definition])
(get-in base-diff [:before :definition])) (assoc :definition {:before (get-in segment1 [:definition])
:after (get-in segment2 [:definition])})))))
(u/strict-extend #_{:clj-kondo/ignore [:metabase/disallow-class-or-type-on-model]} (class Segment)
revision/IRevisioned
(merge
revision/IRevisionedDefaults
{:serialize-instance serialize-segment
:diff-map diff-segments}))
;;; ------------------------------------------------ Serialization ---------------------------------------------------
(defmethod serdes.base/serdes-generate-path "Segment"
......
test/metabase/api/revision_test.clj
+ 3
− 3
View file @ 0b469bca
(ns metabase.api.revision-test
(:require [clojure.test :refer :all]
[metabase.models.card :refer [Card serialize-instance]]
[metabase.models.card :refer [Card]]
[metabase.models.collection :refer [Collection]]
[metabase.models.dashboard :refer [Dashboard]]
[metabase.models.dashboard-card :refer [DashboardCard]]
[metabase.models.revision :refer [push-revision! Revision revisions]]
[metabase.models.revision :as revision :refer [push-revision! Revision revisions]]
[metabase.test :as mt]
[metabase.test.data.users :as test.users]
[metabase.test.fixtures :as fixtures]
......@@ -96,7 +96,7 @@
:model (:name Card)
:model_id id
:user_id (test.users/user->id :rasta)
:object (serialize-instance Card (:id card) card)
:object (revision/serialize-instance Card (:id card) card)
:message "because i wanted to"
:is_creation false
:is_reversion true)
......
test/metabase/models/dashboard_test.clj
+ 23
− 18
View file @ 0b469bca
......@@ -12,6 +12,7 @@
[metabase.models.permissions :as perms]
[metabase.models.pulse :refer [Pulse]]
[metabase.models.pulse-card :refer [PulseCard]]
[metabase.models.revision :as revision]
[metabase.models.serialization.hash :as serdes.hash]
[metabase.models.table :refer [Table]]
[metabase.models.user :as user]
......@@ -42,17 +43,19 @@
:id true
:card_id true
:series true}]}
(update (dashboard/serialize-dashboard dashboard) :cards (fn [[{:keys [id card_id series], :as card}]]
[(assoc card
:id (= dashcard-id id)
:card_id (= card-id card_id)
:series (= [series-id-1 series-id-2] series))]))))))
(update (revision/serialize-instance Dashboard (:id dashboard) dashboard)
:cards
(fn [[{:keys [id card_id series], :as card}]]
[(assoc card
:id (= dashcard-id id)
:card_id (= card-id card_id)
:series (= [series-id-1 series-id-2] series))]))))))
(deftest diff-dashboards-str-test
(is (= "renamed it from \"Diff Test\" to \"Diff Test Changed\" and added a description."
(#'dashboard/diff-dashboards-str
nil
(revision/diff-str
Dashboard
{:name "Diff Test"
:description nil
:cards []}
......@@ -61,8 +64,8 @@
:cards []})))
(is (= "added a card."
(#'dashboard/diff-dashboards-str
nil
(revision/diff-str
Dashboard
{:name "Diff Test"
:description nil
:cards []}
......@@ -77,8 +80,8 @@
:series []}]})))
(is (= "changed the cache ttl from \"333\" to \"1227\", rearranged the cards, modified the series on card 1 and added some series to card 2."
(#'dashboard/diff-dashboards-str
nil
(revision/diff-str
Dashboard
{:name "Diff Test"
:description nil
:cache_ttl 333
......@@ -131,7 +134,7 @@
:description "something"
:cache_ttl nil
:cards []}
serialized-dashboard (dashboard/serialize-dashboard dashboard)]
serialized-dashboard (revision/serialize-instance Dashboard (:id dashboard) dashboard)]
(testing "original state"
(is (= {:name "Test Dashboard"
:description nil
......@@ -150,10 +153,11 @@
:name "Revert Test"
:description "something")
(testing "capture updated Dashboard state"
(is (= empty-dashboard
(dashboard/serialize-dashboard (db/select-one Dashboard :id dashboard-id))))))
(let [dashboard (db/select-one Dashboard :id dashboard-id)]
(is (= empty-dashboard
(revision/serialize-instance Dashboard (:id dashboard) dashboard))))))
(testing "now do the reversion; state should return to original"
(#'dashboard/revert-dashboard! nil dashboard-id (test.users/user->id :crowberto) serialized-dashboard)
(revision/revert-to-revision! Dashboard dashboard-id (test.users/user->id :crowberto) serialized-dashboard)
(is (= {:name "Test Dashboard"
:description nil
:cache_ttl nil
......@@ -164,11 +168,12 @@
:id false
:card_id true
:series true}]}
(update (dashboard/serialize-dashboard (db/select-one Dashboard :id dashboard-id)) :cards check-ids))))
(update (revision/serialize-instance Dashboard dashboard-id (db/select-one Dashboard :id dashboard-id))
:cards check-ids))))
(testing "revert back to the empty state"
(#'dashboard/revert-dashboard! nil dashboard-id (test.users/user->id :crowberto) empty-dashboard)
(revision/revert-to-revision! Dashboard dashboard-id (test.users/user->id :crowberto) empty-dashboard)
(is (= empty-dashboard
(dashboard/serialize-dashboard (db/select-one Dashboard :id dashboard-id))))))))
(revision/serialize-instance Dashboard dashboard-id (db/select-one Dashboard :id dashboard-id))))))))
(deftest public-sharing-test
(testing "test that a Dashboard's :public_uuid comes back if public sharing is enabled..."
......
test/metabase/models/metric_test.clj
+ 25
− 24
View file @ 0b469bca
......@@ -2,6 +2,7 @@
(:require [clojure.test :refer :all]
[metabase.models.database :refer [Database]]
[metabase.models.metric :as metric :refer [Metric]]
[metabase.models.revision :as revision]
[metabase.models.serialization.hash :as serdes.hash]
[metabase.models.table :refer [Table]]
[metabase.test :as mt]
......@@ -80,7 +81,7 @@
:definition {:aggregation [[:count]]
:filter [:> [:field 4 nil] "2014-10-19"]}})
(into {}
(-> (#'metric/serialize-metric Metric (:id metric) metric)
(-> (revision/serialize-instance Metric (:id metric) metric)
(update :id boolean)
(update :table_id boolean))))))))
......@@ -96,42 +97,42 @@
:after "BBB"}
:name {:before "Toucans in the rainforest"
:after "Something else"}}
(#'metric/diff-metrics Metric metric (assoc metric
:name "Something else"
:description "BBB"
:definition {:filter [:between [:field 4 nil] "2014-07-01" "2014-10-19"]})))))
(revision/diff-map Metric metric (assoc metric
:name "Something else"
:description "BBB"
:definition {:filter [:between [:field 4 nil] "2014-07-01" "2014-10-19"]})))))
(testing "test case where definition doesn't change"
(is (= {:name {:before "A"
:after "B"}}
(#'metric/diff-metrics Metric
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}
{:name "B"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}))))
(revision/diff-map Metric
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}
{:name "B"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}))))
(testing "first version, so comparing against nil"
(is (= {:name {:after "A"}
:description {:after "Unchanged"}
:definition {:after {:filter [:and [:> 4 "2014-10-19"]]}}}
(#'metric/diff-metrics Metric
nil
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}))))
(revision/diff-map Metric
nil
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}))))
(testing "removals only"
(is (= {:definition {:before {:filter [:and [:> 4 "2014-10-19"] [:= 5 "yes"]]}
:after {:filter [:and [:> 4 "2014-10-19"]]}}}
(#'metric/diff-metrics Metric
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"] [:= 5 "yes"]]}}
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}))))))
(revision/diff-map Metric
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"] [:= 5 "yes"]]}}
{:name "A"
:description "Unchanged"
:definition {:filter [:and [:> 4 "2014-10-19"]]}}))))))
(deftest identity-hash-test
(testing "Metric hashes are composed of the metric name and table identity-hash"
......
test/metabase/models/revision_test.clj
+ 36
− 26
View file @ 0b469bca
......@@ -12,17 +12,22 @@
(models/defmodel ^:private FakedCard :report_card)
(extend-type #_{:clj-kondo/ignore [:unresolved-symbol]} FakedCardInstance
revision/IRevisioned
(serialize-instance [_ _ obj]
(into {} (assoc obj :serialized true)))
(revert-to-revision! [_ _ _ serialized-instance]
(reset! reverted-to (dissoc serialized-instance :serialized)))
(diff-map [_ o1 o2]
{:o1 (when o1 (into {} o1)), :o2 (when o2 (into {} o2))})
(diff-str [_ o1 o2]
(when o1
(str "BEFORE=" (into {} o1) ",AFTER=" (into {} o2)))))
(defmethod revision/serialize-instance FakedCard
[_model _id obj]
(into {} (assoc obj :serialized true)))
(defmethod revision/revert-to-revision! FakedCard
[_model _id _user-id serialized-instance]
(reset! reverted-to (dissoc serialized-instance :serialized)))
(defmethod revision/diff-map FakedCard
[_model o1 o2]
{:o1 (when o1 (into {} o1)), :o2 (when o2 (into {} o2))})
(defmethod revision/diff-str FakedCard
[_model o1 o2]
(when o1
(str "BEFORE=" (into {} o1) ",AFTER=" (into {} o2))))
(defn- push-fake-revision! [card-id & {:keys [message] :as object}]
(revision/push-revision!
......@@ -46,35 +51,40 @@
(testing (str "Check that pattern matching allows specialization and that string only reflects the keys that have "
"changed")
(is (= "renamed this Card from \"Tips by State\" to \"Spots by State\"."
(revision/default-diff-str Card
{:name "Tips by State", :private false}
{:name "Spots by State", :private false})))
((get-method revision/diff-str :default)
Card
{:name "Tips by State", :private false}
{:name "Spots by State", :private false})))
(is (= "made this Card private."
(revision/default-diff-str Card
{:name "Spots by State", :private false}
{:name "Spots by State", :private true})))))
((get-method revision/diff-str :default)
Card
{:name "Spots by State", :private false}
{:name "Spots by State", :private true})))))
(deftest fallback-description-test
(testing "Check the fallback sentence fragment for key without specialized sentence fragment"
(is (= "changed priority from \"Important\" to \"Regular\"."
(revision/default-diff-str Card
{:priority "Important"}
{:priority "Regular"})))))
((get-method revision/diff-str :default)
Card
{:priority "Important"}
{:priority "Regular"})))))
(deftest multiple-changes-test
(testing "Check that 2 changes are handled nicely"
(is (= "made this Card private and renamed it from \"Tips by State\" to \"Spots by State\"."
(revision/default-diff-str Card
{:name "Tips by State", :private false}
{:name "Spots by State", :private true}))))
((get-method revision/diff-str :default)
Card
{:name "Tips by State", :private false}
{:name "Spots by State", :private true}))))
(testing "Check that several changes are handled nicely"
(is (= (str "changed priority from \"Important\" to \"Regular\", made this Card private and renamed it from "
"\"Tips by State\" to \"Spots by State\".")
(revision/default-diff-str Card
{:name "Tips by State", :private false, :priority "Important"}
{:name "Spots by State", :private true, :priority "Regular"})))))
((get-method revision/diff-str :default)
Card
{:name "Tips by State", :private false, :priority "Important"}
{:name "Spots by State", :private true, :priority "Regular"})))))
;;; # REVISIONS + PUSH-REVISION!
......
test/metabase/models/segment_test.clj
+ 6
− 5
View file @ 0b469bca
(ns metabase.models.segment-test
(:require [clojure.test :refer :all]
[metabase.models.database :refer [Database]]
[metabase.models.revision :as revision]
[metabase.models.segment :as segment :refer [Segment]]
[metabase.models.serialization.hash :as serdes.hash]
[metabase.models.table :refer [Table]]
......@@ -67,7 +68,7 @@
:entity_id (:entity_id segment)
:definition {:filter [:> [:field 4 nil] "2014-10-19"]}
:archived false}
(into {} (-> (#'segment/serialize-segment Segment (:id segment) segment)
(into {} (-> (revision/serialize-instance Segment (:id segment) segment)
(update :id boolean)
(update :table_id boolean)))))))
......@@ -82,7 +83,7 @@
:after "BBB"}
:name {:before "Toucans in the rainforest"
:after "Something else"}}
(#'segment/diff-segments
(revision/diff-map
Segment
segment
(assoc segment
......@@ -93,7 +94,7 @@
(testing "test case where definition doesn't change"
(is (= {:name {:before "A"
:after "B"}}
(#'segment/diff-segments
(revision/diff-map
Segment
{:name "A"
:description "Unchanged"
......@@ -106,7 +107,7 @@
(is (= {:name {:after "A"}
:description {:after "Unchanged"}
:definition {:after {:filter [:and [:> [:field 4 nil] "2014-10-19"]]}}}
(#'segment/diff-segments
(revision/diff-map
Segment
nil
{:name "A"
......@@ -116,7 +117,7 @@
(testing "removals only"
(is (= {:definition {:before {:filter [:and [:> [:field 4 nil] "2014-10-19"] [:= 5 "yes"]]}
:after {:filter [:and [:> [:field 4 nil] "2014-10-19"]]}}}
(#'segment/diff-segments
(revision/diff-map
Segment
{:name "A"
:description "Unchanged"
......
test/metabase/test/redefs.clj
+ 1
− 1
View file @ 0b469bca
......@@ -54,7 +54,7 @@
(throw (if (::with-temp-error? (ex-data e))
e
(ex-info (format "with-temp error for %s: %s" (name model) (ex-message e))
{:model (name model)
{:model model
:attributes attributes
:attributes-with-defaults attributes-with-defaults
:primary-key-column (models/primary-key model)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment