Skip to content

Commit fd77eb3

Browse files
🤖 backported "Add missing dimensions in underlying-records drill-thru" (metabase#75939)
Add missing dimensions in underlying-records drill-thru (metabase#75048) Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
1 parent acf7aac commit fd77eb3

2 files changed

Lines changed: 71 additions & 10 deletions

File tree

‎src/metabase/lib/drill_thru.cljc‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
(ns metabase.lib.drill-thru
2-
(:refer-clojure :exclude [select-keys empty? not-empty #?(:clj for)])
2+
(:refer-clojure :exclude [select-keys not-empty #?(:clj for)])
33
(:require
44
[metabase.lib.drill-thru.automatic-insights :as lib.drill-thru.automatic-insights]
55
[metabase.lib.drill-thru.column-extract :as lib.drill-thru.column-extract]
@@ -32,7 +32,7 @@
3232
[metabase.util :as u]
3333
[metabase.util.log :as log]
3434
[metabase.util.malli :as mu]
35-
[metabase.util.performance :refer [select-keys empty? not-empty #?(:clj for)]]))
35+
[metabase.util.performance :refer [select-keys not-empty #?(:clj for)]]))
3636

3737
(comment
3838
lib.drill-thru.fk-details/keep-me
@@ -83,16 +83,23 @@
8383
(update :value lib.drill-thru.common/js->drill-value)))))
8484

8585
(mu/defn- context-with-dimensions-or-row-dimensions :- ::lib.schema.drill-thru/context
86-
"Return an updated `context` with either the existing `dimensions` or dimensions constructed from the `row`."
86+
"Return an updated `context`, supplementing `dimensions` from breakout-sourced `row` entries.
87+
88+
If no dimensions were provided but the underlying column comes from an aggregation, construct the dimensions from
89+
the row data. This is needed in cases where the frontend normally provides dimensions, but will not if it cannot
90+
determine that the clicked table cell was from an \"underlying\" aggregation-sourced column. See, e.g. the comment
91+
in `getTableCellClickedObject` in `table.js`.
92+
93+
If dimensions were provided but are missing a breakout-sourced entry that the row has (e.g. a chart whose viz
94+
settings only include one of several breakouts as a series breakout — see #73803), append the missing entries so
95+
every breakout produced by an earlier stage contributes a filter to the drill output."
8796
[query :- ::lib.schema/query
8897
{:keys [column dimensions row] :as context} :- ::lib.schema.drill-thru/context]
89-
;; If no dimensions were provided but the underlying column comes from an aggregation, then construct the dimensions
90-
;; from the row data. This is needed in cases where the frontend normally provides dimensions, but will not if it
91-
;; cannot determine that the clicked table cell was from an "underlying" aggregation-sourced column. See, e.g. the
92-
;; comment in getTableCellClickedObject in table.js.
93-
(let [row-dimensions (lib.drill-thru.common/dimensions-from-breakout-columns query column row)]
94-
(if (and (empty? dimensions) (seq row-dimensions))
95-
(assoc context :dimensions row-dimensions)
98+
(let [row-dimensions (lib.drill-thru.common/dimensions-from-breakout-columns query column row)
99+
existing-columns (into #{} (keep :column) dimensions)
100+
missing-from-row (remove (comp existing-columns :column) row-dimensions)]
101+
(if (seq missing-from-row)
102+
(update context :dimensions #(into (vec %) missing-from-row))
96103
context)))
97104

98105
(mu/defn available-drill-thrus :- [:sequential [:ref ::lib.schema.drill-thru/drill-thru]]

‎test/metabase/lib/drill_thru/underlying_records_test.cljc‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,3 +919,57 @@
919919
:breakout (symbol "nil #_\"key is not present.\"")
920920
:fields (symbol "nil #_\"key is not present.\"")}]}
921921
second-result)))))))
922+
923+
(deftest ^:parallel partial-dimensions-fill-from-row-test
924+
(testing "underlying-records drill fills in missing breakout-sourced row entries when the
925+
FE provides only some of the dimensions (#73803)"
926+
;; Models a card where stage 1 has agg+breakouts (count by created-at:month, state); stage 2
927+
;; adds a filter `state = "AK"`. On the FE the scatter only puts CREATED_AT in graph.dimensions,
928+
;; so the click context's :dimensions has just CREATED_AT — STATE is present in :row but, at
929+
;; the last stage, its column metadata shows :source/previous-stage / :source :fields rather
930+
;; than a breakout. The drill must still apply STATE = clicked-value to the underlying records.
931+
(let [base (-> (lib/query meta/metadata-provider (meta/table-metadata :people))
932+
(lib/aggregate (lib/count))
933+
(lib/breakout (-> (meta/field-metadata :people :created-at)
934+
(lib/with-temporal-bucket :month)))
935+
(lib/breakout (meta/field-metadata :people :state))
936+
lib/append-stage)
937+
base-cols (lib/returned-columns base)
938+
created-at (lib.tu.notebook/find-col-with-spec
939+
base base-cols {} {:display-name "Created At: Month"})
940+
state-col (lib.tu.notebook/find-col-with-spec
941+
base base-cols {} {:display-name "State"})
942+
count-col (lib.tu.notebook/find-col-with-spec
943+
base base-cols {} {:display-name "Count"})
944+
query (lib/filter base (lib/= state-col "AK"))
945+
context {:column count-col
946+
:column-ref (lib/ref count-col)
947+
:value 4
948+
:row [{:column created-at
949+
:column-ref (lib/ref created-at)
950+
:value "2026-07-01T00:00:00Z"}
951+
{:column state-col
952+
:column-ref (lib/ref state-col)
953+
:value "AK"}
954+
{:column count-col
955+
:column-ref (lib/ref count-col)
956+
:value 4}]
957+
:dimensions [{:column created-at
958+
:column-ref (lib/ref created-at)
959+
:value "2026-07-01T00:00:00Z"}]}
960+
drill (m/find-first #(= (:type %) :drill-thru/underlying-records)
961+
(lib/available-drill-thrus query context))
962+
_ (is (some? drill))
963+
result (lib/drill-thru query drill)]
964+
(is (=? {:stages [{:source-table (meta/id :people)
965+
:filters [[:between {}
966+
[:field {:temporal-unit :month} (meta/id :people :created-at)]
967+
string?
968+
string?]
969+
[:= {}
970+
[:field {} (meta/id :people :state)]
971+
"AK"]]
972+
:aggregation (symbol "nil #_\"key is not present.\"")
973+
:breakout (symbol "nil #_\"key is not present.\"")
974+
:fields (symbol "nil #_\"key is not present.\"")}]}
975+
result)))))

0 commit comments

Comments
 (0)