From 1c5d4146833eba35f48fbf469c8911fcdcbabe03 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin <alexander.polyankin@metabase.com> Date: Wed, 6 Jul 2022 16:09:42 +0300 Subject: [PATCH] Select collections for selected questions, models, and dashboards in the sidebar (#23725) --- frontend/src/metabase-types/api/dashboard.ts | 1 + .../src/metabase-types/api/mocks/dashboard.ts | 1 + frontend/src/metabase/collections/utils.ts | 6 ++ .../metabase/nav/containers/AppBar/AppBar.tsx | 2 +- .../MainNavbar/BookmarkList/BookmarkList.tsx | 7 +- .../MainNavbar/MainNavbarContainer.tsx | 70 +++++++++++++++---- .../containers/MainNavbar/MainNavbarView.tsx | 34 +++++---- .../nav/containers/MainNavbar/types.ts | 14 ++-- frontend/src/metabase/selectors/app.ts | 8 ++- 9 files changed, 93 insertions(+), 50 deletions(-) diff --git a/frontend/src/metabase-types/api/dashboard.ts b/frontend/src/metabase-types/api/dashboard.ts index cd07829159d..b7ef2d57b0d 100644 --- a/frontend/src/metabase-types/api/dashboard.ts +++ b/frontend/src/metabase-types/api/dashboard.ts @@ -7,6 +7,7 @@ import { CardId, SavedCard } from "metabase-types/types/Card"; export interface Dashboard { id: number; + collection_id: number | null; name: string; description: string | null; model?: string; diff --git a/frontend/src/metabase-types/api/mocks/dashboard.ts b/frontend/src/metabase-types/api/mocks/dashboard.ts index f9b33ad1133..62af61d80bf 100644 --- a/frontend/src/metabase-types/api/mocks/dashboard.ts +++ b/frontend/src/metabase-types/api/mocks/dashboard.ts @@ -2,6 +2,7 @@ import { Dashboard } from "metabase-types/api"; export const createMockDashboard = (opts?: Partial<Dashboard>): Dashboard => ({ id: 1, + collection_id: null, name: "Dashboard", ordered_cards: [], can_write: true, diff --git a/frontend/src/metabase/collections/utils.ts b/frontend/src/metabase/collections/utils.ts index f002db571ba..495e0da839a 100644 --- a/frontend/src/metabase/collections/utils.ts +++ b/frontend/src/metabase/collections/utils.ts @@ -92,6 +92,12 @@ export function isFullyParametrized(item: Item) { return item.fully_parametrized ?? true; } +export function coerceCollectionId( + collectionId: number | null | undefined, +): string | number { + return collectionId == null ? "root" : collectionId; +} + // API requires items in "root" collection be persisted with a "null" collection ID // Also ensure it's parsed as a number export function canonicalCollectionId( diff --git a/frontend/src/metabase/nav/containers/AppBar/AppBar.tsx b/frontend/src/metabase/nav/containers/AppBar/AppBar.tsx index 4c733877343..b43b5d3ff46 100644 --- a/frontend/src/metabase/nav/containers/AppBar/AppBar.tsx +++ b/frontend/src/metabase/nav/containers/AppBar/AppBar.tsx @@ -24,7 +24,7 @@ const mapStateToProps = (state: State, props: RouterProps) => ({ isNewButtonVisible: getIsNewButtonVisible(state), isProfileLinkVisible: getIsProfileLinkVisible(state), isCollectionPathVisible: getIsCollectionPathVisible(state, props), - isQuestionLineageVisible: getIsQuestionLineageVisible(state), + isQuestionLineageVisible: getIsQuestionLineageVisible(state, props), }); const mapDispatchToProps = { diff --git a/frontend/src/metabase/nav/containers/MainNavbar/BookmarkList/BookmarkList.tsx b/frontend/src/metabase/nav/containers/MainNavbar/BookmarkList/BookmarkList.tsx index 709a8118733..cc2648fd954 100644 --- a/frontend/src/metabase/nav/containers/MainNavbar/BookmarkList/BookmarkList.tsx +++ b/frontend/src/metabase/nav/containers/MainNavbar/BookmarkList/BookmarkList.tsx @@ -1,7 +1,6 @@ import React, { useCallback, useEffect, useState } from "react"; import { t } from "ttag"; import { connect } from "react-redux"; -import _ from "underscore"; import "./sortable.css"; @@ -18,7 +17,7 @@ import { PLUGIN_COLLECTIONS } from "metabase/plugins"; import Bookmarks from "metabase/entities/bookmarks"; import * as Urls from "metabase/lib/urls"; -import { SelectedEntityItem } from "../types"; +import { SelectedItem } from "../types"; import { SidebarHeading } from "../MainNavbar.styled"; import { DragIcon, SidebarBookmarkItem } from "./BookmarkList.styled"; @@ -29,7 +28,7 @@ const mapDispatchToProps = { interface CollectionSidebarBookmarksProps { bookmarks: BookmarksType; - selectedItem?: SelectedEntityItem; + selectedItem?: SelectedItem; onSelect: () => void; onDeleteBookmark: (bookmark: Bookmark) => void; reorderBookmarks: ({ @@ -45,7 +44,7 @@ interface BookmarkItemProps { bookmark: Bookmark; index: number; isSorting: boolean; - selectedItem?: SelectedEntityItem; + selectedItem?: SelectedItem; onSelect: () => void; onDeleteBookmark: (bookmark: Bookmark) => void; } diff --git a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer.tsx b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer.tsx index 6f46e6725a9..2c3c4d67502 100644 --- a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer.tsx +++ b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer.tsx @@ -9,7 +9,14 @@ import { IconProps } from "metabase/components/Icon"; import Modal from "metabase/components/Modal"; import LoadingSpinner from "metabase/components/LoadingSpinner"; -import { Bookmark, BookmarksType, Collection, User } from "metabase-types/api"; +import Question from "metabase-lib/lib/Question"; +import { + Bookmark, + BookmarksType, + Collection, + Dashboard, + User, +} from "metabase-types/api"; import { State } from "metabase-types/store"; import Bookmarks, { getOrderedBookmarks } from "metabase/entities/bookmarks"; @@ -25,10 +32,13 @@ import { getHasOwnDatabase, getHasDataAccess, } from "metabase/new_query/selectors"; +import { getQuestion } from "metabase/query_builder/selectors"; +import { getDashboard } from "metabase/dashboard/selectors"; import { nonPersonalOrArchivedCollection, currentUserPersonalCollections, + coerceCollectionId, } from "metabase/collections/utils"; import * as Urls from "metabase/lib/urls"; @@ -52,6 +62,8 @@ function mapStateToProps(state: State) { hasDataAccess: getHasDataAccess(state), hasOwnDatabase: getHasOwnDatabase(state), bookmarks: getOrderedBookmarks(state), + question: getQuestion(state), + dashboard: getDashboard(state), }; } @@ -75,6 +87,8 @@ type Props = { bookmarks: BookmarksType; collections: Collection[]; rootCollection: Collection; + question?: Question; + dashboard?: Dashboard; hasDataAccess: boolean; hasOwnDatabase: boolean; allFetched: boolean; @@ -99,6 +113,8 @@ function MainNavbarContainer({ hasOwnDatabase, collections = [], rootCollection, + question, + dashboard, hasDataAccess, allFetched, location, @@ -129,23 +145,49 @@ function MainNavbarContainer({ }; }, [isOpen, openNavbar, closeNavbar]); - const selectedItem = useMemo<SelectedItem>(() => { + const selectedItems = useMemo<SelectedItem[]>(() => { const { pathname } = location; const { slug } = params; - if (pathname.startsWith("/collection")) { - const id = pathname.startsWith("/collection/users") - ? "users" - : Urls.extractCollectionId(slug); - return { type: "collection", id }; + const isCollectionPath = pathname.startsWith("/collection"); + const isUsersCollectionPath = pathname.startsWith("/collection/users"); + const isQuestionPath = pathname.startsWith("/question"); + const isModelPath = pathname.startsWith("/model"); + const isDashboardPath = pathname.startsWith("/dashboard"); + + if (isCollectionPath) { + return [ + { + id: isUsersCollectionPath ? "users" : Urls.extractCollectionId(slug), + type: "collection", + }, + ]; } - if (pathname.startsWith("/dashboard")) { - return { type: "dashboard", id: Urls.extractEntityId(slug) }; + if (isDashboardPath && dashboard) { + return [ + { + id: dashboard.id, + type: "dashboard", + }, + { + id: coerceCollectionId(dashboard.collection_id), + type: "collection", + }, + ]; } - if (pathname.startsWith("/question") || pathname.startsWith("/model")) { - return { type: "card", id: Urls.extractEntityId(slug) }; + if ((isQuestionPath || isModelPath) && question) { + return [ + { + id: question.id(), + type: "card", + }, + { + id: coerceCollectionId(question.collectionId()), + type: "collection", + }, + ]; } - return { type: "non-entity", url: pathname }; - }, [location, params]); + return [{ url: pathname, type: "non-entity" }]; + }, [location, params, question, dashboard]); const collectionTree = useMemo<CollectionTreeItem[]>(() => { if (!rootCollection) { @@ -220,7 +262,7 @@ function MainNavbarContainer({ currentUser={currentUser} collections={collectionTree} hasOwnDatabase={hasOwnDatabase} - selectedItem={selectedItem} + selectedItems={selectedItems} hasDataAccess={hasDataAccess} reorderBookmarks={reorderBookmarks} handleCreateNewCollection={onCreateNewCollection} diff --git a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarView.tsx b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarView.tsx index ac94b326497..7139966fbb1 100644 --- a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarView.tsx +++ b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarView.tsx @@ -1,5 +1,6 @@ import React, { useCallback } from "react"; import { t } from "ttag"; +import _ from "underscore"; import { BookmarksType, Collection, User } from "metabase-types/api"; @@ -43,7 +44,7 @@ type Props = { hasDataAccess: boolean; hasOwnDatabase: boolean; collections: CollectionTreeItem[]; - selectedItem: SelectedItem; + selectedItems: SelectedItem[]; handleCloseNavbar: () => void; handleLogout: () => void; handleCreateNewCollection: () => void; @@ -67,15 +68,18 @@ function MainNavbarView({ bookmarks, collections, hasOwnDatabase, - selectedItem, + selectedItems, hasDataAccess, reorderBookmarks, handleCreateNewCollection, handleCloseNavbar, }: Props) { - const isNonEntityLinkSelected = selectedItem.type === "non-entity"; - const isCollectionSelected = - selectedItem.type === "collection" && selectedItem.id !== "users"; + const { + card: cardItem, + collection: collectionItem, + dashboard: dashboardItem, + "non-entity": nonEntityItem, + } = _.indexBy(selectedItems, item => item.type); const onItemSelect = useCallback(() => { if (isSmallScreen()) { @@ -89,7 +93,7 @@ function MainNavbarView({ <SidebarSection> <ul> <HomePageLink - isSelected={isNonEntityLinkSelected && selectedItem.url === "/"} + isSelected={nonEntityItem?.url === "/"} icon="home" onClick={onItemSelect} url="/" @@ -103,9 +107,7 @@ function MainNavbarView({ <SidebarSection> <BookmarkList bookmarks={bookmarks} - selectedItem={ - selectedItem.type !== "non-entity" ? selectedItem : undefined - } + selectedItem={cardItem ?? dashboardItem ?? collectionItem} onSelect={onItemSelect} reorderBookmarks={reorderBookmarks} /> @@ -119,7 +121,7 @@ function MainNavbarView({ /> <Tree data={collections} - selectedId={isCollectionSelected ? selectedItem.id : undefined} + selectedId={collectionItem?.id} onSelect={onItemSelect} TreeNode={SidebarCollectionLink} role="tree" @@ -134,10 +136,7 @@ function MainNavbarView({ <BrowseLink icon="database" url={BROWSE_URL} - isSelected={ - isNonEntityLinkSelected && - selectedItem.url.startsWith(BROWSE_URL) - } + isSelected={nonEntityItem?.url?.startsWith(BROWSE_URL)} onClick={onItemSelect} data-metabase-event="NavBar;Data Browse" > @@ -147,10 +146,9 @@ function MainNavbarView({ <AddYourOwnDataLink icon="add" url={ADD_YOUR_OWN_DATA_URL} - isSelected={ - isNonEntityLinkSelected && - selectedItem.url.startsWith(ADD_YOUR_OWN_DATA_URL) - } + isSelected={nonEntityItem?.url?.startsWith( + ADD_YOUR_OWN_DATA_URL, + )} onClick={onItemSelect} data-metabase-event="NavBar;Add your own data" > diff --git a/frontend/src/metabase/nav/containers/MainNavbar/types.ts b/frontend/src/metabase/nav/containers/MainNavbar/types.ts index d124367d77c..47d3f6a7821 100644 --- a/frontend/src/metabase/nav/containers/MainNavbar/types.ts +++ b/frontend/src/metabase/nav/containers/MainNavbar/types.ts @@ -1,11 +1,5 @@ -export type SelectedEntityItem = { - type: "card" | "collection" | "dashboard"; +export interface SelectedItem { + type: "card" | "collection" | "dashboard" | "non-entity"; id?: number | string; -}; - -export type SelectedNonEntityItem = { - type: "non-entity"; - url: string; -}; - -export type SelectedItem = SelectedEntityItem | SelectedNonEntityItem; + url?: string; +} diff --git a/frontend/src/metabase/selectors/app.ts b/frontend/src/metabase/selectors/app.ts index 3aaf2e52703..4e87003793d 100644 --- a/frontend/src/metabase/selectors/app.ts +++ b/frontend/src/metabase/selectors/app.ts @@ -29,6 +29,7 @@ const PATHS_WITH_COLLECTION_BREADCRUMBS = [ /\/model\//, /\/dashboard\//, ]; +const PATHS_WITH_QUESTION_LINEAGE = [/\/question/, /\/model/]; export const getRouterPath = (state: State, props: RouterProps) => { return props.location.pathname; @@ -139,10 +140,11 @@ export const getIsCollectionPathVisible = createSelector( ); export const getIsQuestionLineageVisible = createSelector( - [getQuestion, getOriginalQuestion], - (question, originalQuestion) => + [getQuestion, getOriginalQuestion, getRouterPath], + (question, originalQuestion, path) => question != null && !question.isSaved() && originalQuestion != null && - !originalQuestion.isDataset(), + !originalQuestion.isDataset() && + PATHS_WITH_QUESTION_LINEAGE.some(pattern => pattern.test(path)), ); -- GitLab