From 74b568843a9ee26b117abd2b3e49de8a0ac5a28e Mon Sep 17 00:00:00 2001 From: Cal Herries <39073188+calherries@users.noreply.github.com> Date: Tue, 2 May 2023 21:35:12 +0530 Subject: [PATCH] Migrate field refs in visualization_settings.column_settings from legacy format (#30460) * Create a migration for updating column_settings * Make fn private * Update test description * Fix clj-kondo * Migrate the entire vizualization_settings * Add test for normalization and migration * Update migration class name * Use generate-string to improve key readability * Add testing strings * Fix migration * transduce * Add another test checking that `expected` is actually the up-to-date version * Rework migration to only update field refs * Use run! --- resources/migrations/000_migrations.yaml | 8 +++ src/metabase/db/custom_migrations.clj | 41 +++++++++++ test/metabase/db/custom_migrations_test.clj | 75 ++++++++++++++++++++- 3 files changed, 123 insertions(+), 1 deletion(-) diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml index a006b3286c1..fb49f176c27 100644 --- a/resources/migrations/000_migrations.yaml +++ b/resources/migrations/000_migrations.yaml @@ -14503,6 +14503,14 @@ databaseChangeLog: constraints: nullable: false + - changeSet: + id: v47.00-016 + author: calherres + comment: Added 0.47.0 - Migrate the report_card.visualization_settings.column_settings field refs from legacy format + changes: + - customChange: + class: "metabase.db.custom_migrations.MigrateLegacyColumnSettingsFieldRefs" + # >>>>>>>>>> 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 8f7aed4f36e..5b6e9c7cc3d 100644 --- a/src/metabase/db/custom_migrations.clj +++ b/src/metabase/db/custom_migrations.clj @@ -9,10 +9,12 @@ If you need to use code from elsewhere, consider copying it into this namespace to minimize risk of the code changing behaviour." (:require [cheshire.core :as json] + [clojure.core.match :refer [match]] [clojure.set :as set] [clojurewerkz.quartzite.jobs :as jobs] [clojurewerkz.quartzite.scheduler :as qs] [clojurewerkz.quartzite.triggers :as triggers] + [medley.core :as m] [metabase.db.connection :as mdb.connection] [metabase.plugins.classloader :as classloader] [metabase.util.honey-sql-2 :as h2x] @@ -170,3 +172,42 @@ (t2/query-one {:update :metabase_field :set {:json_unfolding true} :where [:in :metabase_field.id field-ids-to-update]})))) + +(defn- update-legacy-field-refs [viz-settings] + (let [old-to-new (fn [old] + (match old + ["ref" ref] ["ref" (match ref + ["field-id" x] ["field" x nil] + ["field-literal" x y] ["field" x {"base-type" y}] + ["fk->" x y] (let [x (match x + [_x0 x1] x1 + x x) + y (match y + [_y0 y1] y1 + y y)] + ["field" y {:source-field x}]) + ref ref)] + k k))] + (-> viz-settings + (json/parse-string) + (m/update-existing "column_settings" update-keys + (fn [k] + (-> k + (json/parse-string) + (vec) + (old-to-new) + (json/generate-string)))) + (json/generate-string)))) + +(define-migration MigrateLegacyColumnSettingsFieldRefs + (let [update! (fn [{:keys [id visualization_settings]}] + (t2/query-one {:update :report_card + :set {:visualization_settings visualization_settings} + :where [:= :id id]}))] + (run! update! (eduction (keep (fn [{:keys [id visualization_settings]}] + (let [updated (update-legacy-field-refs visualization_settings)] + (when (not= visualization_settings updated) + {:id id + :visualization_settings updated})))) + (t2/reducible-query {:select [:id :visualization_settings] + :from [:report_card]}))))) diff --git a/test/metabase/db/custom_migrations_test.clj b/test/metabase/db/custom_migrations_test.clj index 2b23cb6c0e0..f368a06da97 100644 --- a/test/metabase/db/custom_migrations_test.clj +++ b/test/metabase/db/custom_migrations_test.clj @@ -1,14 +1,19 @@ (ns metabase.db.custom-migrations-test "Tests to make sure the custom migrations work as expected." (:require + [cheshire.core :as json] [clojure.test :refer :all] + [clojure.walk :as walk] [clojurewerkz.quartzite.jobs :as jobs] [clojurewerkz.quartzite.schedule.cron :as cron] [clojurewerkz.quartzite.scheduler :as qs] [clojurewerkz.quartzite.triggers :as triggers] [metabase.db.schema-migrations-test.impl :as impl] + [metabase.models :refer [Card Database User]] + [metabase.models.interface :as mi] [metabase.task :as task] - [metabase.test.fixtures :as fixtures])) + [metabase.test.fixtures :as fixtures] + [toucan2.core :as t2])) (set! *warn-on-reflection* true) @@ -44,3 +49,71 @@ (is (nil? (qs/get-job (@#'task/scheduler) (jobs/key abandonment-emails-job-key)))) (is (nil? (qs/get-trigger (@#'task/scheduler) (triggers/key abandonment-emails-trigger-key))))))) (finally (task/stop-scheduler!)))))) + +(deftest migrate-legacy-column-settings-field-refs-test + (testing "Migrations v47.00-016: update visualization_settings.column_settings legacy field refs" + (impl/test-migrations ["v47.00-016"] [migrate!] + (let [visualization-settings + {"column_settings" (-> {["name" "column_name"] {"column_title" "ID6"}, + ["ref" ["field-literal" "column_name" "type/Text"]] {"column_title" "ID5"}, + ["ref" ["field-id" 39]] {"column_title" "ID1"}, + ["ref" ["field" 40 nil]] {"column_title" "ID2"}, + ["ref" ["fk->" ["field-id" 39] ["field-id" 40]]] {"column_title" "ID3"}, + ["ref" ["fk->" 41 42]] {"column_title" "ID4"}} + (update-keys json/generate-string))} + expected + {"column_settings" (-> {["name" "column_name"] {"column_title" "ID6"}, + ["ref" ["field" "column_name" {"base-type" "type/Text"}]] {"column_title" "ID5"}, + ["ref" ["field" 39 nil]] {"column_title" "ID1"}, + ["ref" ["field" 40 nil]] {"column_title" "ID2"}, + ["ref" ["field" 40 {"source-field" 39}]] {"column_title" "ID3"}, + ["ref" ["field" 42 {"source-field" 41}]] {"column_title" "ID4"}} + (update-keys json/generate-string))} + user-id (t2/insert-returning-pks! User {:first_name "Howard" + :last_name "Hughes" + :email "howard@aircraft.com" + :password "superstrong" + :date_joined :%now}) + database-id (t2/insert-returning-pks! Database {:name "DB" + :engine "h2" + :created_at :%now + :updated_at :%now + :details "{}"}) + card-id (t2/insert-returning-pks! Card {:name "My Saved Question" + :created_at :%now + :updated_at :%now + :creator_id user-id + :display "table" + :dataset_query "{}" + :visualization_settings (json/generate-string visualization-settings) + :database_id database-id + :collection_id nil})] + (migrate!) + (testing "legacy column_settings are updated" + (is (= expected + (-> (t2/query-one {:select [:visualization_settings] + :from [:report_card] + :where [:= :id card-id]}) + :visualization_settings + json/parse-string)))) + (testing "legacy column_settings are updated to the current format" + (is (= (-> visualization-settings + mi/normalize-visualization-settings + (#'mi/migrate-viz-settings) + walk/stringify-keys) + (-> (t2/query-one {:select [:visualization_settings] + :from [:report_card] + :where [:= :id card-id]}) + :visualization_settings + json/parse-string)))) + (testing "visualization_settings are equivalent before and after migration" + (is (= (-> visualization-settings + mi/normalize-visualization-settings + (#'mi/migrate-viz-settings)) + (-> (t2/query-one {:select [:visualization_settings] + :from [:report_card] + :where [:= :id card-id]}) + :visualization_settings + json/parse-string + mi/normalize-visualization-settings + (#'mi/migrate-viz-settings))))))))) -- GitLab