From 8f5f36720ab2ee2a70bfed9d150536b6aca9cda8 Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Tue, 16 May 2023 10:19:26 +0700
Subject: [PATCH] Serdes for dashboardtab (#30249)
---
.../serialization/v2/models.clj | 1 +
.../serialization/v2/e2e_test.clj | 68 +++++++++++++++++++
src/metabase/models/dashboard.clj | 33 +++++++--
src/metabase/models/dashboard_card.clj | 9 ++-
src/metabase/models/dashboard_tab.clj | 15 ++++
src/metabase/models/serialization.clj | 4 +-
6 files changed, 121 insertions(+), 9 deletions(-)
diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/models.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/models.clj
index e178f2b8970..7c6bd449003 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/v2/models.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/models.clj
@@ -29,6 +29,7 @@
These are not extracted and serialized separately, but they may need some processing done.
For example, the models should also have their entity_id fields populated (if they have one)."
["DashboardCard"
+ "DashboardTab"
"Dimension"
"ParameterCard"
"TimelineEvent"])
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
index 45f0433e121..f3c219b4282 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
@@ -629,3 +629,71 @@
(-> (t2/select-one Dashboard :name dashboard-name)
(t2/hydrate :ordered_cards)
dashboard->link-cards)))))))))))))
+
+(deftest dashboard-with-tabs-test
+ (testing "Dashboard with tabs must be deserialized correctly"
+ (ts/with-random-dump-dir [dump-dir "serdesv2-"]
+ (ts/with-source-and-dest-dbs
+ (ts/with-source-db
+ ;; preparation
+ (t2.with-temp/with-temp
+ [Dashboard {dashboard-id :id
+ dashboard-eid :entity_id} {:name "Dashboard with tab"}
+ Card {card-id-1 :id
+ card-eid-1 :entity_id} {:name "Card 1"}
+ Card {card-id-2 :id
+ card-eid-2 :entity_id} {:name "Card 2"}
+ :model/DashboardTab {tab-id-1 :id
+ tab-eid-1 :entity_id} {:name "Tab 1" :position 0 :dashboard_id dashboard-id}
+ :model/DashboardTab {tab-id-2 :id
+ tab-eid-2 :entity_id} {:name "Tab 2" :position 1 :dashboard_id dashboard-id}
+ DashboardCard _ {:dashboard_id dashboard-id
+ :card_id card-id-1
+ :dashboard_tab_id tab-id-1}
+ DashboardCard _ {:dashboard_id dashboard-id
+ :card_id card-id-2
+ :dashboard_tab_id tab-id-1}
+ DashboardCard _ {:dashboard_id dashboard-id
+ :card_id card-id-1
+ :dashboard_tab_id tab-id-2}
+ DashboardCard _ {:dashboard_id dashboard-id
+ :card_id card-id-2
+ :dashboard_tab_id tab-id-2}]
+ (let [extraction (serdes/with-cache (into [] (extract/extract {})))]
+ (storage/store! (seq extraction) dump-dir))
+
+ (testing "ingest and load"
+ (ts/with-dest-db
+ ;; ingest
+ (testing "doing ingestion"
+ (is (serdes/with-cache (serdes.load/load-metabase (ingest/ingest-yaml dump-dir)))
+ "successful"))
+ (let [new-dashboard (-> (t2/select-one Dashboard :entity_id dashboard-eid)
+ (t2/hydrate :ordered_tabs :ordered_cards))
+ new-tab-id-1 (t2/select-one-pk :model/DashboardTab :entity_id tab-eid-1)
+ new-tab-id-2 (t2/select-one-pk :model/DashboardTab :entity_id tab-eid-2)
+ new-card-id-1 (t2/select-one-pk Card :entity_id card-eid-1)
+ new-card-id-2 (t2/select-one-pk Card :entity_id card-eid-2)]
+
+ (is (=? [{:id new-tab-id-1
+ :dashboard_id (:id new-dashboard)
+ :name "Tab 1"
+ :position 0}
+ {:id new-tab-id-2
+ :dashboard_id (:id new-dashboard)
+ :name "Tab 2"
+ :position 1}]
+ (:ordered_tabs new-dashboard)))
+ (is (=? [{:card_id new-card-id-1
+ :dashboard_id (:id new-dashboard)
+ :dashboard_tab_id new-tab-id-1}
+ {:card_id new-card-id-2
+ :dashboard_id (:id new-dashboard)
+ :dashboard_tab_id new-tab-id-1}
+ {:card_id new-card-id-1
+ :dashboard_id (:id new-dashboard)
+ :dashboard_tab_id new-tab-id-2}
+ {:card_id new-card-id-2
+ :dashboard_id (:id new-dashboard)
+ :dashboard_tab_id new-tab-id-2}]
+ (:ordered_cards new-dashboard))))))))))))
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 76f33a4b7e1..4143faf2aec 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -44,6 +44,7 @@
(methodical/defmethod t2/table-name :model/Dashboard [_model] :report_dashboard)
+
(doto :model/Dashboard
(derive :metabase/model)
(derive ::perms/use-parent-collection-perms)
@@ -469,16 +470,24 @@
(dissoc :id :collection_authority_level :dashboard_id :updated_at)
(update :card_id serdes/*export-fk* 'Card)
(update :action_id serdes/*export-fk* 'Action)
+ (update :dashboard_tab_id serdes/*export-fk* :model/DashboardTab)
(update :parameter_mappings serdes/export-parameter-mappings)
(update :visualization_settings serdes/export-visualization-settings)))
+(defn- extract-dashtab
+ [dashtab]
+ (dissoc dashtab :id :dashboard_id :updated_at))
+
(defmethod serdes/extract-one "Dashboard"
[_model-name _opts dash]
- (let [dash (if (contains? dash :ordered_cards)
- dash
- (hydrate dash :ordered_cards))]
+ (let [dash (cond-> dash
+ (nil? (:ordered_cards dash))
+ (hydrate :ordered_cards)
+ (nil? (:ordered_tabs dash))
+ (hydrate :ordered_tabs))]
(-> (serdes/extract-one-basics "Dashboard" dash)
(update :ordered_cards #(mapv extract-dashcard %))
+ (update :ordered_tabs #(mapv extract-dashtab %))
(update :parameters serdes/export-parameters)
(update :collection_id serdes/*export-fk* Collection)
(update :creator_id serdes/*export-user*)
@@ -497,12 +506,24 @@
(defn- dashcard-for [dashcard dashboard]
(assoc dashcard
:dashboard_id (:entity_id dashboard)
- :serdes/meta [{:model "Dashboard" :id (:entity_id dashboard)}
- {:model "DashboardCard" :id (:entity_id dashcard)}]))
+ :serdes/meta (remove nil?
+ [{:model "Dashboard" :id (:entity_id dashboard)}
+ (when-let [dashtab-eeid (last (:dashboard_tab_id dashcard))]
+ {:model "DashboardTab" :id dashtab-eeid})
+ {:model "DashboardCard" :id (:entity_id dashcard)}])))
+
+(defn- dashtab-for [tab dashboard]
+ (assoc tab
+ :dashboard_id (:entity_id dashboard)
+ :serdes/meta [{:model "Dashboard" :id (:entity_id dashboard)}
+ {:model "DashboardTab" :id (:entity_id tab)}]))
;; Call the default load-one! for the Dashboard, then for each DashboardCard.
(defmethod serdes/load-one! "Dashboard" [ingested maybe-local]
- (let [dashboard ((get-method serdes/load-one! :default) (dissoc ingested :ordered_cards) maybe-local)]
+ (let [dashboard ((get-method serdes/load-one! :default) (dissoc ingested :ordered_cards :ordered_tabs) maybe-local)]
+ (doseq [tab (:ordered_tabs ingested)]
+ (serdes/load-one! (dashtab-for tab dashboard)
+ (t2/select-one :model/DashboardTab :entity_id (:entity_id tab))))
(doseq [dashcard (:ordered_cards ingested)]
(serdes/load-one! (dashcard-for dashcard dashboard)
(t2/select-one 'DashboardCard :entity_id (:entity_id dashcard))))))
diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj
index f4628884d8b..b3627c5abe3 100644
--- a/src/metabase/models/dashboard_card.clj
+++ b/src/metabase/models/dashboard_card.clj
@@ -354,10 +354,14 @@
;;; ----------------------------------------------- SERIALIZATION ----------------------------------------------------
;; DashboardCards are not serialized as their own, separate entities. They are inlined onto their parent Dashboards.
+;; If the parent dashboard has tabs, the dashcards are inlined under each DashboardTab, which are inlined on the Dashboard.
;; However, we can reuse some of the serdes machinery (especially load-one!) by implementing a few serdes methods.
(defmethod serdes/generate-path "DashboardCard" [_ dashcard]
- [(serdes/infer-self-path "Dashboard" (t2/select-one 'Dashboard :id (:dashboard_id dashcard)))
- (serdes/infer-self-path "DashboardCard" dashcard)])
+ (remove nil?
+ [(serdes/infer-self-path "Dashboard" (t2/select-one 'Dashboard :id (:dashboard_id dashcard)))
+ (when (:dashboard_tab_id dashcard)
+ (serdes/infer-self-path "DashboardTab" (t2/select-one :model/DashboardTab :id (:dashboard_tab_id dashcard))))
+ (serdes/infer-self-path "DashboardCard" dashcard)]))
(defmethod serdes/load-xform "DashboardCard"
[dashcard]
@@ -366,6 +370,7 @@
(update :card_id serdes/*import-fk* 'Card)
(update :action_id serdes/*import-fk* 'Action)
(update :dashboard_id serdes/*import-fk* 'Dashboard)
+ (update :dashboard_tab_id serdes/*import-fk* :model/DashboardTab)
(update :created_at #(if (string? %) (u.date/parse %) %))
(update :parameter_mappings serdes/import-parameter-mappings)
(update :visualization_settings serdes/import-visualization-settings)))
diff --git a/src/metabase/models/dashboard_tab.clj b/src/metabase/models/dashboard_tab.clj
index 0d9502fb76c..3acd25a4f0f 100644
--- a/src/metabase/models/dashboard_tab.clj
+++ b/src/metabase/models/dashboard_tab.clj
@@ -3,6 +3,7 @@
[metabase.models.dashboard :refer [Dashboard]]
[metabase.models.interface :as mi]
[metabase.models.serialization :as serdes]
+ [metabase.util.date-2 :as u.date]
[methodical.core :as methodical]
[toucan2.core :as t2]))
@@ -21,6 +22,8 @@
(t2/select-one Dashboard :id (:dashboard_id dashtab)))]
(mi/perms-objects-set dashboard read-or-write)))
+
+;;; ----------------------------------------------- SERIALIZATION ----------------------------------------------------
(defmethod serdes/hash-fields :model/DashboardTab
[_dashboard-tab]
[:name
@@ -29,3 +32,15 @@
:dashboard_id)
:position
:created_at])
+
+;; DashboardTabs are not serialized as their own, separate entities. They are inlined onto their parent Dashboards.
+(defmethod serdes/generate-path "DashboardTab" [_ dashcard]
+ [(serdes/infer-self-path "Dashboard" (t2/select-one 'Dashboard :id (:dashboard_id dashcard)))
+ (serdes/infer-self-path "DashboardTab" dashcard)])
+
+(defmethod serdes/load-xform "DashboardTab"
+ [dashtab]
+ (-> dashtab
+ (dissoc :serdes/meta)
+ (update :dashboard_id serdes/*import-fk* 'Dashboard)
+ (update :created_at #(if (string? %) (u.date/parse %) %))))
diff --git a/src/metabase/models/serialization.clj b/src/metabase/models/serialization.clj
index 385aa7a8238..2953c6eee98 100644
--- a/src/metabase/models/serialization.clj
+++ b/src/metabase/models/serialization.clj
@@ -430,7 +430,9 @@
(defn log-path-str
"Returns a string for logging from a serdes path sequence (i.e. in :serdes/meta)"
[elements]
- (->> elements (map #(str (:model %) " " (:id %))) (str/join " > ")))
+ (->> elements
+ (map #(str (:model %) " " (:id %)))
+ (str/join " > ")))
;; utils
--
GitLab