Skip to content
Snippets Groups Projects
Unverified Commit 4cc415ef authored by Cal Herries's avatar Cal Herries Committed by GitHub
Browse files

Migrate card revisions to have join aliases in column_settings keys (#31216)

parent ff00055b
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
resources/migrations/000_migrations.yaml
+ 16
− 0
View file @ 4cc415ef
......@@ -14840,6 +14840,22 @@ databaseChangeLog:
- customChange:
class: "metabase.db.custom_migrations.RevisionDashboardMigrateGridFrom18To24"
 
- changeSet:
id: v47.00-033
author: calherries
comment: Added 0.47.0 - Migrate field refs in visualization_settings.column_settings keys from legacy format
changes:
- customChange:
class: "metabase.db.custom_migrations.RevisionMigrateLegacyColumnSettingsFieldRefs"
- changeSet:
id: v47.00-034
author: calherries
comment: Added 0.47.0 - Add join-alias to the visualization_settings.column_settings field refs in card revisions
changes:
- customChange:
class: "metabase.db.custom_migrations.RevisionAddJoinAliasToColumnSettingsFieldRefs"
- changeSet:
id: v47.00-035
author: calherries
......
src/metabase/db/custom_migrations.clj
+ 101
− 6
View file @ 4cc415ef
......@@ -208,7 +208,16 @@
{:id id
:visualization_settings (json/generate-string updated)}))))
(t2/reducible-query {:select [:id :visualization_settings]
:from [:report_card]})))))
:from [:report_card]
:where [:or
;; these match legacy field refs in column_settings
[:like :visualization_settings "%ref\\\\\",[\\\\\"field-id%"]
[:like :visualization_settings "%ref\\\\\",[\\\\\"field-literal%"]
[:like :visualization_settings "%ref\\\\\",[\\\\\"fk->%"]
;; MySQL with NO_BACKSLASH_ESCAPES disabled:
[:like :visualization_settings "%ref\\\\\\\",[\\\\\\\"field-id%"]
[:like :visualization_settings "%ref\\\\\\\",[\\\\\\\"field-literal%"]
[:like :visualization_settings "%ref\\\\\\\",[\\\\\\\"fk->%"]]})))))
(defn- update-legacy-field-refs-in-result-metadata [result-metadata]
(let [old-to-new (fn [ref]
......@@ -224,9 +233,9 @@
["field" y {:source-field x}])
_ ref))]
(->> result-metadata
(json/parse-string)
json/parse-string
(map #(m/update-existing % "field_ref" old-to-new))
(json/generate-string))))
json/generate-string)))
(define-migration MigrateLegacyResultMetadataFieldRefs
(let [update! (fn [{:keys [id result_metadata]}]
......@@ -240,9 +249,10 @@
:result_metadata updated}))))
(t2/reducible-query {:select [:id :result_metadata]
:from [:report_card]
:where [:and
[:<> :result_metadata nil]
[:<> :result_metadata "[]"]]})))))
:where [:or
[:like :result_metadata "%field-id%"]
[:like :result_metadata "%field-literal%"]
[:like :result_metadata "%fk->%"]]})))))
(defn- remove-opts
"Removes options from the `field_ref` options map. If the resulting map is empty, it's replaced it with nil."
......@@ -425,6 +435,91 @@
:from [:revision]
:where [:= :model "Dashboard"]}))))
(define-migration RevisionMigrateLegacyColumnSettingsFieldRefs
(let [update-one! (fn [{:keys [id object]}]
(let [object (json/parse-string object)
updated (update object "visualization_settings" update-legacy-field-refs-in-viz-settings)]
(when (not= updated object)
(t2/query-one {:update :revision
:set {:object (json/generate-string updated)}
:where [:= :id id]}))))]
(run! update-one! (t2/reducible-query {:select [:id :object]
:from [:revision]
:where [:and
[:= :model "Card"]
[:or
;; these match legacy field refs in column_settings
[:like :object "%ref\\\\\",[\\\\\"field-id%"]
[:like :object "%ref\\\\\",[\\\\\"field-literal%"]
[:like :object "%ref\\\\\",[\\\\\"fk->%"]
;; MySQL with NO_BACKSLASH_ESCAPES disabled:
[:like :object "%ref\\\\\\\",[\\\\\\\"field-id%"]
[:like :object "%ref\\\\\\\",[\\\\\\\"field-literal%"]
[:like :object "%ref\\\\\\\",[\\\\\\\"fk->%"]]]}))))
(define-reversible-migration RevisionAddJoinAliasToColumnSettingsFieldRefs
;; This migration is essentially the same as `AddJoinAliasToColumnSettingsFieldRefs`, but for card revisions.
;; We can't use the same migration because cards in the revision table don't always have `result_metadata`.
;; So instead, we use the join aliases from card's `dataset_query` to create field refs in visualization_settings.
;; There will inevitably be extra entries in visualization_settings.column_settings that don't match field refs in result_metadata, but that's ok.
(let [add-join-aliases
(fn [card]
(let [join-aliases (->> (get-in card ["dataset_query" "query" "joins"])
(map #(get % "alias"))
set)]
(if (seq join-aliases)
(update (get card "visualization_settings") "column_settings"
(fn [column_settings]
(let [copies-with-join-alias (into {}
(mapcat (fn [[k v]]
(match (vec (json/parse-string k))
["ref" ["field" id opts]]
(for [alias join-aliases]
[(json/generate-string ["ref" ["field" id (assoc opts "join-alias" alias)]]) v])
_ '()))
column_settings))]
;; existing column settings should take precedence over the copies in case there is a conflict
(merge copies-with-join-alias column_settings))))
card)))
update-one!
(fn [revision]
(let [card (json/parse-string (:object revision))]
(when (not= (get card "query_type") "native") ; native queries won't have join aliases, so we can exclude them straight away
(let [updated (add-join-aliases card)]
(when (not= updated (get "visualization_settings" card))
(t2/query {:update :revision
:set {:object (json/generate-string (assoc card "visualization_settings" updated))}
:where [:= :id (:id revision)]}))))))]
(run! update-one! (t2/reducible-query {:select [:*]
:from [:revision]
:where [:and
;; only include cards with field refs in column_settings
[:or
[:like :object "%ref\\\\\",[\\\\\"field%"]
[:like :object "%ref\\\\\\\",[\\\\\\\"field%"]]
;; only include cards with joins
[:like :object "%joins%"]
[:= :model "Card"]]})))
;; Reverse migration
(let [update-one!
(fn [revision]
(let [card (json/parse-string (:object revision))]
(when (not= (get card "query_type") "native")
(let [viz-settings (get card "visualization_settings")
updated (remove-join-alias-from-column-settings-field-refs viz-settings)]
(when (not= updated viz-settings)
(t2/query {:update :revision
:set {:object (json/generate-string (assoc card "visualization_settings" updated))}
:where [:= :id (:id revision)]}))))))]
(run! update-one! (t2/reducible-query {:select [:*]
:from [:revision]
:where [:and
[:or
[:like :object "%ref\\\\\",[\\\\\"field%"]
[:like :object "%ref\\\\\\\",[\\\\\\\"field%"]]
[:like :object "%join-alias%"]
[:= :model "Card"]]}))))
(define-migration MigrateLegacyDashboardCardColumnSettingsFieldRefs
(let [update-one! (fn [{:keys [id visualization_settings]}]
(let [parsed (json/parse-string visualization_settings)
......
test/metabase/db/custom_migrations_test.clj
+ 124
− 1
View file @ 4cc415ef
......@@ -15,7 +15,7 @@
[metabase.db.custom-migrations :as custom-migrations]
[metabase.db.schema-migrations-test.impl :as impl]
[metabase.db.setup :as db.setup]
[metabase.models :refer [Card Database User]]
[metabase.models :refer [Card Database Revision User]]
[metabase.models.interface :as mi]
[metabase.task :as task]
[metabase.test.fixtures :as fixtures]
......@@ -513,6 +513,129 @@
(testing "shouldn't have overlapping cards"
(is (true? (no-cards-are-overlap? rollbacked-to-24)))))))
(deftest revision-migrate-legacy-column-settings-field-refs-test
(testing "Migrations v47.00-033: update visualization_settings.column_settings legacy field refs"
(impl/test-migrations ["v47.00-033"] [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})
card {:visualization_settings visualization-settings}
revision-id (t2/insert-returning-pks! Revision {:model "Card"
:model_id 1 ;; TODO: this could be a foreign key in the future
:user_id user-id
:object (json/generate-string card)})]
(migrate!)
(testing "legacy column_settings are updated"
(is (= expected
(-> (t2/query-one {:select [:object]
:from [:revision]
:where [:= :id revision-id]})
:object
json/parse-string
(get "visualization_settings")))))
(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 [:object]
:from [:revision]
:where [:= :id revision-id]})
:object
json/parse-string
(get "visualization_settings")))))
(testing "visualization_settings are equivalent before and after migration"
(is (= (-> visualization-settings
mi/normalize-visualization-settings
(#'mi/migrate-viz-settings))
(-> (t2/query-one {:select [:object]
:from [:revision]
:where [:= :id revision-id]})
:object
json/parse-string
(get "visualization_settings")
mi/normalize-visualization-settings
(#'mi/migrate-viz-settings)))))))))
(deftest revision-add-join-alias-to-column-settings-field-refs-test
(testing "Migrations v47.00-034: update visualization_settings.column_settings legacy field refs"
(impl/test-migrations ["v47.00-034"] [migrate!]
(let [{:keys [db-type ^javax.sql.DataSource data-source]} mdb.connection/*application-db*
visualization-settings
{"column_settings" (-> {["ref" ["field" 1 {"join-alias" "Joined table"}]] {"column_title" "THIS SHOULD TAKE PRECENDCE"}
["ref" ["field" 1 nil]] {"column_title" "THIS SHOULD NOT TAKE PRECEDENCE"}
["ref" ["field" 2 {"source-field" 3}]] {"column_title" "2"}
["ref" ["field" "column_name" {"base-type" "type/Text"}]] {"column_title" "3"}
["name" "column_name"] {"column_title" "4"}}
(update-keys json/generate-string))}
expected
{"column_settings" (-> {["ref" ["field" 1 {"join-alias" "Joined table"}]] {"column_title" "THIS SHOULD TAKE PRECENDCE"}
["ref" ["field" 1 nil]] {"column_title" "THIS SHOULD NOT TAKE PRECEDENCE"}
["ref" ["field" 2 {"source-field" 3}]] {"column_title" "2"}
["ref" ["field" 2 {"source-field" 3
"join-alias" "Joined table"}]] {"column_title" "2"}
["ref" ["field" "column_name" {"base-type" "type/Text"}]] {"column_title" "3"}
["ref" ["field" "column_name" {"base-type" "type/Text"
"join-alias" "Joined table"}]] {"column_title" "3"}
["name" "column_name"] {"column_title" "4"}}
(update-keys json/generate-string))}
card {:visualization_settings visualization-settings
:dataset_query {:database 1
:query {:joins [{:alias "Joined table"
:condition [:=
[:field 43 nil]
[:field 46 {:join-alias "Joined table"}]]
:fields :all
:source-table 5}]
:source-table 2}
:type :query}}
user-id (t2/insert-returning-pks! User {:first_name "Howard"
:last_name "Hughes"
:email "howard@aircraft.com"
:password "superstrong"
:date_joined :%now})
revision-id (t2/insert-returning-pks! Revision {:model "Card"
:model_id 1 ;; TODO: this could be a foreign key in the future
:user_id user-id
:object (json/generate-string card)})]
(migrate!)
(testing "column_settings field refs are updated"
(is (= expected
(-> (t2/query-one {:select [:object]
:from [:revision]
:where [:= :id revision-id]})
:object
json/parse-string
(get "visualization_settings")))))
(db.setup/migrate! db-type data-source :down 46)
(testing "down migration restores original visualization_settings, except it's okay if join-alias are missing"
(is (= (m/dissoc-in visualization-settings
["column_settings" (json/generate-string ["ref" ["field" 1 {"join-alias" "Joined table"}]])])
(-> (t2/query-one {:select [:object]
:from [:revision]
:where [:= :id revision-id]})
:object
json/parse-string
(get "visualization_settings")))))))))
(deftest migrate-legacy-dashboard-card-column-settings-field-refs-test
(testing "Migrations v47.00-043: update report_dashboardcard.visualization_settings.column_settings legacy field refs"
(impl/test-migrations ["v47.00-043"] [migrate!]
......
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