Skip to content
Snippets Groups Projects
Unverified Commit f8b533c9 authored by Mark Bastian's avatar Mark Bastian Committed by GitHub
Browse files

Renamed columns are displayed correctly in downloads (#18572) (#37016) 

* Renamed columns are displayed correctly in downloads (#18572)

When custom viz settings or metadata is applied to questions and models, the downloaded artifact labels do not always mirror what is displayed on the tabular view in Metabase. This happens for several reasons:

- The `:visualization_settings` were not applied to CSV and JSON downloads
- `update-card-viz-settings` in `metabase.query-processor.middleware.visualization-settings` did not correctly merge keys, resulting in dropped column names in Excel file
- The logic for reconciling column metadata in a result set and the keys in `:visualization_settings` of a query result is not straightforward

This PR is an attempt to move this consistency in the right direction without fixing all the challenges of field matching as discussed in [this thread](https://metaboat.slack.com/archives/CKZEMT1MJ/p1703091539541279) and [this issue](Duplicated columns in a source query cannot be distinguished #36185) as this sounds like a significant effort.

The following changes were made:

1. Update `metabase.query-processor.middleware.visualization-settings/update-card-viz-settings` to merge field settings, if present, into `column-viz-settings` without introducing new keys, especially ambiguous keys as follows:

```clojure
(defn- update-card-viz-settings
  "For each field, fetch its settings from the QP store, convert the settings into the normalized form
  for visualization settings, and then merge in the card-level column settings."
  [column-viz-settings field-ids]
  ;; Retrieve field-level settings
  (let [field-id->settings (reduce
                             (fn [m field-id]
                               (let [field-settings      (:settings (lib.metadata/field (qp.store/metadata-provider) field-id))
                                     norm-field-settings (normalize-field-settings field-id field-settings)]
                                 (assoc m field-id norm-field-settings)))
                             {}
                             field-ids)]
    ;; For each column viz setting, if there is a match on the field settings, merge it in,
    ;; with the column viz settings being the default in the event of conflicts.
    (reduce-kv
      (fn [coll {field-id ::mb.viz/field-id :as k} column-viz-setting]
        (assoc coll k (merge (get field-id->settings field-id {}) column-viz-setting)))
      {}
      column-viz-settings)))
```

The primary difference is rather than merging in new keys as `{::mb.viz/field-id field-id} -> field-settings` this code will merge settings found by field id with any `column-viz-setting` containing that field id as part of its key. The merge prefers `column-viz-settings`.

This should be an improvement over what already exists.

2. Move `column-titles` from `metabase.query-processor.streaming.xlsx` to `metabase.query-processor.streaming.common` and use this common function for CSV and JSON export names.

Note that the lifted `column-titles` has some erroneous logic:

- It relies on column `:name` or `:id` as unique identifiers -- they are not always unique.
- It expects the `col-settings` key format to be exactly `{::mb.viz/field-id id}` or `{::mb.viz/column-name name}` -- Sometimes these keys have additional keys. One change made to this function is normalizing `col-settings` to conform to the expected format by removing extra keys. While this isn't a perfect solution, it conforms with the intent of what already exists and is an improvement in the majority of cases.

The _right_ thing to do is _probably_ to use `metabase.lib.equality/find-matching-column` to match the column and settings keys, _but_ the `col-settings` keys are in a weird format that isn't really conducive to doing this. I'd rather we make an incremental step forward and try doing this in another effort.

This second change should not make anything that wasn't already problematic worse, while making the general case better.

* Updated `update-card-viz-settings` for unique fields case

When `update-card-viz-settings` is called, sometimes fields have ids that aren't part of the `column-viz-settings` map. For cases where they are part of the viz settings map, they are merged in, potentially more than once. For cases where the field ids aren't found in the column viz settings but do have settings, we add these in as new entries.

* Test for JSON keys

* Fixed broken unit test that had the wrong data shape.
parent 7a4ebb5a
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 321 additions and 74 deletions
src/metabase/query_processor/middleware/visualization_settings.clj
+ 24
− 6
View file @ f8b533c9
......@@ -17,12 +17,30 @@
"For each field, fetch its settings from the QP store, convert the settings into the normalized form
for visualization settings, and then merge in the card-level column settings."
[column-viz-settings field-ids]
(merge column-viz-settings
(into {} (for [field-id field-ids]
(let [field-settings (:settings (lib.metadata/field (qp.store/metadata-provider) field-id))
norm-field-settings (normalize-field-settings field-id field-settings)
col-settings (get column-viz-settings {::mb.viz/field-id field-id})]
[{::mb.viz/field-id field-id} (merge norm-field-settings col-settings)])))))
;; Retrieve field-level settings
(let [field-id->settings (reduce
(fn [m field-id]
(let [field-settings (:settings (lib.metadata/field (qp.store/metadata-provider) field-id))
norm-field-settings (normalize-field-settings field-id field-settings)]
(cond-> m
(seq norm-field-settings)
(assoc field-id norm-field-settings))))
{}
field-ids)
;; For each column viz setting, if there is a match on the field settings, merge it in,
;; with the column viz settings being the default in the event of conflicts.
merged-settings (reduce-kv
(fn [coll {field-id ::mb.viz/field-id :as k} column-viz-setting]
(assoc coll k (merge (get field-id->settings field-id {}) column-viz-setting)))
{}
column-viz-settings)
;; The field-ids that are in the merged settings
viz-field-ids (set (map ::mb.viz/field-id (keys merged-settings)))
;; Keep any field settings that aren't in the merged settings and have settings
distinct-field-settings (update-keys
(remove (comp viz-field-ids first) field-id->settings)
(fn [k] {::mb.viz/field-id k}))]
(merge merged-settings distinct-field-settings)))
(defn- viz-settings
"Pull viz settings from either the query map or the DB"
......
src/metabase/query_processor/streaming/common.clj
+ 51
− 0
View file @ f8b533c9
......@@ -2,8 +2,11 @@
"Shared util fns for various export (download) streaming formats."
(:require
[java-time.api :as t]
[metabase.public-settings :as public-settings]
[metabase.query-processor.store :as qp.store]
[metabase.query-processor.timezone :as qp.timezone]
[metabase.shared.models.visualization-settings :as mb.viz]
[metabase.shared.util.currency :as currency]
[metabase.util.date-2 :as u.date])
(:import
(clojure.lang ISeq)
......@@ -61,3 +64,51 @@
ZonedDateTime
(format-value [t]
(format-value (t/offset-date-time t))))
(defn merge-global-settings
"Merge format settings defined in the localization preferences into the format settings
for a single column."
[format-settings global-settings-key]
(let [global-settings (global-settings-key (public-settings/custom-formatting))
normalized (mb.viz/db->norm-column-settings-entries global-settings)]
(merge normalized format-settings)))
(defn currency-identifier
"Given the format settings for a currency column, returns the symbol, code or name for the
appropriate currency."
[format-settings]
(let [currency-code (::mb.viz/currency format-settings "USD")]
(condp = (::mb.viz/currency-style format-settings "symbol")
"symbol"
(if (currency/supports-symbol? currency-code)
(get-in currency/currency [(keyword currency-code) :symbol])
;; Fall back to using code if symbol isn't supported on the Metabase frontend
currency-code)
"code"
currency-code
"name"
(get-in currency/currency [(keyword currency-code) :name_plural]))))
(defn column-titles
"Generates the column titles that should be used in the export, taking into account viz settings."
[ordered-cols col-settings]
(for [col ordered-cols]
(let [id-or-name (or (and (:remapped_from col) (:fk_field_id col))
(:id col)
(:name col))
col-settings' (update-keys col-settings #(select-keys % [::mb.viz/field-id ::mb.viz/column-name]))
format-settings (or (get col-settings' {::mb.viz/field-id id-or-name})
(get col-settings' {::mb.viz/column-name id-or-name}))
is-currency? (or (isa? (:semantic_type col) :type/Currency)
(= (::mb.viz/number-style format-settings) "currency"))
merged-settings (if is-currency?
(merge-global-settings format-settings :type/Currency)
format-settings)
column-title (or (::mb.viz/column-title merged-settings)
(:display_name col)
(:name col))]
(if (and is-currency? (::mb.viz/currency-in-header merged-settings true))
(str column-title " (" (currency-identifier merged-settings) ")")
column-title))))
src/metabase/query_processor/streaming/csv.clj
+ 7
− 5
View file @ f8b533c9
......@@ -5,6 +5,7 @@
[metabase.formatter :as formatter]
[metabase.query-processor.streaming.common :as common]
[metabase.query-processor.streaming.interface :as qp.si]
[metabase.shared.models.visualization-settings :as mb.viz]
[metabase.util.date-2 :as u.date])
(:import
(java.io BufferedWriter OutputStream OutputStreamWriter)
......@@ -29,11 +30,12 @@
ordered-formatters (volatile! {})]
(reify qp.si/StreamingResultsWriter
(begin! [_ {{:keys [ordered-cols results_timezone]} :data} viz-settings]
(vreset! ordered-formatters (mapv (fn [col]
(formatter/create-formatter results_timezone col viz-settings))
ordered-cols))
(csv/write-csv writer [(map (some-fn :display_name :name) ordered-cols)])
(.flush writer))
(let [col-names (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings))]
(vreset! ordered-formatters (mapv (fn [col]
(formatter/create-formatter results_timezone col viz-settings))
ordered-cols))
(csv/write-csv writer [col-names])
(.flush writer)))
(write-row! [_ row _row-num _ {:keys [output-order]}]
(let [ordered-row (if output-order
......
src/metabase/query_processor/streaming/json.clj
+ 8
− 7
View file @ f8b533c9
......@@ -2,12 +2,13 @@
"Impls for JSON-based QP streaming response types. `:json` streams a simple array of maps as opposed to the full
response with all the metadata for `:api`."
(:require
[cheshire.core :as json]
[java-time.api :as t]
[metabase.formatter :as formatter]
[metabase.query-processor.streaming.common :as common]
[metabase.query-processor.streaming.interface :as qp.si]
[metabase.util.date-2 :as u.date])
[cheshire.core :as json]
[java-time.api :as t]
[metabase.formatter :as formatter]
[metabase.query-processor.streaming.common :as common]
[metabase.query-processor.streaming.interface :as qp.si]
[metabase.shared.models.visualization-settings :as mb.viz]
[metabase.util.date-2 :as u.date])
(:import
(java.io BufferedWriter OutputStream OutputStreamWriter)
(java.nio.charset StandardCharsets)))
......@@ -33,7 +34,7 @@
(begin! [_ {{:keys [ordered-cols results_timezone]} :data} viz-settings]
;; TODO -- wouldn't it make more sense if the JSON downloads used `:name` preferentially? Seeing how JSON is
;; probably going to be parsed programmatically
(vreset! col-names (mapv (some-fn :display_name :name) ordered-cols))
(vreset! col-names (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings)))
(vreset! ordered-formatters (mapv (fn [col]
(formatter/create-formatter results_timezone col viz-settings))
ordered-cols))
......
src/metabase/query_processor/streaming/xlsx.clj
+ 6
− 54
View file @ f8b533c9
......@@ -6,7 +6,6 @@
[java-time.api :as t]
[metabase.lib.schema.temporal-bucketing
:as lib.schema.temporal-bucketing]
[metabase.public-settings :as public-settings]
[metabase.query-processor.streaming.common :as common]
[metabase.query-processor.streaming.interface :as qp.si]
[metabase.shared.models.visualization-settings :as mb.viz]
......@@ -47,38 +46,12 @@
::mb.viz/time-enabled
::mb.viz/time-style})
(defn- merge-global-settings
"Merge format settings defined in the localization preferences into the format settings
for a single column."
[format-settings global-settings-key]
(let [global-settings (global-settings-key (public-settings/custom-formatting))
normalized (mb.viz/db->norm-column-settings-entries global-settings)]
(merge normalized format-settings)))
(defn- currency-identifier
"Given the format settings for a currency column, returns the symbol, code or name for the
appropriate currency."
[format-settings]
(let [currency-code (::mb.viz/currency format-settings "USD")]
(condp = (::mb.viz/currency-style format-settings "symbol")
"symbol"
(if (currency/supports-symbol? currency-code)
(get-in currency/currency [(keyword currency-code) :symbol])
;; Fall back to using code if symbol isn't not supported on the Metabase frontend
currency-code)
"code"
currency-code
"name"
(get-in currency/currency [(keyword currency-code) :name_plural]))))
(defn- currency-format-string
"Adds a currency to the base format string as either a suffix (for pluralized names) or
prefix (for symbols or codes)."
[base-string format-settings]
(let [currency-code (::mb.viz/currency format-settings "USD")
currency-identifier (currency-identifier format-settings)]
currency-identifier (common/currency-identifier format-settings)]
(condp = (::mb.viz/currency-style format-settings "symbol")
"symbol"
(if (currency/supports-symbol? currency-code)
......@@ -117,7 +90,7 @@
is-currency? (or (isa? semantic-type :type/Currency)
(= (::mb.viz/number-style format-settings) "currency"))
merged-settings (if is-currency?
(merge-global-settings format-settings :type/Currency)
(common/merge-global-settings format-settings :type/Currency)
format-settings)
base-string (if (= (::mb.viz/number-separators format-settings) ".")
;; Omit thousands separator if ommitted in the format settings. Otherwise ignore
......@@ -228,7 +201,7 @@
(datetime-format-string format-settings nil))
([format-settings unit]
(let [merged-settings (merge-global-settings format-settings :type/Temporal)]
(let [merged-settings (common/merge-global-settings format-settings :type/Temporal)]
(->> (date-format merged-settings unit)
(add-time-format merged-settings unit)))))
......@@ -254,8 +227,8 @@
(defn- default-format-strings
"Default strings to use for datetime and number fields if custom format settings are not set."
[]
{:datetime (datetime-format-string (merge-global-settings {} :type/Temporal))
:date (datetime-format-string (merge-global-settings {::mb.viz/time-enabled nil} :type/Temporal))
{:datetime (datetime-format-string (common/merge-global-settings {} :type/Temporal))
:date (datetime-format-string (common/merge-global-settings {::mb.viz/time-enabled nil} :type/Temporal))
;; Use a fixed format for time fields since time formatting isn't currently supported (#17357)
:time "h:mm am/pm"
:integer "#,##0"
......@@ -446,27 +419,6 @@
(set-cell! (.createCell ^SXSSFRow row ^Integer index) parsed-value id-or-name)))
row))
(defn- column-titles
"Generates the column titles that should be used in the export, taking into account viz settings."
[ordered-cols col-settings]
(for [col ordered-cols]
(let [id-or-name (or (and (:remapped_from col) (:fk_field_id col))
(:id col)
(:name col))
format-settings (or (get col-settings {::mb.viz/field-id id-or-name})
(get col-settings {::mb.viz/column-name id-or-name}))
is-currency? (or (isa? (:semantic_type col) :type/Currency)
(= (::mb.viz/number-style format-settings) "currency"))
merged-settings (if is-currency?
(merge-global-settings format-settings :type/Currency)
format-settings)
column-title (or (::mb.viz/column-title merged-settings)
(:display_name col)
(:name col))]
(if (and is-currency? (::mb.viz/currency-in-header merged-settings true))
(str column-title " (" (currency-identifier merged-settings) ")")
column-title))))
(def ^:dynamic *auto-sizing-threshold*
"The maximum number of rows we should use for auto-sizing. If this number is too large, exports
of large datasets will be prohibitively slow."
......@@ -507,7 +459,7 @@
(doseq [i (range (count ordered-cols))]
(.trackColumnForAutoSizing ^SXSSFSheet sheet i))
(setup-header-row! sheet (count ordered-cols))
(spreadsheet/add-row! sheet (column-titles ordered-cols col-settings)))
(spreadsheet/add-row! sheet (common/column-titles ordered-cols col-settings)))
(write-row! [_ row row-num ordered-cols {:keys [output-order] :as viz-settings}]
(let [ordered-row (if output-order
......
test/metabase/api/card_test.clj
+ 87
− 1
View file @ f8b533c9
......@@ -16,7 +16,8 @@
[metabase.events.view-log-test :as view-log-test]
[metabase.http-client :as client]
[metabase.models
:refer [CardBookmark
:refer [Card
CardBookmark
Collection
Dashboard
Database
......@@ -1826,6 +1827,91 @@
(mt/user-http-request :rasta :post 200 (format "card/%d/query/json" (u/the-id card))
:parameters encoded-params)))))))
(deftest renamed-column-names-are-applied-to-json-test
(testing "JSON downloads should have the same columns as displayed in Metabase (#18572)"
(mt/with-temporary-setting-values [custom-formatting nil]
(let [query {:source-table (mt/id :orders)
:fields [[:field (mt/id :orders :id) {:base-type :type/BigInteger}]
[:field (mt/id :orders :tax) {:base-type :type/Float}]
[:field (mt/id :orders :total) {:base-type :type/Float}]
[:field (mt/id :orders :discount) {:base-type :type/Float}]
[:field (mt/id :orders :quantity) {:base-type :type/Integer}]
[:expression "Tax Rate"]],
:expressions {"Tax Rate" [:/
[:field (mt/id :orders :tax) {:base-type :type/Float}]
[:field (mt/id :orders :total) {:base-type :type/Float}]]},
:limit 10}
viz-settings {:table.cell_column "TAX",
:column_settings {(format "[\"ref\",[\"field\",%s,null]]" (mt/id :orders :id))
{:column_title "THE_ID"}
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Float\"}]]"
(mt/id :orders :tax))
{:column_title "ORDER TAX"}
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Float\"}]]"
(mt/id :orders :total))
{:column_title "Total Amount"},
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Float\"}]]"
(mt/id :orders :discount))
{:column_title "Discount Applied"}
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Integer\"}]]"
(mt/id :orders :quantity))
{:column_title "Amount Ordered"}
"[\"ref\",[\"expression\",\"Tax Rate\"]]"
{:column_title "Effective Tax Rate"}}}]
(t2.with-temp/with-temp [Card {base-card-id :id} {:dataset_query {:database (mt/id)
:type :query
:query query}
:visualization_settings viz-settings}
Card {model-card-id :id
model-metadata :result_metadata} {:dataset true
:dataset_query {:database (mt/id)
:type :query
:query {:source-table
(format "card__%s" base-card-id)}}}
Card {meta-model-card-id :id} {:dataset true
:dataset_query {:database (mt/id)
:type :query
:query {:source-table
(format "card__%s" model-card-id)}}
:result_metadata (mapv
(fn [{column-name :name :as col}]
(cond-> col
(= "DISCOUNT" column-name)
(assoc :display_name "Amount of Discount")
(= "TOTAL" column-name)
(assoc :display_name "Grand Total")
(= "QUANTITY" column-name)
(assoc :display_name "N")))
model-metadata)}
Card {question-card-id :id} {:dataset_query {:database (mt/id)
:type :query
:query {:source-table
(format "card__%s" meta-model-card-id)}}
:visualization_settings {:table.pivot_column "DISCOUNT",
:table.cell_column "TAX",
:column_settings {(format
"[\"ref\",[\"field\",%s,{\"base-type\":\"type/Integer\"}]]"
(mt/id :orders :quantity))
{:column_title "Count"}
(format
"[\"ref\",[\"field\",%s,{\"base-type\":\"type/BigInteger\"}]]"
(mt/id :orders :id))
{:column_title "IDENTIFIER"}}}}]
(letfn [(col-names [card-id]
(->> (mt/user-http-request :rasta :post 200 (format "card/%d/query/json" card-id)) first keys (map name) set))]
(testing "Renaming columns via viz settings is correctly applied to the CSV export"
(is (= #{"THE_ID" "ORDER TAX" "Total Amount" "Discount Applied ($)" "Amount Ordered" "Effective Tax Rate"}
(col-names base-card-id))))
(testing "A question derived from another question does not bring forward any renames"
(is (= #{"ID" "Tax" "Total" "Discount ($)" "Quantity" "Tax Rate"}
(col-names model-card-id))))
(testing "A model with custom metadata shows the renamed metadata columns"
(is (= #{"ID" "Tax" "Grand Total" "Amount of Discount ($)" "N" "Tax Rate"}
(col-names meta-model-card-id))))
(testing "A question based on a model retains the curated metadata column names but overrides these with any existing visualization_settings"
(is (= #{"IDENTIFIER" "Tax" "Grand Total" "Amount of Discount ($)" "Count" "Tax Rate"}
(col-names question-card-id))))))))))
(defn- parse-xlsx-results [results]
(->> results
ByteArrayInputStream.
......
test/metabase/pulse/pulse_integration_test.clj
+ 137
− 0
View file @ f8b533c9
......@@ -422,3 +422,140 @@
(testing "Setting time-enabled to nil for a time column just returns an empty string"
(is (= ""
(metamodel-results "Example Time")))))))))
(deftest renamed-column-names-are-applied-test
(testing "CSV attachments should have the same columns as displayed in Metabase (#18572)"
(mt/with-temporary-setting-values [custom-formatting nil]
(let [query {:source-table (mt/id :orders)
:fields [[:field (mt/id :orders :id) {:base-type :type/BigInteger}]
[:field (mt/id :orders :tax) {:base-type :type/Float}]
[:field (mt/id :orders :total) {:base-type :type/Float}]
[:field (mt/id :orders :discount) {:base-type :type/Float}]
[:field (mt/id :orders :quantity) {:base-type :type/Integer}]
[:expression "Tax Rate"]],
:expressions {"Tax Rate" [:/
[:field (mt/id :orders :tax) {:base-type :type/Float}]
[:field (mt/id :orders :total) {:base-type :type/Float}]]},
:limit 10}
viz-settings {:table.cell_column "TAX",
:column_settings {(format "[\"ref\",[\"field\",%s,null]]" (mt/id :orders :id))
{:column_title "THE_ID"}
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Float\"}]]"
(mt/id :orders :tax))
{:column_title "ORDER TAX"}
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Float\"}]]"
(mt/id :orders :total))
{:column_title "Total Amount"},
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Float\"}]]"
(mt/id :orders :discount))
{:column_title "Discount Applied"}
(format "[\"ref\",[\"field\",%s,{\"base-type\":\"type/Integer\"}]]"
(mt/id :orders :quantity))
{:column_title "Amount Ordered"}
"[\"ref\",[\"expression\",\"Tax Rate\"]]"
{:column_title "Effective Tax Rate"}}}]
(t2.with-temp/with-temp [Card {base-card-name :name
base-card-id :id} {:name "RENAMED"
:dataset_query {:database (mt/id)
:type :query
:query query}
:visualization_settings viz-settings}
Card {model-card-name :name
model-card-id :id
model-metadata :result_metadata} {:name "MODEL"
:dataset true
:dataset_query {:database (mt/id)
:type :query
:query {:source-table
(format "card__%s" base-card-id)}}}
Card {meta-model-card-name :name
meta-model-card-id :id} {:name "MODEL_WITH_META"
:dataset true
:dataset_query {:database (mt/id)
:type :query
:query {:source-table
(format "card__%s" model-card-id)}}
:result_metadata (mapv
(fn [{column-name :name :as col}]
(cond-> col
(= "DISCOUNT" column-name)
(assoc :display_name "Amount of Discount")
(= "TOTAL" column-name)
(assoc :display_name "Grand Total")
(= "QUANTITY" column-name)
(assoc :display_name "N")))
model-metadata)}
Card {question-card-name :name
question-card-id :id} {:name "FINAL_QUESTION"
:dataset_query {:database (mt/id)
:type :query
:query {:source-table
(format "card__%s" meta-model-card-id)}}
:visualization_settings {:table.pivot_column "DISCOUNT",
:table.cell_column "TAX",
:column_settings {(format
"[\"ref\",[\"field\",%s,{\"base-type\":\"type/Integer\"}]]"
(mt/id :orders :quantity))
{:column_title "Count"}
(format
"[\"ref\",[\"field\",%s,{\"base-type\":\"type/BigInteger\"}]]"
(mt/id :orders :id))
{:column_title "IDENTIFIER"}}}}
Dashboard {dash-id :id} {:name "The Dashboard"}
DashboardCard {base-dash-card-id :id} {:dashboard_id dash-id
:card_id base-card-id}
DashboardCard {model-dash-card-id :id} {:dashboard_id dash-id
:card_id model-card-id}
DashboardCard {meta-model-dash-card-id :id} {:dashboard_id dash-id
:card_id meta-model-card-id}
DashboardCard {question-dash-card-id :id} {:dashboard_id dash-id
:card_id question-card-id}
Pulse {pulse-id :id
:as pulse} {:name "Consistent Column Names"}
PulseCard _ {:pulse_id pulse-id
:card_id base-card-id
:dashboard_card_id base-dash-card-id
:include_csv true}
PulseCard _ {:pulse_id pulse-id
:card_id model-card-id
:dashboard_card_id model-dash-card-id
:include_csv true}
PulseCard _ {:pulse_id pulse-id
:card_id meta-model-card-id
:dashboard_card_id meta-model-dash-card-id
:include_csv true}
PulseCard _ {:pulse_id pulse-id
:card_id question-card-id
:dashboard_card_id question-dash-card-id
:include_csv true}
PulseChannel {pulse-channel-id :id} {:channel_type :email
:pulse_id pulse-id
:enabled true}
PulseChannelRecipient _ {:pulse_channel_id pulse-channel-id
:user_id (mt/user->id :rasta)}]
(let [attachment-name->cols (mt/with-fake-inbox
(with-redefs [email/bcc-enabled? (constantly false)]
(mt/with-test-user nil
(metabase.pulse/send-pulse! pulse)))
(->>
(get-in @mt/inbox ["rasta@metabase.com" 0 :body])
(keep
(fn [{:keys [type content-type file-name content]}]
(when (and
(= :attachment type)
(= "text/csv" content-type))
[file-name
(first (csv/read-csv (slurp content)))])))
(into {})))]
(testing "Renaming columns via viz settings is correctly applied to the CSV export"
(is (= ["THE_ID" "ORDER TAX" "Total Amount" "Discount Applied ($)" "Amount Ordered" "Effective Tax Rate"]
(attachment-name->cols (format "%s.csv" base-card-name)))))
(testing "A question derived from another question does not bring forward any renames"
(is (= ["ID" "Tax" "Total" "Discount ($)" "Quantity" "Tax Rate"]
(attachment-name->cols (format "%s.csv" model-card-name)))))
(testing "A model with custom metadata shows the renamed metadata columns"
(is (= ["ID" "Tax" "Grand Total" "Amount of Discount ($)" "N" "Tax Rate"]
(attachment-name->cols (format "%s.csv" meta-model-card-name)))))
(testing "A question based on a model retains the curated metadata column names but overrides these with any existing visualization_settings"
(is (= ["IDENTIFIER" "Tax" "Grand Total" "Amount of Discount ($)" "Count" "Tax Rate"]
(attachment-name->cols (format "%s.csv" question-card-name)))))))))))
test/metabase/query_processor/streaming/xlsx_test.clj
+ 1
− 1
View file @ f8b533c9
......@@ -485,7 +485,7 @@
;; Dollar symbol is included by default if semantic type of column derives from :type/Currency
(is (= ["Col ($)"]
(first (xlsx-export [{:id 0, :name "Col", :semantic_type :type/Cost}]
{::mb.viz/column-settings {::mb.viz/field-id 0}}
{::mb.viz/column-settings {{::mb.viz/field-id 0} {}}}
[]))))
;; Currency code is used if requested in viz settings
(is (= ["Col (USD)"]
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment