Return app collections as apps, page dashboards as pages (#25194)

* Return app collections as apps, page dashboards as pages
* Fix bug: descriptions of datasets should also be searchable

src/metabase/api/search.clj

@@ -283,9 +283,10 @@
       (update :select (fn [columns]
                         (cons [(hx/literal "dataset") :model] (rest columns))))))
 
-(s/defmethod search-query-for-model "collection"
-  [model search-ctx :- SearchContext]
-  (-> (base-query-for-model model search-ctx)
+(defn- shared-collection-impl
+  [model search-ctx]
+  (-> (base-query-for-model "collection" search-ctx)
+      (update :where (fn [where] [:and [(if (= model "app") :<> :=) :app.id nil] where]))
       (hh/left-join [CollectionBookmark :bookmark]
                     [:and
                      [:= :bookmark.collection_id :collection.id]
@@ -294,19 +295,40 @@
                     [:= :app.collection_id :collection.id])
       (add-collection-join-and-where-clauses :collection.id search-ctx)))
 
+(s/defmethod search-query-for-model "collection"
+  [model search-ctx :- SearchContext]
+  (shared-collection-impl model search-ctx))
+
+(s/defmethod search-query-for-model "app"
+  [model search-ctx :- SearchContext]
+  (-> (shared-collection-impl model search-ctx)
+      (update :select (fn [columns]
+                        (cons [(hx/literal model) :model] (rest columns))))))
+
 (s/defmethod search-query-for-model "database"
   [model search-ctx :- SearchContext]
   (base-query-for-model model search-ctx))
 
-(s/defmethod search-query-for-model "dashboard"
-  [model search-ctx :- SearchContext]
-  (-> (base-query-for-model model search-ctx)
+(defn- shared-dashboard-impl
+  [model search-ctx]
+  (-> (base-query-for-model "dashboard" search-ctx)
+      (update :where (fn [where] [:and [:= :dashboard.is_app_page (= model "page")] where]))
       (hh/left-join [DashboardBookmark :bookmark]
                     [:and
                      [:= :bookmark.dashboard_id :dashboard.id ]
                      [:= :bookmark.user_id api/*current-user-id*]])
       (add-collection-join-and-where-clauses :dashboard.collection_id search-ctx)))
 
+(s/defmethod search-query-for-model "dashboard"
+  [model search-ctx :- SearchContext]
+  (shared-dashboard-impl model search-ctx))
+
+(s/defmethod search-query-for-model "page"
+  [model search-ctx :- SearchContext]
+  (-> (shared-dashboard-impl model search-ctx)
+      (update :select (fn [columns]
+                        (cons [(hx/literal model) :model] (rest columns))))))
+
 (s/defmethod search-query-for-model "pulse"
   [model search-ctx :- SearchContext]
   ;; Pulses don't currently support being archived, omit if archived is true

src/metabase/search/config.clj

@@ -43,11 +43,13 @@
 (def model-to-db-model
   "Mapping from string model to the Toucan model backing it."
   {"dashboard"  Dashboard
+   "page"       Dashboard
    "metric"     Metric
    "segment"    Segment
    "card"       Card
    "dataset"    Card
    "collection" Collection
+   "app"        Collection
    "table"      Table
    "pulse"      Pulse
    "database"   Database})
@@ -55,7 +57,7 @@
 (def all-models
   "All valid models to search for. The order of this list also influences the order of the results: items earlier in the
   list will be ranked higher."
-  ["dashboard" "metric" "segment" "card" "dataset" "collection" "table" "pulse" "database"])
+  ["dashboard" "page" "metric" "segment" "card" "dataset" "collection" "app" "table" "pulse" "database"])
 
 (def ^:const displayed-columns
   "All of the result components that by default are displayed by the frontend."
@@ -76,11 +78,19 @@
    :dataset_query
    :description])
 
+(defmethod searchable-columns-for-model "dataset"
+  [_]
+  (searchable-columns-for-model "card"))
+
 (defmethod searchable-columns-for-model "dashboard"
   [_]
   [:name
    :description])
 
+(defmethod searchable-columns-for-model "page"
+  [_]
+  (searchable-columns-for-model "dashboard"))
+
 (defmethod searchable-columns-for-model "database"
   [_]
   [:name

test/metabase/api/search_test.clj

@@ -600,15 +600,17 @@
 (deftest card-dataset-query-test
   (testing "Search results should match a native query's dataset_query column, but not an MBQL query's one."
     ;; https://github.com/metabase/metabase/issues/24132
-    (mt/with-temp* [Card [_mbql-card   {:name          "Venues Count"
-                                        :query_type    "query"
-                                        :dataset_query (mt/mbql-query venues {:aggregation [[:count]]})}]
-                    Card [_native-card {:name          "Another SQL query"
-                                        :query_type    "native"
-                                        :dataset_query (mt/native-query {:query "SELECT COUNT(1) AS aggregation FROM venues"})}]]
-      (is (= ["Another SQL query"]
-             (->> (search-request-data :rasta :q "aggregation")
-                  (map :name)))))))
+    (let [native-card {:name          "Another SQL query"
+                       :query_type    "native"
+                       :dataset_query (mt/native-query {:query "SELECT COUNT(1) AS aggregation FROM venues"})}]
+      (mt/with-temp* [Card [_mbql-card   {:name          "Venues Count"
+                                          :query_type    "query"
+                                          :dataset_query (mt/mbql-query venues {:aggregation [[:count]]})}]
+                      Card [_native-card native-card]
+                      Card [_dataset     (assoc native-card :name "Dataset" :dataset true)]]
+        (is (= ["Another SQL query" "Dataset"]
+               (->> (search-request-data :rasta :q "aggregation")
+                    (map :name))))))))
 
 (deftest app-test
   (testing "App collections should come with app_id set"
@@ -618,6 +620,23 @@
                 (fn [result]
                   (cond-> result
                     (not (#{"metric" "segment"} (:model result))) (assoc-in [:collection :app_id] true)
-                    (= (:model result) "collection")              (assoc :app_id true)))
+                    (= (:model result) "collection")              (assoc :model "app" :app_id true)))
                 (default-results-with-collection))
-               (search-request-data :rasta :q "test")))))))
+               (search-request-data :rasta :q "test"))))))
+  (testing "App collections should filterable as \"app\""
+    (mt/with-temp* [Collection [collection {:name "App collection to find"}]
+                    App [_ {:collection_id (:id collection)}]
+                    Collection [_ {:name "Another collection to find"}]]
+      (is (partial= [(assoc (select-keys collection [:name])
+                            :model "app")]
+             (search-request-data :rasta :q "find" :models "app"))))))
+
+(deftest page-test
+  (testing "Search results should pages with model \"page\""
+    (mt/with-temp* [Dashboard [_ {:name "Not a page but contains important text!"}]
+                    Dashboard [page {:name        "Page"
+                                     :description "Contains important text!"
+                                     :is_app_page true}]]
+      (is (partial= [(assoc (select-keys page [:name :description])
+                            :model "page")]
+                    (search-request-data :rasta :q "important text" :models "page"))))))