Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🤖 backported "Fixing static-viz exports of saved x-ray dashboards" #38418

Merged

Conversation

metabase-bot[bot]
Copy link
Contributor

@metabase-bot metabase-bot bot commented Feb 2, 2024

#38379

Important

Manual conflict resolution is required.
Checkout the branch and run ./backport.sh script. Force push your changes after cherry-picking.

@metabase-bot metabase-bot bot added the was-backported apply this to PRs that are themselves backports label Feb 2, 2024
* Fixing static-viz exports of saved x-ray dashboards

When dashboards were generated with x-rays, the `:visualization_settings` were not properly filled in. In particular, the `:graph.dimensions` and `:graph.metrics` were being populated from either the raw template values or not at all. The following namespace demonstrates this:

```clojure
(ns tickets.38350-broken-xray-export
  (:require
    [dev.render-png :as render-png]
    [metabase.models :refer [Card]]
    [metabase.test :as mt]))

(mt/dataset test-data
  (mt/with-temp [Card {bad-card-id :id}
                 {:display :row
                  :dataset_query {:database (mt/id)
                                  :type     :query
                                  :query    {:source-table (mt/id :orders)
                                             :aggregation  [[:count]]
                                             :breakout     [[:field
                                                             (mt/id :products :category)
                                                             {:source-field (mt/id :orders :product_id)}]]}}
                  :visualization_settings {:graph.series_labels [nil]
                                           :graph.dimensions [{:ProductCategoryMedium {}}]
                                           :graph.colors ["#EF8C8C"]
                                           :graph.metrics [nil]}}
                 Card {good-card-id :id}
                 {:display :row
                  :dataset_query {:database (mt/id)
                                  :type     :query
                                  :query    {:source-table (mt/id :orders)
                                             :aggregation  [[:count]]
                                             :breakout     [[:field
                                                             (mt/id :products :category)
                                                             {:base-type :type/Text
                                                              :source-field (mt/id :orders :product_id)}]]}}
                  :visualization_settings {:graph.dimensions ["CATEGORY"]
                                           :graph.metrics ["count"]}}]
    (render-png/render-card-to-png bad-card-id)
    (render-png/render-card-to-png good-card-id)))
```

The FE seemed to have some level of error checking and was robust enough to recover from these bad `:visualization_settings`.

The solution is to correctly populate these values. This required updating the `:dimension-name->field` value in each card to include mappings derived from both metrics and dimensions and then using the right logic in the `visualization-settings` function.

* Removed unneeded `->legacy-MBQL` on values in `:mbql/query` dispatch

(cherry picked from commit ee1581f)
@markbastian markbastian force-pushed the backport-ee1581fb6bec52adb4a0044230bec6ae4e51ad79 branch from 5c40011 to fd07115 Compare February 6, 2024 15:09
@markbastian markbastian enabled auto-merge (squash) February 6, 2024 15:11
Copy link

replay-io bot commented Feb 6, 2024

StatusIn Progress ↗︎ 53 / 54
Commit8ff88c8
Results
⚠️ 2 Flaky
2108 Passed

@markbastian markbastian merged commit dab12cf into release-x.48.x Feb 6, 2024
103 checks passed
@markbastian markbastian deleted the backport-ee1581fb6bec52adb4a0044230bec6ae4e51ad79 branch February 6, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
was-backported apply this to PRs that are themselves backports
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants