Correct Data Shape for Multi-Series Static Viz (#42730)

* Correct Data Shape for Multi-Series Static Viz

Fixes: #41787

During the echarts migration, some backend tests needed fixing. In this process, the shape of data (eg. list of cards
and their data results from execution) were altered.

This was done to fix failing tests and renders for single series cards (all Questions and many dashcards), but didn't
correctly handle a multi-series card.

This conditionally adds back the expected shape.

Perhaps in a follow on PR it's worth inspecting the implementation of the :javascript_visualization render method to
clean it up. Probably worth putting some Malli schemas around it too to make it clearer what shape we need.

* Make sure card/data maps going into static-viz are distinct

* Add a test util to render dashcards, add multiseries card test

We haven't made a 'unit' (perhaps more like a small integration test at this point) test for multi series dashcards
yet.

In most other cases, making sure a card (Question) renders properly is sufficient, but the multi series dashcards are
a special case. It's probably useufl to have a test or two, and the dashcards test util should hopefully help future
test writing a bit now too.

src/metabase/pulse/render/body.clj

@@ -538,16 +538,20 @@
 ;; This conditional is here to cover the case of trend charts in Alerts (not subscriptions). Alerts
 ;; exist on Questions and thus have no associated dashcard, which causes `pu/execute-multi-card` to fail.
 (mu/defmethod render :javascript_visualization :- formatter/RenderedPulseCard
-  [_chart-type render-type _timezone-id card dashcard _data]
+  [_chart-type render-type _timezone-id card dashcard data]
   (let [combined-cards-results (if dashcard
                                  (pu/execute-multi-card card dashcard)
                                  [(pu/execute-card {:creator_id (:creator_id card)} (:id card))])
-        cards-with-data        (map
-                                (comp
-                                 add-dashcard-timeline-events
-                                 (fn [c d] {:card c :data d}))
-                                (map :card combined-cards-results)
-                                (map #(get-in % [:result :data]) combined-cards-results))
+        cards-with-data        (m/distinct-by
+                                #(get-in % [:card :id])
+                                (map
+                                 (comp
+                                  add-dashcard-timeline-events
+                                  (fn [c d] {:card c :data d}))
+                                 (cond-> (map :card combined-cards-results)
+                                   dashcard (conj card))
+                                 (cond-> (map #(get-in % [:result :data]) combined-cards-results)
+                                   dashcard (conj data))))
         dashcard-viz-settings  (get dashcard :visualization_settings)
         {rendered-type :type content :content} (js-svg/javascript-visualization cards-with-data dashcard-viz-settings)]
     (case rendered-type

test/metabase/pulse/render/body_test.clj

@@ -753,6 +753,38 @@
               (is (= 1 (count axis-label-element)))
               (is (< 200 axis-y-transform)))))))))
 
+(deftest multiseries-dashcard-render-test
+  (testing "Multi-series dashcards render with every series. (#42730)"
+    (mt/dataset test-data
+      (let [q {:database (mt/id)
+               :type     :query
+               :query
+               {:source-table (mt/id :products)
+                :aggregation  [[:count]]
+                :breakout
+                [[:field (mt/id :products :category) {:base-type :type/Text}]]}}]
+        (mt/with-temp [:model/Card {card-a-id :id} {:display       :bar
+                                                    :dataset_query q}
+                       :model/Card {card-b-id :id} {:display       :bar
+                                                    :dataset_query q}
+                       :model/Dashboard {dash-id :id} {}
+                       :model/DashboardCard {dashcard-id :id} {:dashboard_id dash-id
+                                                               :card_id      card-a-id}
+                       :model/DashboardCardSeries _ {:dashboardcard_id dashcard-id
+                                                     :card_id          card-b-id}]
+          (let [card-doc               (render.tu/render-card-as-hickory card-a-id)
+                dashcard-doc           (render.tu/render-dashcard-as-hickory dashcard-id)
+                card-path-elements     (hik.s/select (hik.s/tag :path) card-doc)
+                card-paths-count       (count card-path-elements)
+                dashcard-path-elements (hik.s/select (hik.s/tag :path) dashcard-doc)
+                expected-dashcard-paths-count (+ 4 card-paths-count)]
+            ;; SVG Path elements are used to draw the bars in a bar graph.
+            ;; They are also used to create the axes lines, so we establish a count of a single card's path elements
+            ;; to compare against.
+            ;; Since we know that the Products sample data has 4 Product categories, we can reliably expect
+            ;; that Adding a series to the card that is identical to the first card will result in 4 more path elements.
+            (is (= expected-dashcard-paths-count (count dashcard-path-elements)))))))))
+
 (defn- render-card
   [render-type card data]
   (body/render render-type :attachment (pulse/defaulted-timezone card) card nil data))

test/metabase/pulse/render/test_util.clj

@@ -662,3 +662,26 @@
             hiccup/html
             hik/parse
             hik/as-hickory)))))
+
+(defn render-dashcard-as-hickory
+  "Render the dashcard with `dashcard-id` using the static-viz rendering pipeline as a hickory data structure.
+  Redefines some internal rendering functions to keep svg from being rendered into a png.
+  Functions from `hickory.select` can be used on the output of this function and are particularly useful for writing test assertions."
+  [dashcard-id]
+  (let [dashcard (t2/select-one :model/DashboardCard :id dashcard-id)
+        card     (t2/select-one :model/Card :id (:card_id dashcard))
+        viz      (or (:visualilzation_settings dashcard)
+                     (:visualilzation_settings card))
+        query    (qp.card/query-for-card card [] nil {:process-viz-settings? true} nil)
+        results  (qp/process-query (assoc query :viz-settings viz))]
+    (with-redefs [js-svg/svg-string->bytes       identity
+                  image-bundle/make-image-bundle (fn [_ s]
+                                                   {:image-src   s
+                                                    :render-type :inline})]
+      (let [content (-> (render/render-pulse-card :inline "UTC" card dashcard results)
+                        :content)]
+        (-> content
+            (edit-nodes img-node-with-svg? img-node->svg-node) ;; replace the :img tag with its parsed SVG.
+            hiccup/html
+            hik/parse
+            hik/as-hickory)))))