Leave viz-setting part as is if normalization fails (#43336)

Fixes #43089.
metabase · May 30, 2024 · 24ec71c · 24ec71c
1 parent b77443a
commit 24ec71c
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 2 deletions.
diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj
@@ -294,8 +294,14 @@
           (normalize-mbql-clauses [form]
             (walk/postwalk
              (fn [form]
-               (cond-> form
-                 (mbql-field-clause? form) mbql.normalize/normalize))
+               (try
+                 (cond-> form
+                   (mbql-field-clause? form) mbql.normalize/normalize)
+                 (catch Exception e
+                   (log/warnf "Unable to normalize visualization-settings part %s: %s"
+                              (u/pprint-to-str 'red form)
+                              (ex-message e))
+                   form)))
              form))]
     (cond-> (walk/keywordize-keys (dissoc viz-settings "column_settings" "graph.metrics"))
       (get viz-settings "column_settings") (assoc :column_settings (normalize-column-settings (get viz-settings "column_settings")))

diff --git a/test/metabase/db/custom_migrations_test.clj b/test/metabase/db/custom_migrations_test.clj
@@ -177,6 +177,60 @@
                      mi/normalize-visualization-settings
                      (#'mi/migrate-viz-settings)))))))))
 
+(deftest normalize-mbql-clause-impostor-in-visualization-settings-test
+  (let [viz-settings
+        {"table.pivot_column" "TAX",
+         "graph.metrics" ["expression"],
+         "pivot_table.column_split"
+         {"rows"
+          [["field" 39 {"base-type" "type/DateTime", "temporal-unit" "month"}]
+           ["expression" "expression"]
+           ["field"
+            33
+            {"base-type" "type/Float",
+             "binning" {"strategy" "num-bins", "min-value" 0, "max-value" 12, "num-bins" 8, "bin-width" 2}}]],
+          "columns" [],
+          "values" [["aggregation" 0]]},
+         "pivot_table.column_widths" {"leftHeaderWidths" [141 99 80], "totalLeftHeaderWidths" 320, "valueHeaderWidths" {}},
+         "table.cell_column" "expression",
+         "table.column_formatting"
+         [{"columns" ["expression" nil "TAX" "count"],
+           "type" "single",
+           "operator" "is-null",
+           "value" 10,
+           "color" "#EF8C8C",
+           "highlight_row" false,
+           "id" 0}],
+         "column_settings" {"[\"ref\",[\"expression\",\"expression\"]]" {"number_style" "currency"}},
+         "series_settings" {"expression" {"line.interpolate" "step-after", "line.style" "dotted"}},
+         "graph.dimensions" ["CREATED_AT"]}]
+    (is (= {:table.pivot_column "TAX"
+            :graph.metrics ["expression"]
+            :pivot_table.column_split
+            {:rows
+             [[:field 39 {:base-type :type/DateTime, :temporal-unit :month}]
+              [:expression "expression"]
+              [:field
+               33
+               {:base-type :type/Float
+                :binning {:strategy :num-bins, :min-value 0, :max-value 12, :num-bins 8, :bin-width 2}}]]
+             :columns []
+             :values [[:aggregation 0]]}
+            :pivot_table.column_widths {:leftHeaderWidths [141 99 80], :totalLeftHeaderWidths 320, :valueHeaderWidths {}}
+            :table.cell_column "expression"
+            :table.column_formatting
+            [{:columns ["expression" nil "TAX" "count"]
+              :type "single"
+              :operator "is-null"
+              :value 10
+              :color "#EF8C8C"
+              :highlight_row false
+              :id 0}]
+            :column_settings {"[\"ref\",[\"expression\",\"expression\"]]" {:number_style "currency"}}
+            :series_settings {:expression {:line.interpolate "step-after", :line.style "dotted"}}
+            :graph.dimensions ["CREATED_AT"]}
+           (mi/normalize-visualization-settings viz-settings)))))
+
 (deftest migrate-legacy-result-metadata-field-refs-test
   (testing "Migrations v47.00-027: update report_card.result_metadata legacy field refs"
     (impl/test-migrations ["v47.00-027"] [migrate!]