From 077d8e364f34b4bbaa62aa86d7575f06779c3376 Mon Sep 17 00:00:00 2001
From: Braden Shepherdson <Braden.Shepherdson@gmail.com>
Date: Thu, 15 Sep 2022 11:22:47 -0400
Subject: [PATCH] Serdes v2: Fix a missing case in the deps for a Card (#25392)
Cards can depend on other Cards as their `:source-table`, but the code
to extract `serdes-dependencies` from the MBQL query did not
capture that case.
---
.../serialization/v2/extract_test.clj | 48 +++++++++++++++++++
src/metabase/models/serialization/util.clj | 3 ++
2 files changed, 51 insertions(+)
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
index d504b9e5fbc..cd743cb4438 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
@@ -155,6 +155,30 @@
:enabled true}]
:column_settings
{(str "[\"ref\",[\"field\"," field2-id ",null]]") {:column_title "Locus"}}}}]
+
+ Card [{c4-id :id
+ c4-eid :entity_id} {:name "Referenced Question"
+ :database_id db-id
+ :table_id schema-id
+ :collection_id coll-id
+ :creator_id mark-id
+ :dataset_query
+ (json/generate-string
+ {:query {:source-table no-schema-id
+ :filter [:>= [:field field-id nil] 18]}
+ :database db-id})}]
+ Card [{c5-id :id
+ c5-eid :entity_id} {:name "Dependent Question"
+ :database_id db-id
+ :table_id schema-id
+ :collection_id coll-id
+ :creator_id mark-id
+ :dataset_query
+ (json/generate-string
+ {:query {:source-table (str "card__" c4-id)
+ :aggregation [[:count]]}
+ :database db-id})}]
+
Dashboard [{dash-id :id
dash-eid :entity_id} {:name "Shared Dashboard"
:collection_id coll-id
@@ -323,6 +347,30 @@
{:model "Field" :id "Other Field"}]}
(set (serdes.base/serdes-dependencies ser)))))))
+ (testing "Cards can be based on other cards"
+ (let [ser (serdes.base/extract-one "Card" {} (select-one "Card" [:= :id c5-id]))]
+ (is (schema= {:serdes/meta (s/eq [{:model "Card" :id c5-eid}])
+ :table_id (s/eq ["My Database" "PUBLIC" "Schema'd Table"])
+ :creator_id (s/eq "mark@direstrai.ts")
+ :collection_id (s/eq coll-eid)
+ :dataset_query (s/eq {:query {:source-table c4-eid
+ :aggregation [[:count]]}
+ :database "My Database"})
+ :created_at LocalDateTime
+ (s/optional-key :updated_at) LocalDateTime
+ s/Keyword s/Any}
+ ser))
+ (is (not (contains? ser :id)))
+
+ (testing "and depend on their Database, Table and Collection, and the upstream Card"
+ (is (= #{[{:model "Database" :id "My Database"}]
+ [{:model "Database" :id "My Database"}
+ {:model "Schema" :id "PUBLIC"}
+ {:model "Table" :id "Schema'd Table"}]
+ [{:model "Collection" :id coll-eid}]
+ [{:model "Card" :id c4-eid}]}
+ (set (serdes.base/serdes-dependencies ser)))))))
+
(testing "Dashcard :visualization_settings are included in their deps"
(let [ser (serdes.base/extract-one "DashboardCard" {} (select-one "DashboardCard" [:= :id dc2-id]))]
(is (schema= {:serdes/meta (s/eq [{:model "Dashboard" :id other-dash}
diff --git a/src/metabase/models/serialization/util.clj b/src/metabase/models/serialization/util.clj
index 2f9c31e709f..ebee55402dc 100644
--- a/src/metabase/models/serialization/util.clj
+++ b/src/metabase/models/serialization/util.clj
@@ -297,6 +297,9 @@
(cond
(and (= k :database) (string? v)) #{[{:model "Database" :id v}]}
(and (= k :source-table) (vector? v)) #{(table->path v)}
+ (and (= k :source-table)
+ (string? v)
+ (serdes.base/entity-id? v)) #{[{:model "Card" :id v}]}
(and (= k :source-field) (vector? v)) #{(field->path v)}
(map? v) (mbql-deps-map v)
(vector? v) (mbql-deps-vector v)))
--
GitLab