diff --git a/src/metabase/api/search.clj b/src/metabase/api/search.clj index 10fdc77eb01b23b8f4e9723ff4e4310ed0e9f897..a9da4760069ccb29f97daf0fccc38c28caadefab 100644 --- a/src/metabase/api/search.clj +++ b/src/metabase/api/search.clj @@ -47,7 +47,7 @@ "Map with the various allowed search parameters, used to construct the SQL query" {:search-string (s/maybe su/NonBlankString) :archived? s/Bool - :collection (s/maybe su/IntGreaterThanZero) + :collection (s/maybe (s/cond-pre (s/eq :root) su/IntGreaterThanZero)) :visible-collections coll/VisibleCollections}) (defn- merge-search-select @@ -80,7 +80,11 @@ "Update the query to only include collections the user has access to" [query-map column-kwd {:keys [visible-collections collection]} :- SearchContext] (cond - collection + ;; The root collection is the same as a nil collection id + (= :root collection) + (h/merge-where query-map [:= column-kwd nil]) + + (number? collection) (h/merge-where query-map [:= column-kwd collection]) (= :all visible-collections) @@ -146,6 +150,7 @@ [{:keys [collection visible-collections] :as search-ctx} :- SearchContext] ;; If searching for a collection you don't have access to, no need to run a query (if (and collection + (not= :root collection) (not= :all visible-collections) (not (contains? visible-collections collection))) [] @@ -154,22 +159,32 @@ :when query-map] query-map)}))) +(def ^:private CollectionSearchParam + "Search parameters that can either be a `collection_id`, but in string form, or `root`" + (s/maybe (s/cond-pre (s/eq "root") su/IntString))) + (s/defn ^:private make-search-context :- SearchContext [search-string :- (s/maybe su/NonBlankString) archived-string :- (s/maybe su/BooleanString) - collection-id :- (s/maybe su/IntGreaterThanZero)] + collection :- CollectionSearchParam] {:search-string search-string :archived? (Boolean/parseBoolean archived-string) - :collection collection-id + :collection (cond + (= "root" collection) + :root + collection + (Long/parseLong collection) + :else + collection) :visible-collections (coll/permissions-set->visible-collection-ids @*current-user-permissions-set*)}) (defendpoint GET "/" - "Search Cards, Dashboards, Collections and Pulses, optionally filtered by `q`, `archived`, and/or `collection_id`." - [q archived collection_id] + "Search Cards, Dashboards, Collections and Pulses for the substring `q`." + [q archived collection] {q (s/maybe su/NonBlankString) archived (s/maybe su/BooleanString) - collection_id (s/maybe su/IntGreaterThanZero)} - (let [{:keys [visible-collections collection] :as search-ctx} (make-search-context q archived collection_id)] + collection CollectionSearchParam} + (let [{:keys [visible-collections collection] :as search-ctx} (make-search-context q archived collection)] ;; Throw if the user doesn't have access to any collections (check-403 (or (= :all visible-collections) (seq visible-collections))) diff --git a/test/metabase/api/search_test.clj b/test/metabase/api/search_test.clj index c9b0552cba46822738b0536eb6b61fa50492a651..5c579b1703ef1fffce49381f6868b2061b6f2f87 100644 --- a/test/metabase/api/search_test.clj +++ b/test/metabase/api/search_test.clj @@ -99,7 +99,7 @@ Pulse [_ {:name "pulse foo pulse2"}] Metric [_ {:name "metric foo metric"}] Segment [_ {:name "segment foo segment"}]] - (tu/boolean-ids-and-timestamps (set ((user->client :crowberto) :get 200 "search", :q "foo", :collection_id coll-id))))) + (tu/boolean-ids-and-timestamps (set ((user->client :crowberto) :get 200 "search", :q "foo", :collection coll-id))))) ;; Querying for a collection you don't have access to just returns empty (expect @@ -107,7 +107,7 @@ (tt/with-temp* [Collection [coll-1 {:name "collection 1"}] Collection [{coll-2-id :id} {:name "collection 2"}]] (perms/grant-collection-read-permissions! (group/all-users) coll-1) - ((user->client :rasta) :get 200 "search", :q "foo", :collection_id coll-2-id))) + ((user->client :rasta) :get 200 "search", :q "foo", :collection coll-2-id))) ;; Users with access to a collection should be able to search it (expect @@ -123,7 +123,7 @@ Metric [_ {:name "metric foo metric"}] Segment [_ {:name "segment foo segment"}]] (perms/grant-collection-read-permissions! (group/all-users) coll) - (tu/boolean-ids-and-timestamps (set ((user->client :rasta) :get 200 "search", :q "foo", :collection_id coll-id))))) + (tu/boolean-ids-and-timestamps (set ((user->client :rasta) :get 200 "search", :q "foo", :collection coll-id))))) ;; Collections a user doesn't have access to are automatically omitted from the results (expect @@ -140,3 +140,17 @@ Segment [_ {:name "segment foo segment"}]] (perms/grant-collection-read-permissions! (group/all-users) coll-1) (tu/boolean-ids-and-timestamps (set ((user->client :rasta) :get 200 "search", :q "foo"))))) + +;; Searching for the root collection will return all items with a nil collection_id +(expect + (set (remove #(contains? #{"collection" "metric" "segment"} (:type %)) default-search-results)) + (tt/with-temp* [Collection [{coll-id :id, :as coll} {:name "collection foo collection"}] + Card [_ {:name "card foo card"}] + Card [_ {:name "card foo card2", :collection_id coll-id}] + Dashboard [_ {:name "dashboard foo dashboard"}] + Dashboard [_ {:name "dashboard bar dashboard2", :collection_id coll-id}] + Pulse [_ {:name "pulse foo pulse"}] + Pulse [_ {:name "pulse foo pulse2", :collection_id coll-id}] + Metric [_ {:name "metric foo metric"}] + Segment [_ {:name "segment foo segment"}]] + (tu/boolean-ids-and-timestamps (set ((user->client :rasta) :get 200 "search", :q "foo", :collection "root")))))