[PDFs] If a Visualizer card has a name override, use it (#77033)
Fixes UXW-4705. Also refactors the duplicated logic from two other places in the subscriptions rendering into a new `render.util` helper.
This commit is contained in:
committed by
GitHub
parent
851cb65a11
commit
2a8b0fdefe
@@ -1,6 +1,7 @@
|
||||
(ns metabase.channel.email.result-attachment
|
||||
(:require
|
||||
[clojure.java.io :as io]
|
||||
[metabase.channel.render.util :as render.util]
|
||||
[metabase.driver :as driver]
|
||||
[metabase.driver.util :as driver.u]
|
||||
[metabase.lib.schema.id :as lib.schema.id]
|
||||
@@ -92,7 +93,7 @@
|
||||
(defn result-attachment
|
||||
"Create result attachments for an email. `creator-id` is the subscription creator, whose download
|
||||
permissions gate the attachment at send time."
|
||||
[{{original-card-name :name format-rows :format_rows pivot-results :pivot_results :as card} :card
|
||||
[{{format-rows :format_rows pivot-results :pivot_results :as card} :card
|
||||
dashcard :dashcard
|
||||
result :result
|
||||
:as part}
|
||||
@@ -102,9 +103,9 @@
|
||||
(not= (perms/download-perms-level (:dataset_query card) creator-id) :no))
|
||||
(let [maybe-realize-data-rows (requiring-resolve 'metabase.channel.shared/maybe-realize-data-rows)
|
||||
result (:result (maybe-realize-data-rows part))
|
||||
visualizer-title (when (and dashcard (get-in dashcard [:visualization_settings :visualization]))
|
||||
(not-empty (get-in dashcard [:visualization_settings :visualization :settings :card.title])))
|
||||
filename-prefix (or visualizer-title original-card-name)]
|
||||
;; the dashboard-level title override (incl. the visualizer's) names the attachment, matching the card's
|
||||
;; displayed title; falls back to the card's own name (see [[render.util/dashcard-title]]).
|
||||
filename-prefix (render.util/dashcard-title card dashcard)]
|
||||
(when-not (:render/too-large? result)
|
||||
(->>
|
||||
[(when-let [temp-file (and (:include_csv card)
|
||||
|
||||
@@ -45,9 +45,7 @@
|
||||
(mu/defn- make-title-if-needed :- [:maybe ::body/RenderedPartCard]
|
||||
[render-type card dashcard options :- [:maybe ::options]]
|
||||
(when (:channel.render/include-title? options)
|
||||
(let [card-name (or (-> dashcard :visualization_settings :visualization :settings :card.title)
|
||||
(-> dashcard :visualization_settings :card.title)
|
||||
(-> card :name))
|
||||
(let [card-name (render.util/dashcard-title card dashcard)
|
||||
image-bundle (when (:channel.render/include-buttons? options)
|
||||
(image-bundle/external-link-image-bundle render-type))
|
||||
title-href (if dashcard
|
||||
|
||||
@@ -588,10 +588,6 @@
|
||||
(* (double px)
|
||||
(/ 72.0 common/dpi)))
|
||||
|
||||
(defn- card-title [card dashcard]
|
||||
(or (get-in dashcard [:visualization_settings :card.title])
|
||||
(:name card)))
|
||||
|
||||
(defn- effective-display
|
||||
"The display type the card actually renders as. A visualizer dashcard overrides the underlying card's `:display`
|
||||
(e.g. a smartscalar card shown as a `bar`), so prefer the visualizer display when present. This is the same
|
||||
@@ -1085,7 +1081,7 @@
|
||||
{:keys [card dashcard inline-params]
|
||||
{:keys [data]} :result
|
||||
:as part} (channel.shared/maybe-realize-data-rows part)
|
||||
title (card-title card dashcard)
|
||||
title (render.util/dashcard-title card dashcard)
|
||||
title-h (typeset/text-block-height (font/face :bold) common/chart-title-pt cell-w (* 0.5 cell-h) title)
|
||||
;; Inline parameters (filters attached to this card) render between the title and the chart body,
|
||||
;; like the email subscription does.
|
||||
|
||||
@@ -52,6 +52,16 @@
|
||||
[card dashcard]
|
||||
(merge (:visualization_settings card) (:visualization_settings dashcard)))
|
||||
|
||||
(defn dashcard-title
|
||||
"The title to display for `card` on `dashcard`: a non-blank dashboard-level `card.title` override wins over the
|
||||
card's own name. The override lives at the visualizer's nested location (`[:visualization :settings :card.title]`)
|
||||
for visualizer dashcards, otherwise directly on the dashcard's viz settings. Shared by the PDF, email, and Slack
|
||||
renderers (and the attachment-filename logic) so they all resolve titles the same way."
|
||||
[card dashcard]
|
||||
(or (not-empty (get-in dashcard [:visualization_settings :visualization :settings :card.title]))
|
||||
(not-empty (get-in dashcard [:visualization_settings :card.title]))
|
||||
(:name card)))
|
||||
|
||||
(defn viz-setting
|
||||
"Look up the `map.*`-style setting `k` (a string) in `viz-settings`, tolerating either string or keyword
|
||||
keys — production stores viz-settings keys as keywords-with-dots, but they can also arrive as strings."
|
||||
|
||||
@@ -3,6 +3,29 @@
|
||||
[clojure.test :refer :all]
|
||||
[metabase.channel.render.util :as render-util]))
|
||||
|
||||
(deftest ^:parallel dashcard-title-test
|
||||
(testing "dashcard-title resolves a dashboard-level title override over the card name (UXW-4705)"
|
||||
(let [card {:name "Card Name"}]
|
||||
(testing "a regular dashcard title override wins over the card name"
|
||||
(is (= "Dashboard Title"
|
||||
(render-util/dashcard-title card {:visualization_settings {:card.title "Dashboard Title"}}))))
|
||||
(testing "a visualizer dashcard's nested title override is respected"
|
||||
(is (= "Visualizer Title"
|
||||
(render-util/dashcard-title card {:visualization_settings {:visualization {:settings {:card.title "Visualizer Title"}}}}))))
|
||||
(testing "the visualizer override takes precedence over a top-level one"
|
||||
(is (= "Visualizer Title"
|
||||
(render-util/dashcard-title card {:visualization_settings {:card.title "Dashboard Title"
|
||||
:visualization {:settings {:card.title "Visualizer Title"}}}}))))
|
||||
(testing "a blank override is ignored (falls through)"
|
||||
(is (= "Dashboard Title"
|
||||
(render-util/dashcard-title card {:visualization_settings {:card.title "Dashboard Title"
|
||||
:visualization {:settings {:card.title ""}}}})))
|
||||
(is (= "Card Name"
|
||||
(render-util/dashcard-title card {:visualization_settings {:card.title ""}}))))
|
||||
(testing "with no override, falls back to the card's own name"
|
||||
(is (= "Card Name" (render-util/dashcard-title card {:visualization_settings {}})))
|
||||
(is (= "Card Name" (render-util/dashcard-title card {})))))))
|
||||
|
||||
;; Test data and expected results for scalar funnel visualization
|
||||
(def scalar-funnel-definition
|
||||
{:display "funnel"
|
||||
|
||||
Reference in New Issue
Block a user