From 5be9510069c05d9d7d5d0f540a4d750258d19dff Mon Sep 17 00:00:00 2001
From: Chris Truter <crisptrutski@users.noreply.github.com>
Date: Tue, 19 Nov 2024 18:05:46 +0200
Subject: [PATCH] For new search, default to excluding other people's
collections (#50146)
---
src/metabase/api/search.clj | 6 ++++--
src/metabase/search/config.clj | 3 ++-
src/metabase/search/filter.clj | 24 ++++++++++++++++++++----
src/metabase/search/impl.clj | 33 +++++++++++++++++++++------------
4 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/src/metabase/api/search.clj b/src/metabase/api/search.clj
index c907b9a6bef..695c64d80e4 100644
--- a/src/metabase/api/search.clj
+++ b/src/metabase/api/search.clj
@@ -122,14 +122,15 @@
- The `verified` filter supports models and cards.
A search query that has both filters applied will only return models and cards."
- [q archived created_at created_by table_db_id models last_edited_at last_edited_by
+ [q context archived created_at created_by table_db_id models last_edited_at last_edited_by
filter_items_in_personal_collection model_ancestors search_engine search_native_query
verified ids calculate_available_models]
{q [:maybe ms/NonBlankString]
+ context [:maybe :keyword]
archived [:maybe :boolean]
table_db_id [:maybe ms/PositiveInt]
models [:maybe (ms/QueryVectorOf search/SearchableModel)]
- filter_items_in_personal_collection [:maybe [:enum "only" "exclude"]]
+ filter_items_in_personal_collection [:maybe [:enum "all" "only" "only-mine" "exclude" "exclude-others"]]
created_at [:maybe ms/NonBlankString]
created_by [:maybe (ms/QueryVectorOf ms/PositiveInt)]
last_edited_at [:maybe ms/NonBlankString]
@@ -147,6 +148,7 @@
(search/search
(search/search-context
{:archived archived
+ :context context
:created-at created_at
:created-by (set created_by)
:current-user-id api/*current-user-id*
diff --git a/src/metabase/search/config.clj b/src/metabase/search/config.clj
index 6a697c9c4c0..50c19a42270 100644
--- a/src/metabase/search/config.clj
+++ b/src/metabase/search/config.clj
@@ -185,6 +185,7 @@
[:current-user-id pos-int?]
[:is-superuser? :boolean]
;; TODO only optional and maybe for tests, clean that up!
+ [:context {:optional true} [:maybe :keyword]]
[:is-impersonated-user? {:optional true} [:maybe :boolean]]
[:is-sandboxed-user? {:optional true} [:maybe :boolean]]
[:current-user-perms [:set perms.u/PathSchema]]
@@ -199,7 +200,7 @@
;;
[:created-at {:optional true} ms/NonBlankString]
[:created-by {:optional true} [:set {:min 1} ms/PositiveInt]]
- [:filter-items-in-personal-collection {:optional true} [:enum "only" "exclude"]]
+ [:filter-items-in-personal-collection {:optional true} [:enum "all" "only" "only-mine" "exclude" "exclude-others"]]
[:last-edited-at {:optional true} ms/NonBlankString]
[:last-edited-by {:optional true} [:set {:min 1} ms/PositiveInt]]
[:limit-int {:optional true} ms/Int]
diff --git a/src/metabase/search/filter.clj b/src/metabase/search/filter.clj
index cc9c731cad1..882f7e848f5 100644
--- a/src/metabase/search/filter.clj
+++ b/src/metabase/search/filter.clj
@@ -81,10 +81,26 @@
(defn personal-collections-where-clause
"Build a clause limiting the entries to those (not) within or within personal collections, if relevant.
WARNING: this method queries the appdb, and its approach will get very slow when there are many users!"
- [{filter-type :filter-items-in-personal-collection} collection-id-col]
- (when filter-type
- (let [parent-ids (t2/select-pks-vec :model/Collection :personal_owner_id [:not= nil])
- child-patterns (for [id parent-ids] (format "/%d/%%" id))]
+ [{filter-type :filter-items-in-personal-collection :keys [current-user-id] :as search-ctx} collection-id-col]
+ (case (or filter-type "all")
+ "all" nil
+
+ "only-mine"
+ [:or
+ [:= :collection.personal_owner_id current-user-id]
+ (into [:or]
+ (let [your-collection-ids (t2/select-pks-vec :model/Collection :personal_owner_id [:= current-user-id])
+ child-patterns (for [id your-collection-ids] (format "/%d/%%" id))]
+ (for [p child-patterns] [:like :collection.location p])))]
+
+ "exclude-others"
+ (let [with-filter #(personal-collections-where-clause
+ (assoc search-ctx :filter-items-in-personal-collection %)
+ collection-id-col)]
+ [:or (with-filter "only-mine") (with-filter "exclude")])
+
+ (let [personal-ids (t2/select-pks-vec :model/Collection :personal_owner_id [:not= nil])
+ child-patterns (for [id personal-ids] (format "/%d/%%" id))]
(case filter-type
"only"
`[:or
diff --git a/src/metabase/search/impl.clj b/src/metabase/search/impl.clj
index e79dbe252a1..80b0474d998 100644
--- a/src/metabase/search/impl.clj
+++ b/src/metabase/search/impl.clj
@@ -251,6 +251,7 @@
(mr/def ::search-context.input
[:map {:closed true}
[:search-string [:maybe ms/NonBlankString]]
+ [:context {:optional true} [:maybe :keyword]]
[:models [:maybe [:set SearchableModel]]]
[:current-user-id pos-int?]
[:is-impersonated-user? {:optional true} :boolean]
@@ -260,7 +261,7 @@
[:archived {:optional true} [:maybe :boolean]]
[:created-at {:optional true} [:maybe ms/NonBlankString]]
[:created-by {:optional true} [:maybe [:set ms/PositiveInt]]]
- [:filter-items-in-personal-collection {:optional true} [:maybe [:enum "only" "exclude"]]]
+ [:filter-items-in-personal-collection {:optional true} [:maybe [:enum "all" "only" "only-mine" "exclude" "exclude-others"]]]
[:last-edited-at {:optional true} [:maybe ms/NonBlankString]]
[:last-edited-by {:optional true} [:maybe [:set ms/PositiveInt]]]
[:limit {:optional true} [:maybe ms/Int]]
@@ -276,6 +277,7 @@
(mu/defn search-context :- SearchContext
"Create a new search context that you can pass to other functions like [[search]]."
[{:keys [archived
+ context
calculate-available-models?
created-at
created-by
@@ -304,17 +306,24 @@
[:content-verification :official-collections]
(deferred-tru "Content Management or Official Collections")))
(let [models (if (string? models) [models] models)
- ctx (cond-> {:archived? (boolean archived)
- :calculate-available-models? (boolean calculate-available-models?)
- :current-user-id current-user-id
- :current-user-perms current-user-perms
- :is-impersonated-user? is-impersonated-user?
- :is-sandboxed-user? is-sandboxed-user?
- :is-superuser? is-superuser?
- :models models
- :model-ancestors? (boolean model-ancestors?)
- :search-engine (parse-engine search-engine)
- :search-string search-string}
+ engine (parse-engine search-engine)
+ ctx (cond-> {:archived? (boolean archived)
+ :context (or context :unknown)
+ :calculate-available-models? (boolean calculate-available-models?)
+ :current-user-id current-user-id
+ :current-user-perms current-user-perms
+ :filter-items-in-personal-collection (or filter-items-in-personal-collection
+ (if (and (not= engine :search.engine/in-place)
+ (#{:search-app :command-palette} context))
+ "exclude-others"
+ "all"))
+ :is-impersonated-user? is-impersonated-user?
+ :is-sandboxed-user? is-sandboxed-user?
+ :is-superuser? is-superuser?
+ :models models
+ :model-ancestors? (boolean model-ancestors?)
+ :search-engine engine
+ :search-string search-string}
(some? created-at) (assoc :created-at created-at)
(seq created-by) (assoc :created-by created-by)
(some? filter-items-in-personal-collection) (assoc :filter-items-in-personal-collection filter-items-in-personal-collection)
--
GitLab