Fix pivot table de/serialization (#35944)

resolves #30133 and #30134

enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj

@@ -27,9 +27,9 @@
    [reifyhealth.specmonstah.core :as rs]
    [toucan2.core :as t2]
    [toucan2.tools.with-temp :as t2.with-temp])
- (:import
-  (java.io File)
-  (java.nio.file Path)))
+  (:import
+   (java.io File)
+   (java.nio.file Path)))
 
 (set! *warn-on-reflection* true)
 
@@ -735,3 +735,72 @@
                     (is (thrown-with-msg? Exception #"Please upgrade"
                                           (cmd/v2-load! dump-dir {}))
                         "throws")))))))))))
+
+(deftest pivot-export-test
+  (testing "Pivot table export and load correctly"
+    (let [old-ids (atom nil)
+          card1s  (atom nil)]
+      (ts/with-random-dump-dir [dump-dir "serdesv2-"]
+        (ts/with-source-and-dest-dbs
+          (ts/with-source-db
+            (mt/dataset sample-dataset
+              ;; ensuring field ids are stable by loading dataset in db first
+              (mt/db)
+              (mt/$ids nil
+                (t2.with-temp/with-temp
+                  [Collection {coll-id :id}  {:name "Pivot Collection"}
+                   Card       card           {:name          "Pivot Card"
+                                              :collection_id coll-id
+                                              :dataset_query {:type     :query
+                                                              :database (mt/id)
+                                                              :query    {:source-table $$orders
+                                                                         :aggregation  [:sum [:field %orders.id nil]]
+                                                                         :breakout     [[:field %orders.user_id nil]]}}
+                                              :visualization_settings
+                                              {:pivot_table.column_split
+                                               {:rows    [[:field %people.name {:base-type    :type/Text
+                                                                                :source-field %orders.user_id}]]
+                                                :columns [[:field %products.title {:base-type    :type/Text
+                                                                                   :source-field %orders.product_id}]]
+                                                :values  [[:aggregation 0]]}
+                                               :column_settings
+                                               {(format "[\"ref\",[\"field\",%s,null]]" %people.name)
+                                                {:pivot_table.column_sort_order "descending"}}}}]
+                  (reset! old-ids {:people.name       %people.name
+                                   :orders.user_id    %orders.user_id
+                                   :products.title    %products.title
+                                   :orders.product_id %orders.product_id})
+                  (reset! card1s card)
+                  (storage/store! (extract/extract {}) dump-dir)))))
+
+          (ts/with-dest-db
+            ;; ensure there is something in db so that sample-dataset gets different field ids for sure
+            (mt/dataset office-checkins
+              (mt/db))
+
+            (mt/dataset sample-dataset
+              ;; ensuring field ids are stable by loading dataset in db first
+              (mt/db)
+              (mt/$ids nil
+                (testing "Column ids are different in different dbs")
+                (is (not= @old-ids
+                          {:people.name       %people.name
+                           :orders.user_id    %orders.user_id
+                           :products.title    %products.title
+                           :orders.product_id %orders.product_id}))
+
+                (serdes.load/load-metabase! (ingest/ingest-yaml dump-dir))
+
+                (let [viz (t2/select-one-fn :visualization_settings Card :entity_id (:entity_id @card1s))]
+                  (testing "column ids inside pivot table transferred"
+                    (is (= [[:field %people.name {:base-type    :type/Text
+                                                  :source-field %orders.user_id}]]
+                           (get-in viz [:pivot_table.column_split :rows])))
+                    (is (= [[:field %products.title {:base-type    :type/Text
+                                                     :source-field %orders.product_id}]]
+                           (get-in viz [:pivot_table.column_split :columns]))))
+                  (testing "column sort order restored"
+                    (is (= "descending"
+                           (get-in viz [:column_settings
+                                        (format "[\"ref\",[\"field\",%s,null]]" %people.name)
+                                        :pivot_table.column_sort_order])))))))))))))

enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj

@@ -1643,7 +1643,7 @@
                                                      :collection_id coll2-id
                                                      :dataset_query {:query {:source-table (str "card__" card1-id)
                                                                              :aggregation  [[:count]]}}}]
-      (testing "Complain about card to available for exporting"
+      (testing "Complain about card not available for exporting"
         (is (some #(str/starts-with? % "Failed to export Dashboard")
                   (into #{}
                         (map (fn [[_log-level _error message]] message))
@@ -1652,7 +1652,7 @@
                                             :no-settings   true
                                             :no-data-model true}))))))
 
-      (testing "Complain about card depending on a card"
+      (testing "Complain about card depending on an outside card"
         (is (some #(str/starts-with? % "Failed to export Cards")
                   (into #{}
                         (map (fn [[_log-level _error message]] message))

src/metabase/models/serialization.clj

@@ -1194,6 +1194,16 @@
           (m/update-existing    :click_behavior import-viz-click-behavior-link)
           (m/update-existing-in [:click_behavior :parameterMapping] import-viz-click-behavior-mappings)))
 
+(defn- export-pivot-table [settings]
+  (some-> settings
+          (m/update-existing-in [:pivot_table.column_split :rows] ids->fully-qualified-names)
+          (m/update-existing-in [:pivot_table.column_split :columns] ids->fully-qualified-names)))
+
+(defn- import-pivot-table [settings]
+  (some-> settings
+          (m/update-existing-in [:pivot_table.column_split :rows] mbql-fully-qualified-names->ids)
+          (m/update-existing-in [:pivot_table.column_split :columns] mbql-fully-qualified-names->ids)))
+
 (defn- export-visualizations [entity]
   (mbql.u/replace
    entity
@@ -1240,6 +1250,7 @@
         export-visualizations
         export-viz-link-card
         export-viz-click-behavior
+        export-pivot-table
         (update :column_settings export-column-settings))))
 
 (defn- import-viz-link-card
@@ -1288,6 +1299,7 @@
         import-visualizations
         import-viz-link-card
         import-viz-click-behavior
+        import-pivot-table
         (update :column_settings import-column-settings))))
 
 (defn- viz-link-card-deps