Skip to content
Snippets Groups Projects
Unverified Commit 269e19d6 authored by metamben's avatar metamben Committed by GitHub
Browse files

Enable column-filter, disable distribution and summarize-column drills for... 

Enable column-filter, disable distribution and summarize-column drills for aggregated columns (#34681)
parent 1166d8f0
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
frontend/src/metabase-lib/drills.unit.spec.ts
+ 65
− 92
View file @ 269e19d6
......@@ -530,58 +530,51 @@ describe("availableDrillThrus", () => {
// },
// ],
// },
// FIXME for some reason the results for aggregated query are not correct (metabase#34223, metabase#34341)
// We expect column-filter and sort drills, but get distribution and summarize-column
// {
// clickType: "header",
// queryType: "aggregated",
// columnName: "count",
// expectedDrills: [
// {
// initialOp: expect.objectContaining({ short: "=" }),
// type: "drill-thru/column-filter",
// },
// {
// directions: ["asc", "desc"],
// type: "drill-thru/sort",
// },
// ],
// },
// FIXME for some reason the results for aggregated query are not correct (metabase#34223, metabase#34341)
// We expect column-filter and sort drills, but get distribution and summarize-column
// {
// clickType: "header",
// queryType: "aggregated",
// columnName: "PRODUCT_ID",
// expectedDrills: [
// {
// initialOp: expect.objectContaining({ short: "=" }),
// type: "drill-thru/column-filter",
// },
// {
// directions: ["asc", "desc"],
// type: "drill-thru/sort",
// },
// ],
// },
// FIXME for some reason the results for aggregated query are not correct (metabase#34223, metabase#34341)
// We expect column-filter and sort drills, but get distribution and summarize-column
// {
// clickType: "header",
// queryType: "aggregated",
// columnName: "CREATED_AT",
// expectedDrills: [
// {
// initialOp: expect.objectContaining({ short: "=" }),
// type: "drill-thru/column-filter",
// },
// {
// directions: ["asc", "desc"],
// type: "drill-thru/sort",
// },
// ],
// },
{
clickType: "header",
queryType: "aggregated",
columnName: "count",
expectedDrills: [
{
initialOp: expect.objectContaining({ short: "=" }),
type: "drill-thru/column-filter",
},
{
directions: ["asc", "desc"],
type: "drill-thru/sort",
},
],
},
{
clickType: "header",
queryType: "aggregated",
columnName: "PRODUCT_ID",
expectedDrills: [
{
initialOp: expect.objectContaining({ short: "=" }),
type: "drill-thru/column-filter",
},
{
directions: ["asc", "desc"],
type: "drill-thru/sort",
},
],
},
{
clickType: "header",
queryType: "aggregated",
columnName: "CREATED_AT",
expectedDrills: [
{
initialOp: null,
type: "drill-thru/column-filter",
},
{
directions: ["asc", "desc"],
type: "drill-thru/sort",
},
],
},
])(
"should return correct drills for $columnName $clickType in $queryType query",
({
......@@ -864,28 +857,26 @@ describe("availableDrillThrus", () => {
initialOp: null,
},
},
// FIXME "column-filter" should be available for aggregated query metric column (metabase#34223)
// {
// drillType: "drill-thru/column-filter",
// clickType: "header",
// queryType: "aggregated",
// columnName: "count",
// expectedParameters: {
// type: "drill-thru/column-filter",
// initialOp: expect.objectContaining({ short: "=" }),
// },
// },
// FIXME "column-filter" should be available for aggregated query metric column (metabase#34223)
// {
// drillType: "drill-thru/column-filter",
// clickType: "header",
// queryType: "aggregated",
// columnName: "max",
// expectedParameters: {
// type: "drill-thru/column-filter",
// initialOp: expect.objectContaining({ short: "=" }),
// },
// },
{
drillType: "drill-thru/column-filter",
clickType: "header",
queryType: "aggregated",
columnName: "count",
expectedParameters: {
type: "drill-thru/column-filter",
initialOp: expect.objectContaining({ short: "=" }),
},
},
{
drillType: "drill-thru/column-filter",
clickType: "header",
queryType: "aggregated",
columnName: "max",
expectedParameters: {
type: "drill-thru/column-filter",
initialOp: expect.objectContaining({ short: "=" }),
},
},
// endregion
// region --- drill-thru/summarize-column
......@@ -969,24 +960,6 @@ describe("availableDrillThrus", () => {
type: "drill-thru/distribution",
},
},
{
drillType: "drill-thru/distribution",
clickType: "header",
queryType: "aggregated",
columnName: "PRODUCT_ID",
expectedParameters: {
type: "drill-thru/distribution",
},
},
{
drillType: "drill-thru/distribution",
clickType: "header",
queryType: "aggregated",
columnName: "CREATED_AT",
expectedParameters: {
type: "drill-thru/distribution",
},
},
// endregion
// region --- drill-thru/fk-filter
......
src/metabase/lib/breakout.cljc
+ 8
− 0
View file @ 269e19d6
......@@ -83,3 +83,11 @@
(cond-> %
pos (assoc :breakout-position pos)))
cols))))))
(defn breakout-column?
"Returns if `column` is a breakout column of stage with `stage-number` of `query`."
[query stage-number column]
(some?
(some (fn [a-breakout]
(lib.equality/find-matching-column query stage-number a-breakout [column] {:generous? true}))
(breakouts query stage-number))))
src/metabase/lib/drill_thru/column_filter.cljc
+ 2
− 3
View file @ 269e19d6
......@@ -14,12 +14,11 @@
[query :- ::lib.schema/query
stage-number :- :int
{:keys [column value]} :- ::lib.schema.drill-thru/context]
;; Note: original code uses an addition `clicked.column.field_ref != null` condition.
(when (and (lib.drill-thru.common/mbql-stage? query stage-number)
column
(nil? value)
(not (lib.types.isa/structured? column))
;; Must be a real field in the DB. Note: original code uses `clicked.column.field_ref != null` for this.
(some? (:id column)))
(not (lib.types.isa/structured? column)))
(let [initial-op (when-not (lib.types.isa/temporal? column) ; Date fields have special handling in the FE.
(-> (lib.filter.operator/filter-operators column)
first
......
src/metabase/lib/drill_thru/distribution.cljc
+ 2
− 1
View file @ 269e19d6
......@@ -28,7 +28,8 @@
(not (lib.types.isa/primary-key? column))
(not (lib.types.isa/structured? column))
(not (lib.types.isa/comment? column))
(not (lib.types.isa/description? column)))
(not (lib.types.isa/description? column))
(not (lib.breakout/breakout-column? query stage-number column)))
{:lib/type :metabase.lib.drill-thru/drill-thru
:type :drill-thru/distribution
:column column}))
......
src/metabase/lib/drill_thru/summarize_column.cljc
+ 4
− 1
View file @ 269e19d6
(ns metabase.lib.drill-thru.summarize-column
(:require
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.breakout :as lib.breakout]
[metabase.lib.drill-thru.common :as lib.drill-thru.common]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.drill-thru :as lib.schema.drill-thru]
......@@ -16,7 +17,9 @@
(when (and (lib.drill-thru.common/mbql-stage? query stage-number)
column
(nil? value)
(not (lib.types.isa/structured? column)))
(not (lib.types.isa/structured? column))
(not= (:lib/source column) :source/aggregations)
(not (lib.breakout/breakout-column? query stage-number column)))
;; I'm not really super clear on how the FE is supposed to be able to display these.
(let [aggregation-ops (concat [:distinct]
(when (lib.types.isa/summable? column)
......
test/metabase/lib/drill_thru_test.cljc
+ 12
− 10
View file @ 269e19d6
......@@ -457,11 +457,12 @@
(is (some? count-col))
(let [context {:column count-col
:value nil}]
(is (=? [{:type :drill-thru/sort
:column {:name "count"}}
{:type :drill-thru/summarize-column
:column {:name "count"}
:aggregations [:distinct :sum :avg]}]
(is (=? [{:type :drill-thru/column-filter
:column {:name "count"}
:initial-op {:display-name-variant :equal-to
:short :=}}
{:type :drill-thru/sort
:column {:name "count"}}]
(lib/available-drill-thrus query -1 context)))
(test-drill-applications query context))))
(testing "Drills for max(discount) aggregation"
......@@ -471,11 +472,12 @@
(is (some? max-of-discount-col))
(let [context {:column max-of-discount-col
:value nil}]
(is (=? [{:type :drill-thru/sort
:column {:display-name "Max of Discount"}}
{:type :drill-thru/summarize-column
:column {:display-name "Max of Discount"}
:aggregations [:distinct :sum :avg]}]
(is (=? [{:type :drill-thru/column-filter,
:column {:display-name "Max of Discount"}
:initial-op {:display-name-variant :equal-to
:short :=}}
{:type :drill-thru/sort
:column {:display-name "Max of Discount"}}]
(lib/available-drill-thrus query -1 context)))
(test-drill-applications query context))))))
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment