From 839f1f775c9570a3e76f21eaacc13d0f15182f0d Mon Sep 17 00:00:00 2001
From: Alexander Solovyov <alexander@solovyov.net>
Date: Mon, 24 Jun 2024 10:13:15 +0300
Subject: [PATCH] xray should not filter again models with filters (#44547)
This not only does not make sense, but also fails for models with aggregations (and thus not having those fields for filtering available).
---
.../automagic_dashboards/combination.clj | 8 +++-
.../xrays/automagic_dashboards/core_test.clj | 38 +++++++++++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/src/metabase/xrays/automagic_dashboards/combination.clj b/src/metabase/xrays/automagic_dashboards/combination.clj
index 98116d93818..ea1259f461c 100644
--- a/src/metabase/xrays/automagic_dashboards/combination.clj
+++ b/src/metabase/xrays/automagic_dashboards/combination.clj
@@ -21,6 +21,7 @@
[clojure.walk :as walk]
[medley.core :as m]
[metabase.driver.util :as driver.u]
+ [metabase.models.card :as card]
[metabase.models.interface :as mi]
[metabase.query-processor.util :as qp.util]
[metabase.util :as u]
@@ -78,13 +79,16 @@
{{:keys [database]} :root :keys [source query-filter]}]
(let [source-table (if (->> source (mi/instance-of? :model/Table))
(-> source u/the-id)
- (->> source u/the-id (str "card__")))]
+ (->> source u/the-id (str "card__")))
+ model? (and (mi/instance-of? :model/Card source)
+ (card/model? source))]
(assoc ground-metric-with-dimensions
:dataset_query {:database database
:type :query
:query (cond-> (assoc metric-definition
:source-table source-table)
- query-filter (assoc :filter query-filter))})))
+ (and (not model?)
+ query-filter) (assoc :filter query-filter))})))
(defn- instantiate-visualization
[[k v] dimensions metrics]
diff --git a/test/metabase/xrays/automagic_dashboards/core_test.clj b/test/metabase/xrays/automagic_dashboards/core_test.clj
index 11d3cdcfc41..1628e043ac3 100644
--- a/test/metabase/xrays/automagic_dashboards/core_test.clj
+++ b/test/metabase/xrays/automagic_dashboards/core_test.clj
@@ -655,6 +655,44 @@
:when name]
name)))))))))))
+(deftest basic-root-model-test-4
+ (testing "Simple model with an aggregation and a filter should not leak filter to the upper cards"
+ (mt/dataset test-data
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (let [source-query {:database (mt/id)
+ :query (mt/$ids
+ {:source-table $$products
+ :aggregation [[:count]],
+ :breakout [$products.id]
+ :filter [:time-interval $products.created_at -30 :day]})
+ :type :query}]
+ (mt/with-temp
+ [Collection {collection-id :id} {}
+ Card card {:table_id (mt/id :products)
+ :collection_id collection-id
+ :dataset_query source-query
+ :result_metadata (mt/with-test-user
+ :rasta
+ (result-metadata-for-query
+ source-query))
+ :type :model}]
+ (let [dashboard (mt/with-test-user :rasta (magic/automagic-analysis card nil))]
+ (ensure-single-table-sourced (mt/id :products) dashboard)
+ ;; Count of records
+ ;; Distributions:
+ ;; - Summary statistics
+ ;; - Count
+ ;; - Distinct count
+ ;; - Null values
+ (is (= 4 (->> dashboard :dashcards (filter :card) count)))
+ (ensure-dashboard-sourcing card dashboard)
+ ;; This ensures we don't get :filter on queries derived from a model
+ (is (empty?
+ (for [{:keys [dataset_query]} (:dashcards dashboard)
+ :when (:filter dataset_query)]
+ dataset_query))))))))))
+
+
(deftest model-title-does-not-leak-abstraction-test
(testing "The title of a model or question card should not be X model or X question, but just X."
(mt/dataset test-data
--
GitLab