Skip to content
Snippets Groups Projects
Unverified Commit e7efd35a authored by metamben's avatar metamben Committed by GitHub
Browse files

Support filtering by "app" and "page" models (#25223) 


* Support filtering by "app" and "page" models
* Fetch pages instead of dashboards for app sidebar
* Fix pages display

Co-authored-by: default avatarAnton Kulyk <kuliks.anton@gmail.com>
parent 910afcec
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarContainer.tsx
+ 7
− 9
View file @ e7efd35a
......@@ -17,7 +17,7 @@ import { MainNavbarProps, MainNavbarOwnProps, SelectedItem } from "./types";
import NavbarLoadingView from "./NavbarLoadingView";
import DataAppNavbarView from "./DataAppNavbarView";
const FETCHING_SEARCH_MODELS = ["dashboard", "dataset", "card"];
const FETCHING_SEARCH_MODELS = ["page"];
const LIMIT = 100;
function isAtDataAppHomePage(selectedItems: SelectedItem[]) {
......@@ -29,7 +29,7 @@ type NavbarModal = "MODAL_APP_SETTINGS" | "MODAL_NEW_PAGE" | null;
interface DataAppNavbarContainerProps extends MainNavbarProps {
dataApp: DataApp;
items: any[];
pages: any[];
selectedItems: SelectedItem[];
onChangeLocation: (location: LocationDescriptor) => void;
}
......@@ -45,7 +45,7 @@ type SearchRenderProps = {
function DataAppNavbarContainer({
dataApp,
items,
pages,
selectedItems,
onChangeLocation,
...props
......@@ -63,15 +63,13 @@ function DataAppNavbarContainer({
return [
{
type: "data-app-page",
id: getDataAppHomePageId(
items.filter(item => item.model === "dashboard"),
),
id: getDataAppHomePageId(pages),
},
];
}
return selectedItems;
}, [items, selectedItems]);
}, [pages, selectedItems]);
const onEditAppSettings = useCallback(() => {
setModal("MODAL_APP_SETTINGS");
......@@ -120,7 +118,7 @@ function DataAppNavbarContainer({
<DataAppNavbarView
{...props}
dataApp={dataApp}
items={items}
pages={pages}
selectedItems={finalSelectedItems}
onNewPage={onNewPage}
onEditAppSettings={onEditAppSettings}
......@@ -152,7 +150,7 @@ function DataAppNavbarContainerLoader({
return <NavbarLoadingView />;
}
return (
<DataAppNavbarContainer {...props} dataApp={dataApp} items={list} />
<DataAppNavbarContainer {...props} dataApp={dataApp} pages={list} />
);
}}
</Search.ListLoader>
......
frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbarView.tsx
+ 4
− 9
View file @ e7efd35a
import React, { useMemo } from "react";
import React from "react";
import _ from "underscore";
import { t } from "ttag";
......@@ -25,7 +25,7 @@ import {
interface Props extends MainNavbarProps {
dataApp: DataApp;
items: any[];
pages: any[];
selectedItems: SelectedItem[];
onEditAppSettings: () => void;
onNewPage: () => void;
......@@ -33,16 +33,11 @@ interface Props extends MainNavbarProps {
function DataAppNavbarView({
dataApp,
items,
pages,
selectedItems,
onEditAppSettings,
onNewPage,
}: Props) {
const appPages = useMemo(
() => items.filter(item => item.model === "dashboard"),
[items],
);
const { "data-app-page": dataAppPage } = _.indexBy(
selectedItems,
item => item.type,
......@@ -56,7 +51,7 @@ function DataAppNavbarView({
<SidebarHeading>{dataApp.collection.name}</SidebarHeading>
</SidebarHeadingWrapper>
<ul>
{appPages.map(page => (
{pages.map(page => (
<PaddedSidebarLink
key={page.id}
url={Urls.dataAppPage(dataApp, page)}
......
src/metabase/api/collection.clj
+ 49
− 13
View file @ e7efd35a
......@@ -122,7 +122,7 @@
(def ^:private valid-model-param-values
"Valid values for the `?model=` param accepted by endpoints in this namespace.
`no_models` is for nilling out the set because a nil model set is actually the total model set"
#{"card" "dataset" "collection" "dashboard" "pulse" "snippet" "no_models" "timeline"})
#{"card" "dataset" "collection" "app" "dashboard" "page" "pulse" "snippet" "no_models" "timeline"})
(def ^:private ModelString
(apply s/enum valid-model-param-values))
......@@ -348,9 +348,9 @@
[_ rows]
(map post-process-card-row rows))
(defmethod collection-children-query :dashboard
[_ collection {:keys [archived? pinned-state]}]
(-> {:select [:d.id :d.name :d.description :d.entity_id :d.collection_position [(hx/literal "dashboard") :model]
(defn- dashboard-query [collection {:keys [page? archived? pinned-state]}]
(-> {:select [:d.id :d.name :d.description :d.entity_id :d.collection_position
[(hx/literal (if page? "page" "dashboard")) :model]
[:u.id :last_edit_user] [:u.email :last_edit_email]
[:u.first_name :last_edit_first_name] [:u.last_name :last_edit_last_name]
[:r.timestamp :last_edit_timestamp]]
......@@ -367,10 +367,15 @@
[:= :r.model_id :d.id]
[:core_user :u] [:= :u.id :r.user_id]]
:where [:and
[:= :is_app_page page?]
[:= :collection_id (:id collection)]
[:= :archived (boolean archived?)]]}
(hh/merge-where (pinned-state->clause pinned-state))))
(defmethod collection-children-query :dashboard
[_ collection options]
(dashboard-query collection (assoc options :page? false)))
(defmethod post-process-collection-children :dashboard
[_ rows]
(map #(dissoc %
......@@ -378,12 +383,20 @@
:dataset_query)
rows))
(defmethod collection-children-query :collection
[_ collection {:keys [archived? collection-namespace pinned-state]}]
(defmethod collection-children-query :page
[_ collection options]
(dashboard-query collection (assoc options :page? true)))
(defmethod post-process-collection-children :page
[_ rows]
(post-process-collection-children :dashboard rows))
(defn- collection-query
[collection {:keys [app? archived? collection-namespace pinned-state]}]
(-> (assoc (collection/effective-children-query
collection
[:= :archived archived?]
[:= :namespace (u/qualified-name collection-namespace)])
collection
[:= :archived archived?]
[:= :namespace (u/qualified-name collection-namespace)])
;; We get from the effective-children-query a normal set of columns selected:
;; want to make it fit the others to make UNION ALL work
:select [:id
......@@ -391,11 +404,23 @@
:description
:entity_id
:personal_owner_id
[(hx/literal "collection") :model]
:authority_level])
[(hx/literal (if app? "app" "collection")) :model]
:authority_level
:app_id]
;; A simple left join would force us qualifying :id from
;; collection and that doesn't work with effective-children-query.
;; The sub-query makes sure that :app.id is only visible as :app_id.
:left-join [[{:select [[:id :app_id] :collection_id]
:from [:app]} :app]
[:= :app.collection_id :col.id]])
(hh/merge-where [(if app? :<> :=) :app_id nil])
;; the nil indicates that collections are never pinned.
(hh/merge-where (pinned-state->clause pinned-state nil))))
(defmethod collection-children-query :collection
[_ collection options]
(collection-query collection (assoc options :app? false)))
(defmethod post-process-collection-children :collection
[_ rows]
(for [row rows]
......@@ -406,10 +431,19 @@
(cond-> row
;; when fetching root collection, we might have personal collection
(:personal_owner_id row) (assoc :name (collection/user->personal-collection-name (:personal_owner_id row) :user))
(nil? (:app_id row)) (dissoc :app_id)
true (assoc :can_write (mi/can-write? Collection (:id row)))
true (dissoc :collection_position :display :moderated_status :icon :personal_owner_id
:collection_preview :dataset_query))))
(defmethod collection-children-query :app
[_ collection options]
(collection-query collection (assoc options :app? true)))
(defmethod post-process-collection-children :app
[_ rows]
(post-process-collection-children :collection rows))
(s/defn ^:private coalesce-edit-info :- last-edit/MaybeAnnotated
"Hoist all of the last edit information into a map under the key :last-edit-info. Considers this information present
if `:last_edit_user` is not nil."
......@@ -443,9 +477,11 @@
(defn- model-name->toucan-model [model-name]
(case (keyword model-name)
:collection Collection
:app Collection
:card Card
:dataset Card
:dashboard Dashboard
:page Dashboard
:pulse Pulse
:snippet NativeQuerySnippet
:timeline Timeline))
......@@ -467,7 +503,7 @@
are optional (not id, but last_edit_user for example) must have a type so that the union-all can unify the nil with
the correct column type."
[:id :name :description :entity_id :display [:collection_preview :boolean] :dataset_query
:model :collection_position :authority_level [:personal_owner_id :integer]
:model :collection_position :authority_level [:app_id :integer] [:personal_owner_id :integer]
:last_edit_email :last_edit_first_name :last_edit_last_name :moderated_status :icon
[:last_edit_user :integer] [:last_edit_timestamp :timestamp]])
......@@ -583,7 +619,7 @@
"Fetch a sequence of 'child' objects belonging to a Collection, filtered using `options`."
[{collection-namespace :namespace, :as collection} :- collection/CollectionWithLocationAndIDOrRoot
{:keys [models], :as options} :- CollectionChildrenOptions]
(let [valid-models (for [model-kw [:collection :dataset :card :dashboard :pulse :snippet :timeline]
(let [valid-models (for [model-kw [:app :collection :dataset :card :page :dashboard :pulse :snippet :timeline]
;; only fetch models that are specified by the `model` param; or everything if it's empty
:when (or (empty? models) (contains? models model-kw))
:let [toucan-model (model-name->toucan-model model-kw)
......
test/metabase/api/collection_test.clj
+ 56
− 1
View file @ e7efd35a
......@@ -403,7 +403,7 @@
items))
(defn- default-item [{:keys [model] :as item-map}]
(merge {:id true, :collection_position nil, :entity_id true}
(merge {:id true, :collection_position nil, :entity_id true, :app_id false}
(when (= model "collection")
{:authority_level nil})
(when (= model "card")
......@@ -436,6 +436,7 @@
:name (:name card)
:collection_position nil
:collection_preview true
:app_id nil
:display "table"
:description nil
:entity_id (:entity_id card)
......@@ -682,6 +683,54 @@
(is (= #{"card" "dash" "subcollection" "dataset"}
(into #{} (map :name) items))))))))
(deftest filter-facet-test
(testing "Filter facets"
(mt/with-temp* [Collection [_ {:name "Top level collection"}]
Collection [{app-coll-id :id} {:name "App with items"}]
App [{app-id :id} {:collection_id app-coll-id}]
Collection [_ {:name "subcollection"
:location (format "/%d/" app-coll-id)
:authority_level "official"}]
Card [_ {:name "card" :collection_id app-coll-id}]
Card [_ {:name "dataset" :dataset true :collection_id app-coll-id}]
Dashboard [_ {:name "dash" :collection_id app-coll-id}]
Dashboard [_ {:name "page" :collection_id app-coll-id :is_app_page true}]]
(let [items (->> "/items?models=dashboard&models=card&models=collection"
(str "collection/" app-coll-id)
(mt/user-http-request :rasta :get 200)
:data)]
(is (= #{"card" "dash" "subcollection"}
(into #{} (map :name) items))))
(let [items (->> "/items?models=dashboard&models=card&models=collection&models=dataset"
(str "collection/" app-coll-id)
(mt/user-http-request :rasta :get 200)
:data)]
(is (= #{"card" "dash" "subcollection" "dataset"}
(into #{} (map :name) items))))
(let [items (->> "/items?models=page"
(str "collection/" app-coll-id)
(mt/user-http-request :rasta :get 200)
:data)]
(is (= #{"page"}
(into #{} (map :name) items))))
(let [items (mt/user-http-request
:rasta :get 200 "collection/root/items?models=app")]
(is (partial= [{:id app-coll-id
:app_id app-id
:model "app"}]
(:data items))))
(let [items (mt/user-http-request
:rasta :get 200 "collection/root/items")]
(is (= #{["app" "App with items"]
["collection" "Top level collection"]
["collection" "Rasta Toucan's Personal Collection"]}
(into #{} (map (juxt :model :name)) (:data items)))))
(let [items (->> (str "collection/" app-coll-id "/items")
(mt/user-http-request :rasta :get 200)
:data)]
(is (= #{"card" "dash" "subcollection" "dataset" "page"}
(into #{} (map :name) items)))))))
(deftest children-sort-clause-test
(testing "Default sort"
(doseq [app-db [:mysql :h2 :postgres]]
......@@ -726,6 +775,7 @@
(is (= [{:id (:id snippet)
:name "My Snippet"
:entity_id (:entity_id snippet)
:app_id nil
:model "snippet"}]
(:data (mt/user-http-request :rasta :get 200 (format "collection/%d/items" (:id collection))))))
......@@ -733,6 +783,7 @@
(is (= [{:id (:id archived)
:name "Archived Snippet"
:entity_id (:entity_id archived)
:app_id nil
:model "snippet"}]
(:data (mt/user-http-request :rasta :get 200 (format "collection/%d/items?archived=true" (:id collection)))))))
......@@ -740,6 +791,7 @@
(is (= [{:id (:id snippet)
:name "My Snippet"
:entity_id (:entity_id snippet)
:app_id nil
:model "snippet"}]
(:data (mt/user-http-request :rasta :get 200 (format "collection/%d/items?model=snippet" (:id collection)))))))))))
......@@ -1067,6 +1119,7 @@
:description nil
:collection_position nil
:collection_preview true
:app_id nil
:display "table"
:moderated_status nil
:entity_id (:entity_id card)
......@@ -1220,10 +1273,12 @@
(is (= [{:id (:id snippet)
:name "My Snippet"
:entity_id (:entity_id snippet)
:app_id nil
:model "snippet"}
{:id (:id snippet-2)
:name "My Snippet 2"
:entity_id (:entity_id snippet-2)
:app_id nil
:model "snippet"}]
(only-test-items (:data (mt/user-http-request :rasta :get 200 "collection/root/items?namespace=snippets")))))
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment