From f60e1e49d82e71dbbb33bb5f687319561693bd4a Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Mon, 22 May 2023 15:35:22 +0700
Subject: [PATCH] Stack cards vertically for dashboard with tabs on downgrade
(#30784)
---
resources/migrations/000_migrations.yaml | 10 ++-
src/metabase/db/custom_migrations.clj | 27 ++++++
test/metabase/db/custom_migrations_test.clj | 97 +++++++++++++++++++++
3 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index 784f619f87d..620e4dab4c0 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -14623,7 +14623,15 @@ databaseChangeLog:
- customChange:
class: "metabase.db.custom_migrations.MigrateLegacyResultMetadataFieldRefs"
-# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
+ - changeSet:
+ id: v47.00-029
+ author: qnkhuat
+ comment: Added 0.47.0 - Stack cards vertically for dashboard with tabs on downgrade
+ changes:
+ - customChange:
+ class: "metabase.db.custom_migrations.DowngradeDashboardTab"
+
+ # >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
########################################################################################################################
#
diff --git a/src/metabase/db/custom_migrations.clj b/src/metabase/db/custom_migrations.clj
index 6463f890676..5fc76731279 100644
--- a/src/metabase/db/custom_migrations.clj
+++ b/src/metabase/db/custom_migrations.clj
@@ -245,3 +245,30 @@
:where [:and
[:<> :result_metadata nil]
[:<> :result_metadata "[]"]]})))))
+
+(defn- update-card-row-on-downgrade-for-dashboard-tab
+ [dashboard-id]
+ (let [ordered-tab+cards (->> (t2/query {:select [:report_dashboardcard.* [:dashboard_tab.position :tab_position]]
+ :from [:report_dashboardcard]
+ :where [:= :report_dashboardcard.dashboard_id dashboard-id]
+ :left-join [:dashboard_tab [:= :dashboard_tab.id :report_dashboardcard.dashboard_tab_id]]})
+ (group-by :tab_position)
+ ;; sort by tab position
+ (sort-by first))
+ cards->max-height (fn [cards] (apply max (map #(+ (:row %) (:size_y %)) cards)))]
+ (loop [position+cards ordered-tab+cards
+ next-tab-row 0]
+ (when-let [[tab-pos cards] (first position+cards)]
+ (if (zero? tab-pos)
+ (recur (rest position+cards) (long (cards->max-height cards)))
+ (do
+ (t2/query {:update :report_dashboardcard
+ :set {:row [:+ :row next-tab-row]}
+ :where [:= :dashboard_tab_id (:dashboard_tab_id (first cards))]})
+ (recur (rest position+cards) (long (+ next-tab-row (cards->max-height cards))))))))))
+
+(define-reversible-migration DowngradeDashboardTab
+ (log/info "No forward migration for DowngradeDashboardTab")
+ (run! update-card-row-on-downgrade-for-dashboard-tab
+ (eduction (map :dashboard_id) (t2/reducible-query {:select-distinct [:dashboard_id]
+ :from [:dashboard_tab]}))))
diff --git a/test/metabase/db/custom_migrations_test.clj b/test/metabase/db/custom_migrations_test.clj
index 273934cf50f..e5cb93a9099 100644
--- a/test/metabase/db/custom_migrations_test.clj
+++ b/test/metabase/db/custom_migrations_test.clj
@@ -8,7 +8,9 @@
[clojurewerkz.quartzite.schedule.cron :as cron]
[clojurewerkz.quartzite.scheduler :as qs]
[clojurewerkz.quartzite.triggers :as triggers]
+ [metabase.db.connection :as mdb.connection]
[metabase.db.schema-migrations-test.impl :as impl]
+ [metabase.db.setup :as db.setup]
[metabase.models :refer [Card Database User]]
[metabase.models.interface :as mi]
[metabase.task :as task]
@@ -177,3 +179,98 @@
json/parse-string
((:out mi/transform-result-metadata))
json/generate-string)))))))))
+
+(deftest downgrade-dashboard-tabs-test
+ (testing "Migrations v47.00-029: downgrade dashboard tab test"
+ (impl/test-migrations ["v47.00-029"] [_migrate!]
+ (let [{:keys [db-type ^javax.sql.DataSource data-source]} mdb.connection/*application-db*
+ migrate! (partial db.setup/migrate! db-type data-source)
+ _ (migrate! :up)
+ user-id (first (t2/insert-returning-pks! User {:first_name "Howard"
+ :last_name "Hughes"
+ :email "howard@aircraft.com"
+ :password "superstrong"
+ :date_joined :%now}))
+ dashboard-id (first (t2/insert-returning-pks! :model/Dashboard {:name "A dashboard"
+ :creator_id user-id}))
+ tab1-id (first (t2/insert-returning-pks! :model/DashboardTab {:name "Tab 1"
+ :position 0
+ :dashboard_id dashboard-id}))
+ tab2-id (first (t2/insert-returning-pks! :model/DashboardTab {:name "Tab 2"
+ :position 1
+ :dashboard_id dashboard-id}))
+ ;; adds a dummy tab without cards to make sure our migration doesn't fail on such case
+ _ (first (t2/insert-returning-pks! :model/DashboardTab {:name "Tab 3"
+ :position 2
+ :dashboard_id dashboard-id}))
+ tab4-id (first (t2/insert-returning-pks! :model/DashboardTab {:name "Tab 4"
+ :position 3
+ :dashboard_id dashboard-id}))
+ default-card {:dashboard_id dashboard-id
+ :visualization_settings {:virtual_card {:display "text"}
+ :text "A text card"}}
+ tab1-card1-id (first (t2/insert-returning-pks! :model/DashboardCard (merge
+ default-card
+ {:dashboard_tab_id tab1-id
+ :row 0
+ :col 0
+ :size_x 4
+ :size_y 4})))
+
+ tab1-card2-id (first (t2/insert-returning-pks! :model/DashboardCard (merge
+ default-card
+ {:dashboard_tab_id tab1-id
+ :row 2
+ :col 0
+ :size_x 2
+ :size_y 6})))
+
+ tab2-card1-id (first (t2/insert-returning-pks! :model/DashboardCard (merge
+ default-card
+ {:dashboard_tab_id tab2-id
+ :row 0
+ :col 0
+ :size_x 4
+ :size_y 4})))
+
+ tab2-card2-id (first (t2/insert-returning-pks! :model/DashboardCard (merge
+ default-card
+ {:dashboard_tab_id tab2-id
+ :row 4
+ :col 0
+ :size_x 4
+ :size_y 2})))
+ tab4-card1-id (first (t2/insert-returning-pks! :model/DashboardCard (merge
+ default-card
+ {:dashboard_tab_id tab4-id
+ :row 0
+ :col 0
+ :size_x 4
+ :size_y 4})))
+
+ tab4-card2-id (first (t2/insert-returning-pks! :model/DashboardCard (merge
+ default-card
+ {:dashboard_tab_id tab4-id
+ :row 4
+ :col 0
+ :size_x 4
+ :size_y 2})))]
+ (migrate! :down 46)
+ (is (= [;; tab 1
+ {:id tab1-card1-id
+ :row 0}
+ {:id tab1-card2-id
+ :row 2}
+
+ ;; tab 2
+ {:id tab2-card1-id
+ :row 8}
+ {:id tab2-card2-id
+ :row 12}
+
+ ;; tab 3
+ {:id tab4-card1-id
+ :row 14}
+ {:id tab4-card2-id
+ :row 18}]
+ (t2/select-fn-vec #(select-keys % [:id :row]) :model/DashboardCard :dashboard_id dashboard-id)))))))
--
GitLab