Skip to content
Snippets Groups Projects
Unverified Commit 5be95100 authored by Chris Truter's avatar Chris Truter Committed by GitHub
Browse files

For new search, default to excluding other people's collections (#50146)

parent 731497f6
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/search.clj
+ 4
− 2
View file @ 5be95100
......@@ -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*
......
src/metabase/search/config.clj
+ 2
− 1
View file @ 5be95100
......@@ -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]
......
src/metabase/search/filter.clj
+ 20
− 4
View file @ 5be95100
......@@ -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
......
src/metabase/search/impl.clj
+ 21
− 12
View file @ 5be95100
......@@ -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)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment