Merge remote-tracking branch 'origin/more-search-metadata' into collections-project-bulk-actions

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)))

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")))))