From d8bf730df83cb8fa016fa11dce7b870ff48b8e80 Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Thu, 11 May 2023 16:03:02 +0700
Subject: [PATCH] report_* models to toucan2 (#30068)
* Dashboard, DashboardCard, DashboardSeries to toucan2
---
.../models/entity_id_test.clj | 2 +-
.../serialization/upsert_test.clj | 4 +-
src/metabase/api/dashboard.clj | 58 ++++----
src/metabase/db/util.clj | 3 +-
src/metabase/models/dashboard.clj | 126 +++++++++---------
src/metabase/models/dashboard_card.clj | 48 ++++---
src/metabase/models/dashboard_card_series.clj | 16 ++-
src/metabase/models/params.clj | 5 +-
src/metabase/models/serialization.clj | 2 +-
test/metabase/api/public_test.clj | 3 +-
test/metabase/models/card_test.clj | 4 +-
test/metabase/test/util.clj | 9 +-
12 files changed, 148 insertions(+), 132 deletions(-)
diff --git a/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj b/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
index da94c10c915..3c4518d18b8 100644
--- a/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj
@@ -45,7 +45,7 @@
:metabase.models.bookmark/DashboardBookmark
:metabase.models.collection.root/RootCollection
:metabase.models.collection-permission-graph-revision/CollectionPermissionGraphRevision
- :metabase.models.dashboard-card-series/DashboardCardSeries
+ :model/DashboardCardSeries
:metabase.models.field-values/FieldValues
:metabase.models.login-history/LoginHistory
:metabase.models.metric-important-field/MetricImportantField
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj
index 5ca7a2a45a6..b859d757c67 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/upsert_test.clj
@@ -44,8 +44,8 @@
;; the new style. Feel free to give them better names - Cam
(deftest maybe-upsert-many!-skip-test
(mt/with-model-cleanup [Card]
- (let [existing-ids (t2/insert-returning-pks! Card @cards)
- inserted-ids (vec (upsert/maybe-upsert-many! {:mode :skip} Card @cards))]
+ (let [existing-ids (sort (t2/insert-returning-pks! Card @cards))
+ inserted-ids (sort (vec (upsert/maybe-upsert-many! {:mode :skip} Card @cards)))]
(is (= existing-ids inserted-ids)))))
(deftest maybe-upsert-many!-same-objects-test
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index be84366f61f..02048ca6820 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -50,11 +50,11 @@
(set! *warn-on-reflection* true)
(defn- dashboards-list [filter-option]
- (as-> (t2/select Dashboard {:where [:and (case (or (keyword filter-option) :all)
- (:all :archived) true
- :mine [:= :creator_id api/*current-user-id*])
- [:= :archived (= (keyword filter-option) :archived)]]
- :order-by [:%lower.name]}) <>
+ (as-> (t2/select :model/Dashboard {:where [:and (case (or (keyword filter-option) :all)
+ (:all :archived) true
+ :mine [:= :creator_id api/*current-user-id*])
+ [:= :archived (= (keyword filter-option) :archived)]]
+ :order-by [:%lower.name]}) <>
(hydrate <> :creator)
(filter mi/can-read? <>)))
@@ -100,7 +100,7 @@
;; position, check that and fix up if needed
(api/maybe-reconcile-collection-position! dashboard-data)
;; Ok, now save the Dashboard
- (first (t2/insert-returning-instances! Dashboard dashboard-data)))]
+ (first (t2/insert-returning-instances! :model/Dashboard dashboard-data)))]
(events/publish-event! :dashboard-create dash)
(snowplow/track-event! ::snowplow/dashboard-created api/*current-user-id* {:dashboard-id (u/the-id dash)})
(assoc dash :last-edit-info (last-edit/edit-information-for-user @api/*current-user*))))
@@ -212,7 +212,7 @@
(defn- get-dashboard
"Get Dashboard with ID."
[id]
- (-> (t2/select-one Dashboard :id id)
+ (-> (t2/select-one :model/Dashboard :id id)
api/check-404
;; i'm a bit worried that this is an n+1 situation here. The cards can be batch hydrated i think because they
;; have a hydration key and an id. moderation_reviews currently aren't batch hydrated but i'm worried they
@@ -339,7 +339,7 @@
;; collection to change position, check that and fix up if needed
(api/maybe-reconcile-collection-position! dashboard-data)
;; Ok, now save the Dashboard
- (let [dash (first (t2/insert-returning-instances! Dashboard dashboard-data))
+ (let [dash (first (t2/insert-returning-instances! :model/Dashboard dashboard-data))
{id->new-card :copied uncopied :uncopied}
(when is_deep_copy
(duplicate-cards existing-dashboard collection_id))]
@@ -401,7 +401,7 @@
collection_id (s/maybe su/IntGreaterThanZero)
collection_position (s/maybe su/IntGreaterThanZero)
cache_ttl (s/maybe su/IntGreaterThanZero)}
- (let [dash-before-update (api/write-check Dashboard id)]
+ (let [dash-before-update (api/write-check :model/Dashboard id)]
;; Do various permissions checks as needed
(collection/check-allowed-to-change-collection dash-before-update dash-updates)
(check-allowed-to-change-embedding dash-before-update dash-updates)
@@ -415,9 +415,9 @@
:present #{:description :position :collection_id :collection_position :cache_ttl}
:non-nil #{:name :parameters :caveats :points_of_interest :show_in_getting_started :enable_embedding
:embedding_params :archived :auto_apply_filters}))]
- (t2/update! Dashboard id updates))))
+ (t2/update! :model/Dashboard id updates))))
;; now publish an event and return the updated Dashboard
- (let [dashboard (t2/select-one Dashboard :id id)]
+ (let [dashboard (t2/select-one :model/Dashboard :id id)]
(events/publish-event! :dashboard-update (assoc dashboard :actor_id api/*current-user-id*))
(assoc dashboard :last-edit-info (last-edit/edit-information-for-user @api/*current-user*))))
@@ -431,8 +431,8 @@
[id]
(log/warn (str "DELETE /api/dashboard/:id is deprecated. Instead of deleting a Dashboard, you should change its "
"`archived` value via PUT /api/dashboard/:id."))
- (let [dashboard (api/write-check Dashboard id)]
- (t2/delete! Dashboard :id id)
+ (let [dashboard (api/write-check :model/Dashboard id)]
+ (t2/delete! :model/Dashboard :id id)
(events/publish-event! :dashboard-delete (assoc dashboard :actor_id api/*current-user-id*)))
api/generic-204-no-content)
@@ -616,17 +616,17 @@
(api/defendpoint-schema GET "/:id/revisions"
"Fetch `Revisions` for Dashboard with ID."
[id]
- (api/read-check Dashboard id)
- (revision/revisions+details Dashboard id))
+ (api/read-check :model/Dashboard id)
+ (revision/revisions+details :model/Dashboard id))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema POST "/:id/revert"
"Revert a Dashboard to a prior `Revision`."
[id :as {{:keys [revision_id]} :body}]
{revision_id su/IntGreaterThanZero}
- (api/write-check Dashboard id)
+ (api/write-check :model/Dashboard id)
(revision/revert!
- :entity Dashboard
+ :entity :model/Dashboard
:id id
:user-id api/*current-user-id*
:revision-id revision_id))
@@ -641,10 +641,10 @@
[dashboard-id]
(api/check-superuser)
(validation/check-public-sharing-enabled)
- (api/check-not-archived (api/read-check Dashboard dashboard-id))
- {:uuid (or (t2/select-one-fn :public_uuid Dashboard :id dashboard-id)
+ (api/check-not-archived (api/read-check :model/Dashboard dashboard-id))
+ {:uuid (or (t2/select-one-fn :public_uuid :model/Dashboard :id dashboard-id)
(u/prog1 (str (UUID/randomUUID))
- (t2/update! Dashboard dashboard-id
+ (t2/update! :model/Dashboard dashboard-id
{:public_uuid <>
:made_public_by_id api/*current-user-id*})))})
@@ -654,8 +654,8 @@
[dashboard-id]
(validation/check-has-application-permission :setting)
(validation/check-public-sharing-enabled)
- (api/check-exists? Dashboard :id dashboard-id, :public_uuid [:not= nil], :archived false)
- (t2/update! Dashboard dashboard-id
+ (api/check-exists? :model/Dashboard :id dashboard-id, :public_uuid [:not= nil], :archived false)
+ (t2/update! :model/Dashboard dashboard-id
{:public_uuid nil
:made_public_by_id nil})
{:status 204, :body nil})
@@ -667,7 +667,7 @@
[]
(validation/check-has-application-permission :setting)
(validation/check-public-sharing-enabled)
- (t2/select [Dashboard :name :id :public_uuid], :public_uuid [:not= nil], :archived false))
+ (t2/select [:model/Dashboard :name :id :public_uuid], :public_uuid [:not= nil], :archived false))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema GET "/embeddable"
@@ -676,13 +676,13 @@
[]
(validation/check-has-application-permission :setting)
(validation/check-embedding-enabled)
- (t2/select [Dashboard :name :id], :enable_embedding true, :archived false))
+ (t2/select [:model/Dashboard :name :id], :enable_embedding true, :archived false))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema GET "/:id/related"
"Return related entities."
[id]
- (-> (t2/select-one Dashboard :id id) api/read-check related/related))
+ (-> (t2/select-one :model/Dashboard :id id) api/read-check related/related))
;;; ---------------------------------------------- Transient dashboards ----------------------------------------------
@@ -811,7 +811,7 @@
;; fetch values for Dashboard 1 parameter 'abc' that are possible when parameter 'def' is set to 100
GET /api/dashboard/1/params/abc/values?def=100"
[id param-key :as {:keys [query-params]}]
- (let [dashboard (api/read-check Dashboard id)]
+ (let [dashboard (api/read-check :model/Dashboard id)]
;; If a user can read the dashboard, then they can lookup filters. This also works with sandboxing.
(binding [qp.perms/*param-values-query* true]
(param-values dashboard param-key query-params))))
@@ -827,7 +827,7 @@
Currently limited to first 1000 results."
[id param-key query :as {:keys [query-params]}]
- (let [dashboard (api/read-check Dashboard id)]
+ (let [dashboard (api/read-check :model/Dashboard id)]
;; If a user can read the dashboard, then they can lookup filters. This also works with sandboxing.
(binding [qp.perms/*param-values-query* true]
(param-values dashboard param-key query-params query))))
@@ -887,7 +887,7 @@
{dashboard-id su/IntGreaterThanZero
dashcard-id su/IntGreaterThanZero
parameters su/JSONString}
- (api/read-check Dashboard dashboard-id)
+ (api/read-check :model/Dashboard dashboard-id)
(actions.execution/fetch-values dashboard-id dashcard-id (json/parse-string parameters)))
#_{:clj-kondo/ignore [:deprecated-var]}
@@ -900,7 +900,7 @@
{dashboard-id su/IntGreaterThanZero
dashcard-id su/IntGreaterThanZero
parameters (s/maybe {s/Keyword s/Any})}
- (api/read-check Dashboard dashboard-id)
+ (api/read-check :model/Dashboard dashboard-id)
;; Undo middleware string->keyword coercion
(actions.execution/execute-dashcard! dashboard-id dashcard-id (update-keys parameters name)))
diff --git a/src/metabase/db/util.clj b/src/metabase/db/util.clj
index d77ccbdc0d6..3d2a934dd30 100644
--- a/src/metabase/db/util.clj
+++ b/src/metabase/db/util.clj
@@ -10,8 +10,7 @@
[toucan2.model :as t2.model]))
(defn toucan-model?
- "Check if `model` is a toucan model.
- In toucan2 any keywords can be a model so it's always true for keyword."
+ "Check if `model` is a toucan model."
[model]
(or
;; toucan 2 models
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index bdf708e83b4..aa9e9e85c31 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -32,69 +32,49 @@
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
[metabase.util.schema :as su]
+ [methodical.core :as methodical]
[schema.core :as s]
[toucan.hydrate :refer [hydrate]]
- [toucan.models :as models]
[toucan2.core :as t2]))
-;;; --------------------------------------------------- Hydration ----------------------------------------------------
-
-(mi/define-simple-hydration-method ordered-cards
- :ordered_cards
- "Return the DashboardCards associated with `dashboard`, in the order they were created."
- [dashboard-or-id]
- (t2/select DashboardCard
- {:select [:dashcard.* [:collection.authority_level :collection_authority_level]]
- :from [[:report_dashboardcard :dashcard]]
- :left-join [[:report_card :card] [:= :dashcard.card_id :card.id]
- [:collection :collection] [:= :collection.id :card.collection_id]]
- :where [:and
- [:= :dashcard.dashboard_id (u/the-id dashboard-or-id)]
- [:or
- [:= :card.archived false]
- [:= :card.archived nil]]] ; e.g. DashCards with no corresponding Card, e.g. text Cards
- :order-by [[:dashcard.created_at :asc]]}))
-
-(mi/define-batched-hydration-method collections-authority-level
- :collection_authority_level
- "Efficiently hydrate the `:collection_authority_level` of a sequence of dashboards."
- [dashboards]
- (when (seq dashboards)
- (let [coll-id->level (into {}
- (map (juxt :id :authority_level))
- (mdb.query/query {:select [:dashboard.id :collection.authority_level]
- :from [[:report_dashboard :dashboard]]
- :left-join [[:collection :collection] [:= :collection.id :dashboard.collection_id]]
- :where [:in :dashboard.id (into #{} (map u/the-id) dashboards)]}))]
- (for [dashboard dashboards]
- (assoc dashboard :collection_authority_level (get coll-id->level (u/the-id dashboard)))))))
+(def Dashboard
+ "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], not it's a reference to the toucan2 model name.
+ We'll keep this till we replace all the Dashboard symbol in our codebase."
+ :model/Dashboard)
-(comment moderation/keep-me)
+(methodical/defmethod t2/table-name :model/Dashboard [_model] :report_dashboard)
-(models/defmodel Dashboard :report_dashboard)
+(doto :model/Dashboard
+ (derive ::perms/use-parent-collection-perms)
+ (derive :metabase/model)
+ (derive :hook/timestamped?)
+ (derive :hook/entity-id))
-(derive Dashboard ::perms/use-parent-collection-perms)
+(t2/deftransforms :model/Dashboard
+ {:parameters mi/transform-parameters-list
+ :embedding_params mi/transform-json})
-;;; ----------------------------------------------- Entity & Lifecycle -----------------------------------------------
-
-(defn- pre-delete [dashboard]
+(t2/define-before-delete :model/Dashboard
+ [dashboard]
(let [dashboard-id (u/the-id dashboard)]
(parameter-card/delete-all-for-parameterized-object! "dashboard" dashboard-id)
(t2/delete! 'Revision :model "Dashboard" :model_id dashboard-id)))
-(defn- pre-insert [dashboard]
+(t2/define-before-insert :model/Dashboard
+ [dashboard]
(let [defaults {:parameters []}
dashboard (merge defaults dashboard)]
(u/prog1 dashboard
(params/assert-valid-parameters dashboard)
(collection/check-collection-namespace Dashboard (:collection_id dashboard)))))
-(defn- post-insert
+(t2/define-after-insert :model/Dashboard
[dashboard]
(u/prog1 dashboard
(parameter-card/upsert-or-delete-from-parameters! "dashboard" (:id dashboard) (:parameters dashboard))))
-(defn- pre-update [dashboard]
+(t2/define-before-update :model/Dashboard
+ [dashboard]
(u/prog1 dashboard
(params/assert-valid-parameters dashboard)
(parameter-card/upsert-or-delete-from-parameters! "dashboard" (:id dashboard) (:parameters dashboard))
@@ -142,30 +122,56 @@
:collection_id (:collection_id dashboard)})
(pulse-card/bulk-create! new-pulse-cards)))))))
-(defn- post-update
+(t2/define-after-update :model/Dashboard
[dashboard]
(update-dashboard-subscription-pulses! dashboard))
-(mi/define-methods
- Dashboard
- {:properties (constantly {::mi/timestamped? true
- ::mi/entity-id true})
- :types (constantly {:parameters :parameters-list, :embedding_params :json})
- :pre-delete pre-delete
- :pre-insert pre-insert
- :post-insert post-insert
- :pre-update pre-update
- :post-update post-update
- :post-select (comp public-settings/remove-public-uuid-if-public-sharing-is-disabled)})
-
-(defmethod serdes/hash-fields Dashboard
+(t2/define-after-select :model/Dashboard
+ [dashboard]
+ (-> dashboard
+ public-settings/remove-public-uuid-if-public-sharing-is-disabled))
+
+(defmethod serdes/hash-fields :model/Dashboard
[_dashboard]
[:name (serdes/hydrated-hash :collection) :created_at])
+;;; --------------------------------------------------- Hydration ----------------------------------------------------
+
+(mi/define-simple-hydration-method ordered-cards
+ :ordered_cards
+ "Return the DashboardCards associated with `dashboard`, in the order they were created."
+ [dashboard-or-id]
+ (t2/select DashboardCard
+ {:select [:dashcard.* [:collection.authority_level :collection_authority_level]]
+ :from [[:report_dashboardcard :dashcard]]
+ :left-join [[:report_card :card] [:= :dashcard.card_id :card.id]
+ [:collection :collection] [:= :collection.id :card.collection_id]]
+ :where [:and
+ [:= :dashcard.dashboard_id (u/the-id dashboard-or-id)]
+ [:or
+ [:= :card.archived false]
+ [:= :card.archived nil]]] ; e.g. DashCards with no corresponding Card, e.g. text Cards
+ :order-by [[:dashcard.created_at :asc]]}))
+
+(mi/define-batched-hydration-method collections-authority-level
+ :collection_authority_level
+ "Efficiently hydrate the `:collection_authority_level` of a sequence of dashboards."
+ [dashboards]
+ (when (seq dashboards)
+ (let [coll-id->level (into {}
+ (map (juxt :id :authority_level))
+ (mdb.query/query {:select [:dashboard.id :collection.authority_level]
+ :from [[:report_dashboard :dashboard]]
+ :left-join [[:collection :collection] [:= :collection.id :dashboard.collection_id]]
+ :where [:in :dashboard.id (into #{} (map u/the-id) dashboards)]}))]
+ (for [dashboard dashboards]
+ (assoc dashboard :collection_authority_level (get coll-id->level (u/the-id dashboard)))))))
+
+(comment moderation/keep-me)
;;; --------------------------------------------------- Revisions ----------------------------------------------------
-(defmethod revision/serialize-instance Dashboard
+(defmethod revision/serialize-instance :model/Dashboard
[_model _id dashboard]
(-> dashboard
(select-keys [:description :name :cache_ttl :auto_apply_filters])
@@ -173,10 +179,10 @@
(-> (select-keys dashboard-card [:size_x :size_y :row :col :id :card_id])
(assoc :series (mapv :id (dashboard-card/series dashboard-card)))))))))
-(defmethod revision/revert-to-revision! Dashboard
+(defmethod revision/revert-to-revision! :model/Dashboard
[_model dashboard-id user-id serialized-dashboard]
;; Update the dashboard description / name / permissions
- (t2/update! Dashboard dashboard-id, (dissoc serialized-dashboard :cards))
+ (t2/update! :model/Dashboard dashboard-id, (dissoc serialized-dashboard :cards))
;; Now update the cards as needed
(let [serialized-cards (:cards serialized-dashboard)
id->serialized-card (zipmap (map :id serialized-cards) serialized-cards)
@@ -202,7 +208,7 @@
serialized-dashboard)
-(defmethod revision/diff-str Dashboard
+(defmethod revision/diff-str :model/Dashboard
[_model dashboard1 dashboard2]
(let [[removals changes] (diff dashboard1 dashboard2)
check-series-change (fn [idx card-changes]
@@ -376,7 +382,7 @@
"Automatically generated cards."
parent-collection-id)
dashboard (first (t2/insert-returning-instances!
- Dashboard
+ :model/Dashboard
(-> dashboard
(dissoc :ordered_cards :rule :related :transient_name
:transient_filters :param_fields :more)
diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj
index 6d5d0ee845e..48a4799207d 100644
--- a/src/metabase/models/dashboard_card.clj
+++ b/src/metabase/models/dashboard_card.clj
@@ -17,23 +17,40 @@
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
[metabase.util.schema :as su]
+ [methodical.core :as methodical]
[schema.core :as s]
[toucan.db :as db]
[toucan.hydrate :refer [hydrate]]
- [toucan.models :as models]
[toucan2.core :as t2]))
-(models/defmodel DashboardCard :report_dashboardcard)
+(def DashboardCard
+ "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], not it's a reference to the toucan2 model name.
+ We'll keep this till we replace all the DashboardCard symbol in our codebase."
+ :model/DashboardCard)
-(doto DashboardCard
+(methodical/defmethod t2/table-name :model/DashboardCard [_model] :report_dashboardcard)
+
+(doto :model/DashboardCard
+ (derive :metabase/model)
(derive ::mi/read-policy.full-perms-for-perms-set)
- (derive ::mi/write-policy.full-perms-for-perms-set))
+ (derive ::mi/write-policy.full-perms-for-perms-set)
+ (derive :hook/timestamped?)
+ (derive :hook/entity-id))
+
+(t2/deftransforms :model/DashboardCard
+ {:parameter_mappings mi/transform-parameters-list
+ :visualization_settings mi/transform-visualization-settings})
+
+(t2/define-before-insert :model/DashboardCard
+ [dashcard]
+ (merge {:parameter_mappings []
+ :visualization_settings {}} dashcard))
(declare series)
;;; Return the set of permissions required to `read-or-write` this DashboardCard. If `:card` and `:series` are already
;;; hydrated this method doesn't need to make any DB calls.
-(defmethod mi/perms-objects-set DashboardCard
+(defmethod mi/perms-objects-set :model/DashboardCard
[dashcard read-or-write]
(let [card (or (:card dashcard)
(t2/select-one [Card :dataset_query] :id (u/the-id (:card_id dashcard))))
@@ -42,19 +59,6 @@
(apply set/union (mi/perms-objects-set card read-or-write) (for [series-card series]
(mi/perms-objects-set series-card read-or-write)))))
-(defn- pre-insert [dashcard]
- (let [defaults {:parameter_mappings []
- :visualization_settings {}}]
- (merge defaults dashcard)))
-
-(mi/define-methods
- DashboardCard
- {:properties (constantly {::mi/timestamped? true
- ::mi/entity-id true})
- :types (constantly {:parameter_mappings :parameters-list
- :visualization_settings :visualization-settings})
- :pre-insert pre-insert})
-
(defn from-parsed-json
"Convert a map with dashboard-card into a Toucan instance assuming it came from parsed JSON and the map keys have
been keywordized. This is useful if the data from a request body inside a `defendpoint` body, and you need it in the
@@ -71,12 +75,12 @@
true
```"
[dashboard-card]
- (t2/instance DashboardCard
+ (t2/instance :model/DashboardCard
(-> dashboard-card
(m/update-existing :parameter_mappings mi/normalize-parameters-list)
(m/update-existing :visualization_settings mi/normalize-visualization-settings))))
-(defmethod serdes/hash-fields DashboardCard
+(defmethod serdes/hash-fields :model/DashboardCard
[_dashboard-card]
[(serdes/hydrated-hash :card) ; :card is optional, eg. text cards
(comp serdes/identity-hash
@@ -105,7 +109,7 @@
(s/defn retrieve-dashboard-card
"Fetch a single DashboardCard by its ID value."
[id :- su/IntGreaterThanZero]
- (-> (t2/select-one DashboardCard :id id)
+ (-> (t2/select-one :model/DashboardCard :id id)
(hydrate :series)))
(defn dashcard->multi-cards
@@ -182,7 +186,7 @@
updates (shallow-updates (select-keys dashboard-card update-ks)
(select-keys old-dashboard-card update-ks))]
(when (seq updates)
- (t2/update! DashboardCard id updates))
+ (t2/update! :model/DashboardCard id updates))
(when (not= (:series dashboard-card [])
(:series old-dashboard-card []))
(update-dashboard-cards-series! {(:id dashboard-card) (:series dashboard-card)}))
diff --git a/src/metabase/models/dashboard_card_series.clj b/src/metabase/models/dashboard_card_series.clj
index 5277f622e64..fcbc2475d46 100644
--- a/src/metabase/models/dashboard_card_series.clj
+++ b/src/metabase/models/dashboard_card_series.clj
@@ -1,15 +1,23 @@
(ns metabase.models.dashboard-card-series
(:require
[metabase.models.serialization :as serdes]
- [toucan.models :as models]
+ [methodical.core :as methodical]
[toucan2.core :as t2]))
-(models/defmodel DashboardCardSeries :dashboardcard_series)
+(def DashboardCardSeries
+ "Used to be the toucan1 model name defined using [[toucan.models/defmodel]], not it's a reference to the toucan2 model name.
+ We'll keep this till we replace all the DashboardCardSeries symbol in our codebase."
+ :model/DashboardCardSeries)
+
+(methodical/defmethod t2/table-name :model/DashboardCardSeries [_model] :dashboardcard_series)
+
+(doto :model/DashboardCardSeries
+ (derive :metabase/model))
(defn- dashboard-card [{:keys [dashboardcard_id]}]
- (t2/select-one 'DashboardCard :id dashboardcard_id))
+ (t2/select-one :model/DashboardCardSeries :id dashboardcard_id))
-(defmethod serdes/hash-fields DashboardCardSeries
+(defmethod serdes/hash-fields :model/DashboardCardSeries
[_dashboard-card-series]
[(comp serdes/identity-hash dashboard-card)
(serdes/hydrated-hash :card)
diff --git a/src/metabase/models/params.clj b/src/metabase/models/params.clj
index a30e3190061..ba818326ee6 100644
--- a/src/metabase/models/params.clj
+++ b/src/metabase/models/params.clj
@@ -241,16 +241,17 @@
id))
(cards->card-param-field-ids (map :card dashcards))))
+
(defn- dashboard->param-field-values
"Return a map of Field ID to FieldValues (if any) for any Fields referenced by Cards in `dashboard`,
or `nil` if none are referenced or none of them have FieldValues."
[dashboard]
(field-ids->param-field-values (dashcards->param-field-ids (:ordered_cards dashboard))))
-(defmethod param-values :metabase.models.dashboard/Dashboard [dashboard]
+(defmethod param-values :model/Dashboard [dashboard]
(dashboard->param-field-values dashboard))
-(defmethod param-fields :metabase.models.dashboard/Dashboard [dashboard]
+(defmethod param-fields :model/Dashboard [dashboard]
(-> (hydrate dashboard [:ordered_cards :card])
:ordered_cards
dashcards->param-field-ids
diff --git a/src/metabase/models/serialization.clj b/src/metabase/models/serialization.clj
index e5bcb2df484..776b8570a03 100644
--- a/src/metabase/models/serialization.clj
+++ b/src/metabase/models/serialization.clj
@@ -857,7 +857,7 @@
"dataset" :model/Card
"collection" :metabase.models.collection/Collection
"database" :metabase.models.database/Database
- "dashboard" :metabase.models.dashboard/Dashboard
+ "dashboard" :model/Dashboard
"table" :metabase.models.table/Table})
(defn- export-viz-link-card
diff --git a/test/metabase/api/public_test.clj b/test/metabase/api/public_test.clj
index d254355d4b4..c371625fc57 100644
--- a/test/metabase/api/public_test.clj
+++ b/test/metabase/api/public_test.clj
@@ -166,7 +166,8 @@
:widget-type "category"
:required true}}}}}]
(is (= {(mt/id :categories :name) {:values (t2/select-one-fn (comp count :values)
- 'FieldValues :field_id category-name-id)
+ 'FieldValues :field_id category-name-id
+ :type :full)
:human_readable_values []
:field_id category-name-id}}
(-> (:param_values (#'api.public/public-card :id (u/the-id card)))
diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj
index f7f49152bbf..60dffd7fd80 100644
--- a/test/metabase/models/card_test.clj
+++ b/test/metabase/models/card_test.clj
@@ -484,7 +484,7 @@
:parameterized_object_type :card
:parameterized_object_id card-id-2
:parameter_id "_CATEGORY_NAME_"}]
- (t2/select 'ParameterCard :card_id source-card-id)))
+ (t2/select 'ParameterCard :card_id source-card-id {:order-by [[:parameterized_object_id :asc]]})))
(t2/delete! :model/Card :id source-card-id)
(is (= []
(t2/select 'ParameterCard :card_id source-card-id)))))))
@@ -519,7 +519,7 @@
:parameter_id "param_2"
:parameterized_object_type :dashboard
:parameterized_object_id (:id dashboard)}]
- (t2/select ParameterCard :card_id source-card-id)))
+ (t2/select ParameterCard :card_id source-card-id {:order-by [[:parameter_id :asc]]})))
;; update card with removing the products.category
(testing "on update result_metadata"
(t2/update! :model/Card source-card-id
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index 00bf9023f2a..014b9b5298f 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -17,9 +17,6 @@
[metabase.models
:refer [Card
Collection
- Dashboard
- DashboardCard
- DashboardCardSeries
Database
Dimension
Field
@@ -123,17 +120,17 @@
(fn [_] {:name (tu.random/random-name)
:color "#ABCDEF"})
- Dashboard
+ :model/Dashboard
(fn [_] {:creator_id (rasta-id)
:name (tu.random/random-name)})
- DashboardCard
+ :model/DashboardCard
(fn [_] {:row 0
:col 0
:size_x 4
:size_y 4})
- DashboardCardSeries
+ :model/DashboardCardSeries
(constantly {:position 0})
Database
--
GitLab