Skip to content
Snippets Groups Projects
Unverified Commit 5f4549b3 authored by Noah Moss's avatar Noah Moss Committed by GitHub
Browse files

Check for and ignore broken `table-columns` when determining export column order (#19530)

parent f27ab0ec
No related branches found
No related tags found
No related merge requests found
...@@ -34,13 +34,24 @@ ...@@ -34,13 +34,24 @@
cols cols
(mbql.u/uniquify-names (map :name cols)))) (mbql.u/uniquify-names (map :name cols))))
(defn- validate-table-columms
"Validate that all of the field refs in `table-columns` correspond to actual columns in `cols`, if `cols` contains
field refs. Returns `nil` if any do not, so that we fall back to using `cols` directly for the export (#19465).
Otherwise returns `table-columns`."
[table-columns cols]
(let [col-field-refs (set (remove nil? (map :field_ref cols)))]
;; If there are no field refs in `cols` (e.g. for native queries), we should use `table-columns` as-is
(when (or (empty? col-field-refs)
(every? (fn [table-col] (col-field-refs (::mb.viz/table-column-field-ref table-col))) table-columns))
table-columns)))
(defn- export-column-order (defn- export-column-order
"For each entry in `table-columns` that is enabled, finds the index of the corresponding "For each entry in `table-columns` that is enabled, finds the index of the corresponding
entry in `cols` by name or id. If a col has been remapped, uses the index of the new column. entry in `cols` by name or id. If a col has been remapped, uses the index of the new column.
The resulting list of indices determines the order of column names and data in exports." The resulting list of indices determines the order of column names and data in exports."
[cols table-columns] [cols table-columns]
(let [table-columns' (or table-columns (let [table-columns' (or (validate-table-columms table-columns cols)
;; If table-columns is not provided (e.g. for saved cards), we can construct a fake one ;; If table-columns is not provided (e.g. for saved cards), we can construct a fake one
;; that retains the original column ordering in `cols` ;; that retains the original column ordering in `cols`
(for [col cols] (for [col cols]
......
...@@ -470,12 +470,13 @@ ...@@ -470,12 +470,13 @@
{:id 1, :name "Col2", :remapped_from "Col1", :field_ref ["field" 1 nil]}] {:id 1, :name "Col2", :remapped_from "Col1", :field_ref ["field" 1 nil]}]
[{::mb.viz/table-column-field-ref ["field" 0 nil], ::mb.viz/table-column-enabled true}])))) [{::mb.viz/table-column-field-ref ["field" 0 nil], ::mb.viz/table-column-enabled true}]))))
(testing "entries in table-columns without corresponding entries in cols are ignored" (testing "if table-columns contains a column without a corresponding entry in cols, table-columns is ignored and
cols is used as the source of truth for column order (#19465)"
(is (= [0] (is (= [0]
(@#'qp.streaming/export-column-order (@#'qp.streaming/export-column-order
[{:id 0, :name "Col1" :field_ref [:field 0 nil]}] [{:id 0, :name "Col1" :field_ref [:field 0 nil]}]
[{::mb.viz/table-column-field-ref [:field 0 nil], ::mb.viz/table-column-enabled true} [{::mb.viz/table-column-field-ref [:field 1 nil], ::mb.viz/table-column-enabled true}
{::mb.viz/table-column-field-ref [:field 1 nil], ::mb.viz/table-column-enabled true}])))) {::mb.viz/table-column-field-ref [:field 2 nil], ::mb.viz/table-column-enabled true}]))))
(testing "if table-columns is nil, original order of cols is used" (testing "if table-columns is nil, original order of cols is used"
(is (= [0 1] (is (= [0 1]
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment