From be0d5b907b37b2726006dcebd70be6618bfae499 Mon Sep 17 00:00:00 2001
From: Cam Saul <cammsaul@gmail.com>
Date: Mon, 8 Jul 2019 15:00:08 -0700
Subject: [PATCH] Include Table info in Metric/Segment search results (#10305)
---
src/metabase/api/search.clj | 374 ++++++++++++++++++------------
test/metabase/api/search_test.clj | 139 ++++++-----
2 files changed, 318 insertions(+), 195 deletions(-)
diff --git a/src/metabase/api/search.clj b/src/metabase/api/search.clj
index 14f80b6be04..28016d83e1a 100644
--- a/src/metabase/api/search.clj
+++ b/src/metabase/api/search.clj
@@ -1,9 +1,14 @@
(ns metabase.api.search
(:require [clojure.string :as str]
[compojure.core :refer [GET]]
- [honeysql.helpers :as h]
- [metabase.api.common :refer [*current-user-id* *current-user-permissions-set* check-403 defendpoint
- define-routes]]
+ [flatland.ordered.map :as ordered-map]
+ [honeysql
+ [core :as hsql]
+ [helpers :as h]]
+ [metabase
+ [db :as mdb]
+ [util :as u]]
+ [metabase.api.common :as api]
[metabase.models
[card :refer [Card]]
[card-favorite :refer [CardFavorite]]
@@ -12,202 +17,285 @@
[dashboard-favorite :refer [DashboardFavorite]]
[metric :refer [Metric]]
[pulse :refer [Pulse]]
- [segment :refer [Segment]]]
- [metabase.util :as u]
+ [segment :refer [Segment]]
+ [table :refer [Table]]]
[metabase.util
[honeysql-extensions :as hx]
[schema :as su]]
[schema.core :as s]
[toucan.db :as db]))
+(def ^:private SearchContext
+ "Map with the various allowed search parameters, used to construct the SQL query"
+ {:search-string (s/maybe su/NonBlankString)
+ :archived? s/Bool
+ :visible-collections coll/VisibleCollections})
+
+(def ^:private searchable-models
+ [Card Dashboard Pulse Collection Segment Metric])
+
+(def ^:private SearchableModel
+ (apply s/enum searchable-models))
+
+(def ^:private HoneySQLColumn
+ (s/cond-pre
+ s/Keyword
+ [(s/one s/Any "column or value")
+ (s/one s/Keyword "alias")]))
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Columns for each Entity |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(def ^:private all-search-columns
+ "All columns that will appear in the search results, and the types of those columns. The generated search query is a
+ `UNION ALL` of the queries for each different entity; it looks something like:
+
+ SELECT 'card' AS model, id, cast(NULL AS integer) AS table_id, ...
+ FROM report_card
+ UNION ALL
+ SELECT 'metric' as model, id, table_id, ...
+ FROM metric
+
+ Columns that aren't used in any individual query are replaced with `SELECT cast(NULL AS <type>)` statements. (These
+ are cast to the appropriate type because Postgres will assume `SELECT NULL` is `TEXT` by default and will refuse to
+ `UNION` two columns of two different types.)"
+ (ordered-map/ordered-map
+ ;; returned for all models
+ :model :text
+ :id :integer
+ :name :text
+ :description :text
+ :archived :boolean
+ ;; returned for Card, Dashboard, Pulse, and Collection
+ :collection_id :integer
+ ;; returned for Card and Dashboard
+ :collection_position :integer
+ :favorite :boolean
+ ;; returned for Metric and Segment
+ :table_id :integer
+ :database_id :integer
+ :table_schema :text
+ :table_name :text
+ :table_description :text))
+
+;; below are the actual columns returned for any given entity
+
(def ^:private default-columns
+ "Columns returned for all models."
[:id :name :description :archived])
-(def ^:private card-columns-without-type
- (concat default-columns
- [:collection_id :collection_position [:card_fav.id :favorite]]))
+(def ^:private favorite-col
+ "Case statement to return boolean values of `:favorite` for Card and Dashboard."
+ [(hsql/call :case [:not= :fave.id nil] true :else false) :favorite])
+
+(def ^:private table-columns
+ "Columns containing information about the table this model references. Returned for Metrics and Segments."
+ [:table_id
+ [:table.db_id :database_id]
+ [:table.schema :table_schema]
+ [:table.name :table_name]
+ [:table.description :table_description]])
-(def ^:private dashboard-columns-without-type
- (concat default-columns
- [:collection_id :collection_position [:dashboard_fav.id :favorite]]))
+(defmulti ^:private columns-for-model
+ "The columns that will be returned by the query for `model`, excluding `:model`, which is added automatically."
+ {:arglists '([model])}
+ class)
-(def ^:private pulse-columns-without-type
+(defmethod columns-for-model (class Card)
+ [_]
+ (conj default-columns :collection_id :collection_position favorite-col))
+
+(defmethod columns-for-model (class Dashboard)
+ [_]
+ (conj default-columns :collection_id :collection_position favorite-col))
+
+(defmethod columns-for-model (class Pulse)
+ [_]
[:id :name :collection_id])
-(def ^:private collection-columns-without-type
- (concat default-columns
- [[:id :collection_id]]))
+(defmethod columns-for-model (class Collection)
+ [_]
+ (conj default-columns [:id :collection_id]))
+
+(defmethod columns-for-model (class Segment)
+ [_]
+ (into default-columns table-columns))
+
+(defmethod columns-for-model (class Metric)
+ [_]
+ (into default-columns table-columns))
-(def ^:private segment-columns-without-type
- default-columns)
-(def ^:private metric-columns-without-type
- default-columns)
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Shared Query Logic |
+;;; +----------------------------------------------------------------------------------------------------------------+
-(defn- ->column
+(s/defn ^:private model->alias :- s/Keyword
+ [model :- SearchableModel]
+ (keyword (str/lower-case (name model))))
+
+(s/defn ^:private ->column-alias :- s/Keyword
"Returns the column name. If the column is aliased, i.e. [`:original_name` `:aliased_name`], return the aliased
column name"
- [column-or-aliased]
+ [column-or-aliased :- HoneySQLColumn]
(if (sequential? column-or-aliased)
(second column-or-aliased)
column-or-aliased))
-(def ^:private search-columns-without-type
- "The columns found in search query clauses except type. Type is added automatically"
- (set (map ->column (concat card-columns-without-type
- dashboard-columns-without-type
- pulse-columns-without-type
- collection-columns-without-type
- segment-columns-without-type
- metric-columns-without-type))))
+(s/defn ^:private canonical-columns :- [HoneySQLColumn]
+ "Returns a seq of canonicalized list of columns for the search query with the given `model` Will return column names
+ prefixed with the `model` name so that it can be used in criteria. Projects a `nil` for columns the `model` doesn't
+ have and doesn't modify aliases."
+ [model :- SearchableModel, col-alias->honeysql-clause :- {s/Keyword HoneySQLColumn}]
+ (for [[search-col col-type] all-search-columns
+ :let [maybe-aliased-col (get col-alias->honeysql-clause search-col)]]
+ (cond
+ (= search-col :model)
+ [(hx/literal (name (model->alias model))) :model]
-(def ^:private SearchContext
- "Map with the various allowed search parameters, used to construct the SQL query"
- {:search-string (s/maybe su/NonBlankString)
- :archived? s/Bool
- :visible-collections coll/VisibleCollections})
+ ;; This is an aliased column, no need to include the table alias
+ (sequential? maybe-aliased-col)
+ maybe-aliased-col
+
+ ;; This is a column reference, need to add the table alias to the column
+ maybe-aliased-col
+ (hsql/qualify (model->alias model) (name maybe-aliased-col))
-(defn- make-canonical-columns
- "Returns a seq of canonicalized list of columns for the search query with the given `entity-type`. Will return
- column names prefixed with the `entity-type` name so that it can be used in criteria. Projects a nil for columns the
- `entity-type` doesn't have and doesn't modify aliases."
- [entity-type col-name->columns]
- (concat (for [search-col search-columns-without-type
- :let [maybe-aliased-col (get col-name->columns search-col)]]
- (cond
- ;; This is an aliased column, no need to include the table alias
- (sequential? maybe-aliased-col)
- maybe-aliased-col
-
- ;; This is a column reference, need to add the table alias to the column
- maybe-aliased-col
- (keyword (str entity-type "." (name maybe-aliased-col)))
-
- ;; This entity is missing the column, project a null for that column value
- :else
- [nil search-col]))
- [[(hx/literal entity-type) :model]]))
-
-(defn- merge-search-select
+ ;; This entity is missing the column, project a null for that column value. For Postgres and H2, cast it to the
+ ;; correct type, e.g.
+ ;;
+ ;; SELECT cast(NULL AS integer)
+ ;;
+ ;; For MySQL, this is not needed.
+ :else
+ [(if (= (mdb/db-type) :mysql)
+ nil
+ (hx/cast col-type nil))
+ search-col])))
+
+(s/defn ^:private select-clause-for-model :- [HoneySQLColumn]
"The search query uses a `union-all` which requires that there be the same number of columns in each of the segments
- of the query. This function will take `entity-columns` and will inject constant `nil` values for any column missing
- from `entity-columns` but found in `search-columns`"
- [query-map entity-type entity-columns]
- (let [col-name->column (u/key-by ->column entity-columns)
- cols-or-nils (make-canonical-columns entity-type col-name->column)]
- (apply h/merge-select query-map (concat cols-or-nils))))
-
-;; TODO - not used anywhere except `merge-name-and-archived-search` anymore so we can roll it into that
-(s/defn ^:private merge-name-search
- "Add case-insensitive name query criteria to `query-map`"
- [query-map {:keys [search-string]} :- SearchContext]
- (if (empty? search-string)
- query-map
- (h/merge-where query-map [:like :%lower.name (str "%" (str/lower-case search-string) "%")])))
-
-(s/defn ^:private merge-name-and-archived-search
- "Add name and archived query criteria to `query-map`"
- [query-map {:keys [search-string archived?] :as search-ctx} :- SearchContext]
- (-> query-map
- (merge-name-search search-ctx)
- (h/merge-where [:= :archived archived?])))
-
-(s/defn ^:private add-collection-criteria
+ of the query. This function will take the columns for `model` and will inject constant `nil` values for any column
+ missing from `entity-columns` but found in `all-search-columns`."
+ [model :- SearchableModel]
+ (let [entity-columns (columns-for-model model)
+ column-alias->honeysql-clause (u/key-by ->column-alias entity-columns)
+ cols-or-nils (canonical-columns model column-alias->honeysql-clause)]
+ cols-or-nils))
+
+(s/defn ^:private from-clause-for-model :- [(s/one [(s/one SearchableModel "model") (s/one s/Keyword "alias")]
+ "from clause")]
+ [model :- SearchableModel]
+ [[model (model->alias model)]])
+
+(s/defn ^:private base-where-clause-for-model :- [(s/one (s/enum :and :=) "type") s/Any]
+ [model :- SearchableModel, {:keys [search-string archived?]} :- SearchContext]
+ (let [archived-clause [:= (hsql/qualify (model->alias model) :archived) archived?]
+ search-string-clause (when (seq search-string)
+ [:like
+ (hsql/call :lower (hsql/qualify (model->alias model) :name))
+ (str "%" (str/lower-case search-string) "%")])]
+ (if search-string-clause
+ [:and archived-clause search-string-clause]
+ archived-clause)))
+
+(s/defn ^:private base-query-for-model :- {:select s/Any, :from s/Any, :where s/Any}
+ "Create a HoneySQL query map with `:select`, `:from`, and `:where` clauses for `model`, suitable for the `UNION ALL`
+ used in search."
+ [model :- SearchableModel, context :- SearchContext]
+ {:select (select-clause-for-model model)
+ :from (from-clause-for-model model)
+ :where (base-where-clause-for-model model context)})
+
+(s/defn ^:private add-where-clause-for-collection-id
"Update the query to only include collections the user has access to"
- [query-map, column-kwd :- s/Keyword, {:keys [visible-collections]} :- SearchContext]
+ [honeysql-query :- su/Map, collection-id-column :- s/Keyword, {:keys [visible-collections]} :- SearchContext]
(h/merge-where
- query-map
- (coll/visible-collection-ids->honeysql-filter-clause
- column-kwd
- visible-collections)))
+ honeysql-query
+ (coll/visible-collection-ids->honeysql-filter-clause collection-id-column visible-collections)))
-(defn- make-honeysql-search-query
- "Create a HoneySQL query map to search for `entity`, suitable for the UNION ALL used in search."
- [entity search-type projected-columns]
- (-> {}
- (merge-search-select search-type projected-columns)
- (h/merge-from [entity (keyword search-type)])))
-(defmulti ^:private create-search-query (fn [entity search-context] entity))
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Search Queries for each Toucan Model |
+;;; +----------------------------------------------------------------------------------------------------------------+
-(s/defmethod ^:private create-search-query :card
+(defmulti ^:private search-query-for-model
+ {:arglists '([model search-context])}
+ (fn [model _] (class model)))
+
+(s/defmethod ^:private search-query-for-model (class Card)
[_ search-ctx :- SearchContext]
- (-> (make-honeysql-search-query Card "card" card-columns-without-type)
- (h/left-join [(-> (h/select :id :card_id)
- (h/merge-from CardFavorite)
- (h/merge-where [:= :owner_id *current-user-id*]))
- :card_fav]
- [:= :card.id :card_fav.card_id])
- (merge-name-and-archived-search search-ctx)
- (add-collection-criteria :collection_id search-ctx)))
-
-(s/defmethod ^:private create-search-query :collection
+ (-> (base-query-for-model Card search-ctx)
+ (h/left-join [CardFavorite :fave]
+ [:and
+ [:= :card.id :fave.card_id]
+ [:= :fave.owner_id api/*current-user-id*]])
+ (add-where-clause-for-collection-id :card.collection_id search-ctx)))
+
+(s/defmethod ^:private search-query-for-model (class Collection)
[_ search-ctx :- SearchContext]
- (-> (make-honeysql-search-query Collection "collection" collection-columns-without-type)
- (merge-name-and-archived-search search-ctx)
- (add-collection-criteria :id search-ctx)))
+ (-> (base-query-for-model Collection search-ctx)
+ (add-where-clause-for-collection-id :collection.id search-ctx)))
-(s/defmethod ^:private create-search-query :dashboard
+(s/defmethod ^:private search-query-for-model (class Dashboard)
[_ search-ctx :- SearchContext]
- (-> (make-honeysql-search-query Dashboard "dashboard" dashboard-columns-without-type)
- (h/left-join [(-> (h/select :id :dashboard_id)
- (h/merge-from DashboardFavorite)
- (h/merge-where [:= :user_id *current-user-id*]))
- :dashboard_fav]
- [:= :dashboard.id :dashboard_fav.dashboard_id])
- (merge-name-and-archived-search search-ctx)
- (add-collection-criteria :collection_id search-ctx)))
-
-(s/defmethod ^:private create-search-query :pulse
+ (-> (base-query-for-model Dashboard search-ctx)
+ (h/left-join [DashboardFavorite :fave]
+ [:and
+ [:= :dashboard.id :fave.dashboard_id]
+ [:= :fave.user_id api/*current-user-id*]])
+ (add-where-clause-for-collection-id :dashboard.collection_id search-ctx)))
+
+(s/defmethod ^:private search-query-for-model (class Pulse)
[_ search-ctx :- SearchContext]
;; Pulses don't currently support being archived, omit if archived is true
- (-> (make-honeysql-search-query Pulse "pulse" pulse-columns-without-type)
- (merge-name-and-archived-search search-ctx)
- (add-collection-criteria :collection_id search-ctx)
+ (-> (base-query-for-model Pulse search-ctx)
+ (add-where-clause-for-collection-id :pulse.collection_id search-ctx)
;; We don't want alerts included in pulse results
(h/merge-where [:= :alert_condition nil])))
-(s/defmethod ^:private create-search-query :metric
+(s/defmethod ^:private search-query-for-model (class Metric)
[_ search-ctx :- SearchContext]
- (-> (make-honeysql-search-query Metric "metric" metric-columns-without-type)
- (merge-name-and-archived-search search-ctx)))
+ (-> (base-query-for-model Metric search-ctx)
+ (h/left-join [Table :table] [:= :metric.table_id :table.id])))
-(s/defmethod ^:private create-search-query :segment
+(s/defmethod ^:private search-query-for-model (class Segment)
[_ search-ctx :- SearchContext]
- (-> (make-honeysql-search-query Segment "segment" segment-columns-without-type)
- (merge-name-and-archived-search search-ctx)))
-
-(defn- favorited->boolean [row]
- (if-let [fav-value (get row :favorite)]
- (assoc row :favorite (and (integer? fav-value)
- (not (zero? fav-value))))
- row))
+ (-> (base-query-for-model Segment search-ctx)
+ (h/left-join [Table :table] [:= :segment.table_id :table.id])))
(s/defn ^:private search
"Builds a search query that includes all of the searchable entities and runs it"
[search-ctx :- SearchContext]
- (map favorited->boolean
- (db/query {:union-all (for [entity [:card :collection :dashboard :pulse :segment :metric]
- :let [query-map (create-search-query entity search-ctx)]
- :when query-map]
- query-map)})))
+ (for [row (db/query {:union-all (for [model searchable-models]
+ (search-query-for-model model search-ctx))})]
+ ;; MySQL returns `:favorite` as `1` or `0` so convert those to boolean as needed
+ (update row :favorite (fn [favorite]
+ (if (integer? favorite)
+ (not (zero? favorite))
+ favorite)))))
+
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Endpoint |
+;;; +----------------------------------------------------------------------------------------------------------------+
(s/defn ^:private make-search-context :- SearchContext
- [search-string :- (s/maybe su/NonBlankString)
- archived-string :- (s/maybe su/BooleanString)]
+ [search-string :- (s/maybe su/NonBlankString), archived-string :- (s/maybe su/BooleanString)]
{:search-string search-string
:archived? (Boolean/parseBoolean archived-string)
- :visible-collections (coll/permissions-set->visible-collection-ids @*current-user-permissions-set*)})
+ :visible-collections (coll/permissions-set->visible-collection-ids @api/*current-user-permissions-set*)})
-(defendpoint GET "/"
+(api/defendpoint GET "/"
"Search Cards, Dashboards, Collections and Pulses for the substring `q`."
[q archived]
{q (s/maybe su/NonBlankString)
archived (s/maybe su/BooleanString)}
(let [{:keys [visible-collections] :as search-ctx} (make-search-context q archived)]
;; Throw if the user doesn't have access to any collections
- (check-403 (or (= :all visible-collections)
- (seq visible-collections)))
+ (api/check-403 (or (= :all visible-collections)
+ (seq visible-collections)))
(search search-ctx)))
-(define-routes)
+(api/define-routes)
diff --git a/test/metabase/api/search_test.clj b/test/metabase/api/search_test.clj
index 722570ba45e..29d7b1bf8ef 100644
--- a/test/metabase/api/search_test.clj
+++ b/test/metabase/api/search_test.clj
@@ -2,7 +2,7 @@
(:require [clojure
[set :as set]
[string :as str]]
- [expectations :refer :all]
+ [expectations :refer [expect]]
[metabase.models
[card :refer [Card]]
[card-favorite :refer [CardFavorite]]
@@ -14,30 +14,65 @@
[permissions-group :as group :refer [PermissionsGroup]]
[permissions-group-membership :refer [PermissionsGroupMembership]]
[pulse :refer [Pulse]]
- [segment :refer [Segment]]]
- [metabase.test.data.users :refer :all]
- [metabase.test.util :as tu]
+ [segment :refer [Segment]]
+ [table :refer [Table]]]
+ [metabase.test
+ [data :as data]
+ [util :as tu]]
+ [metabase.test.data.users :as test-users]
[metabase.util :as u]
+ [toucan.db :as db]
[toucan.util.test :as tt]))
(def default-search-row
- {:description nil, :id true, :collection_id false,
- :collection_position nil, :archived false, :favorite nil})
-
-(def ^:private default-search-results
- (set (map #(merge default-search-row %)
- [{:name "dashboard test dashboard", :model "dashboard"}
- {:name "collection test collection", :model "collection", :collection_id true}
- {:name "card test card", :model "card"}
- {:name "pulse test pulse", :model "pulse", :archived nil}
- {:name "metric test metric", :description "Lookin' for a blueberry", :model "metric"}
- {:name "segment test segment", :description "Lookin' for a blueberry", :model "segment"}])))
-
-(def ^:private default-metric-segment-results
- (set (filter (comp #{"metric" "segment"} :model) default-search-results)))
-
-(def ^:private default-archived-results
- (set (for [result default-search-results
+ {:id true
+ :description nil
+ :archived false
+ :collection_id false
+ :collection_position nil
+ :favorite nil
+ :table_id false
+ :database_id false
+ :table_schema nil
+ :table_name nil
+ :table_description nil})
+
+(defn- table-search-results
+ "Segments and Metrics come back with information about their Tables as of 0.33.0. The `model-defaults` for Segment and
+ Metric put them both in the `:checkins` Table."
+ []
+ (merge
+ {:table_id true, :database_id true}
+ (db/select-one [Table [:name :table_name] [:schema :table_schema] [:description :table_description]]
+ :id (data/id :checkins))))
+
+(defn- default-search-results []
+ #{(merge
+ default-search-row
+ {:name "dashboard test dashboard", :model "dashboard", :favorite false})
+ (merge
+ default-search-row
+ {:name "collection test collection", :model "collection", :collection_id true})
+ (merge
+ default-search-row
+ {:name "card test card", :model "card", :favorite false})
+ (merge
+ default-search-row
+ {:name "pulse test pulse", :model "pulse", :archived nil})
+ (merge
+ default-search-row
+ {:model "metric", :name "metric test metric", :description "Lookin' for a blueberry"}
+ (table-search-results))
+ (merge
+ default-search-row
+ {:model "segment", :name "segment test segment", :description "Lookin' for a blueberry"}
+ (table-search-results))})
+
+(defn- default-metric-segment-results []
+ (set (filter (comp #{"metric" "segment"} :model) (default-search-results))))
+
+(defn- default-archived-results []
+ (set (for [result (default-search-results)
:when (false? (:archived result))]
(assoc result :archived true))))
@@ -47,10 +82,10 @@
(f search-item)
search-item))))
-(def ^:private default-results-with-collection
+(defn- default-results-with-collection []
(on-search-types #{"dashboard" "pulse" "card"}
#(assoc % :collection_id true)
- default-search-results))
+ (default-search-results)))
(defn- do-with-search-items [search-string in-root-collection? f]
(let [data-map (fn [instance-name]
@@ -73,17 +108,17 @@
:segment segment}))))
(defmacro ^:private with-search-items-in-root-collection [search-string & body]
- `(do-with-search-items ~search-string true (fn [_#] ~@body)))
+ `(do-with-search-items ~search-string true (fn [~'_] ~@body)))
(defmacro ^:private with-search-items-in-collection [created-items-sym search-string & body]
`(do-with-search-items ~search-string false (fn [~created-items-sym] ~@body)))
(defn- search-request [user-kwd & params]
- (tu/boolean-ids-and-timestamps (set (apply (user->client user-kwd) :get 200 "search" params))))
+ (tu/boolean-ids-and-timestamps (set (apply (test-users/user->client user-kwd) :get 200 "search" params))))
;; Basic search, should find 1 of each entity type, all items in the root collection
(expect
- default-search-results
+ (default-search-results)
(with-search-items-in-root-collection "test"
(search-request :crowberto :q "test")))
@@ -91,110 +126,110 @@
;; previous tests. Instead of an = comparison here, just ensure our default results are included
(expect
(set/subset?
- default-search-results
+ (default-search-results)
(with-search-items-in-root-collection "test"
(search-request :crowberto))))
;; Ensure that users without perms for the root collection don't get results
;; NOTE: Metrics and segments don't have collections, so they'll be returned
(expect
- default-metric-segment-results
+ (default-metric-segment-results)
(tu/with-non-admin-groups-no-root-collection-perms
(with-search-items-in-root-collection "test"
(search-request :rasta :q "test"))))
;; Users that have root collection permissions should get root collection search results
(expect
- (set (remove (comp #{"collection"} :model) default-search-results))
+ (set (remove (comp #{"collection"} :model) (default-search-results)))
(tu/with-non-admin-groups-no-root-collection-perms
(with-search-items-in-root-collection "test"
(tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (user->id :rasta), :group_id (u/get-id group)}]]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection/is-root? true}))
(search-request :rasta :q "test")))))
;; Users without root collection permissions should still see other collections they have access to
(expect
- (into default-results-with-collection
- (map #(merge default-search-row %)
+ (into (default-results-with-collection)
+ (map #(merge default-search-row % (table-search-results))
[{:name "metric test2 metric", :description "Lookin' for a blueberry", :model "metric"}
{:name "segment test2 segment", :description "Lookin' for a blueberry", :model "segment"}]))
(tu/with-non-admin-groups-no-root-collection-perms
(with-search-items-in-collection {:keys [collection]} "test"
(with-search-items-in-root-collection "test2"
(tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (user->id :rasta), :group_id (u/get-id group)}]]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-collection-read-permissions! group (u/get-id collection))
(search-request :rasta :q "test"))))))
;; Users with root collection permissions should be able to search root collection data long with collections they
;; have access to
(expect
- (into default-results-with-collection
- (for [row default-search-results
+ (into (default-results-with-collection)
+ (for [row (default-search-results)
:when (not= "collection" (:model row))]
(update row :name #(str/replace % "test" "test2"))))
(tu/with-non-admin-groups-no-root-collection-perms
(with-search-items-in-collection {:keys [collection]} "test"
(with-search-items-in-root-collection "test2"
(tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (user->id :rasta), :group_id (u/get-id group)}]]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection/is-root? true}))
(perms/grant-collection-read-permissions! group collection)
(search-request :rasta :q "test"))))))
;; Users with access to multiple collections should see results from all collections they have access to
(expect
- (into default-results-with-collection
+ (into (default-results-with-collection)
(map (fn [row] (update row :name #(str/replace % "test" "test2")))
- default-results-with-collection))
+ (default-results-with-collection)))
(with-search-items-in-collection {coll-1 :collection} "test"
(with-search-items-in-collection {coll-2 :collection} "test2"
(tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (user->id :rasta), :group_id (u/get-id group)}]]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-collection-read-permissions! group (u/get-id coll-1))
(perms/grant-collection-read-permissions! group (u/get-id coll-2))
(search-request :rasta :q "test")))))
;; User should only see results in the collection they have access to
(expect
- (into default-results-with-collection
- (map #(merge default-search-row %)
+ (into (default-results-with-collection)
+ (map #(merge default-search-row % (table-search-results))
[{:name "metric test2 metric", :description "Lookin' for a blueberry", :model "metric"}
{:name "segment test2 segment", :description "Lookin' for a blueberry", :model "segment"}]))
(tu/with-non-admin-groups-no-root-collection-perms
(with-search-items-in-collection {coll-1 :collection} "test"
(with-search-items-in-collection {coll-2 :collection} "test2"
(tt/with-temp* [PermissionsGroup [group]
- PermissionsGroupMembership [_ {:user_id (user->id :rasta), :group_id (u/get-id group)}]]
+ PermissionsGroupMembership [_ {:user_id (test-users/user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-collection-read-permissions! group (u/get-id coll-1))
(search-request :rasta :q "test"))))))
;; Favorites are per user, so other user's favorites don't cause search results to be favorited
(expect
- default-results-with-collection
+ (default-results-with-collection)
(with-search-items-in-collection {:keys [card dashboard]} "test"
(tt/with-temp* [CardFavorite [_ {:card_id (u/get-id card)
- :owner_id (user->id :rasta)}]
+ :owner_id (test-users/user->id :rasta)}]
DashboardFavorite [_ {:dashboard_id (u/get-id dashboard)
- :user_id (user->id :rasta)}]]
+ :user_id (test-users/user->id :rasta)}]]
(search-request :crowberto :q "test"))))
;; Basic search, should find 1 of each entity type and include favorites when available
(expect
(on-search-types #{"dashboard" "card"}
#(assoc % :favorite true)
- default-results-with-collection)
+ (default-results-with-collection))
(with-search-items-in-collection {:keys [card dashboard]} "test"
(tt/with-temp* [CardFavorite [_ {:card_id (u/get-id card)
- :owner_id (user->id :crowberto)}]
+ :owner_id (test-users/user->id :crowberto)}]
DashboardFavorite [_ {:dashboard_id (u/get-id dashboard)
- :user_id (user->id :crowberto)}]]
+ :user_id (test-users/user->id :crowberto)}]]
(search-request :crowberto :q "test"))))
;; Basic search should only return substring matches
(expect
- default-search-results
+ (default-search-results)
(with-search-items-in-root-collection "test"
(with-search-items-in-root-collection "something different"
(search-request :crowberto :q "test"))))
@@ -204,7 +239,7 @@
;; Should return unarchived results by default
(expect
- default-search-results
+ (default-search-results)
(with-search-items-in-root-collection "test"
(tt/with-temp* [Card [_ (archived {:name "card test card 2"})]
Dashboard [_ (archived {:name "dashboard test dashboard 2"})]
@@ -215,7 +250,7 @@
;; Should return archived results when specified
(expect
- default-archived-results
+ (default-archived-results)
(with-search-items-in-root-collection "test2"
(tt/with-temp* [Card [_ (archived {:name "card test card"})]
Dashboard [_ (archived {:name "dashboard test dashboard"})]
@@ -235,4 +270,4 @@
(filter (fn [{:keys [model id]}]
(and (= id (u/get-id pulse))
(= "pulse" model)))
- ((user->client :crowberto) :get 200 "search")))))
+ ((test-users/user->client :crowberto) :get 200 "search")))))
--
GitLab