diff --git a/e2e/test/scenarios/collections/trash.cy.spec.js b/e2e/test/scenarios/collections/trash.cy.spec.js index 31a6a89819dc3..3bd4c38d35071 100644 --- a/e2e/test/scenarios/collections/trash.cy.spec.js +++ b/e2e/test/scenarios/collections/trash.cy.spec.js @@ -66,7 +66,9 @@ describe("scenarios > collections > trash", () => { toggleEllipsisMenuFor("Collection A"); cy.log("items in trash should have greyed out icons"); - cy.icon("model").should("have.attr", "color", "#949AAB"); + collectionTable().within(() => { + cy.icon("model").should("have.css", "color", "rgb(148, 154, 171)"); + }); cy.log("there should not be pins in the trash"); cy.findByTestId("pinned-items").should("not.exist"); diff --git a/frontend/src/metabase-types/api/collection.ts b/frontend/src/metabase-types/api/collection.ts index 2cebae072f48f..48b6ea56dd8ab 100644 --- a/frontend/src/metabase-types/api/collection.ts +++ b/frontend/src/metabase-types/api/collection.ts @@ -13,7 +13,12 @@ import type { UserId } from "./user"; export type RegularCollectionId = number; -export type CollectionId = RegularCollectionId | "root" | "personal" | "users"; +export type CollectionId = + | RegularCollectionId + | "root" + | "personal" + | "users" + | "trash"; export type CollectionContentModel = "card" | "dataset"; diff --git a/frontend/src/metabase/archive/actions.ts b/frontend/src/metabase/archive/actions.ts index 8f2ea7c0b2e20..6c8204bd031bc 100644 --- a/frontend/src/metabase/archive/actions.ts +++ b/frontend/src/metabase/archive/actions.ts @@ -1,9 +1,7 @@ import { push } from "react-router-redux"; import { t } from "ttag"; -import { TRASH_COLLECTION } from "metabase/entities/collections"; import { createThunkAction } from "metabase/lib/redux"; -import * as Urls from "metabase/lib/urls"; import { addUndo } from "metabase/redux/undo"; export const deletePermanently = createThunkAction( @@ -11,7 +9,7 @@ export const deletePermanently = createThunkAction( // pass in result of Entity.actions.delete(...) (entityDeleteAction: any) => async dispatch => { await dispatch(entityDeleteAction); - dispatch(push(Urls.collection(TRASH_COLLECTION))); + dispatch(push("/trash")); dispatch(addUndo({ message: t`This item has been permanently deleted.` })); }, ); diff --git a/frontend/src/metabase/collections/components/CollectionContent/CollectionContentView.tsx b/frontend/src/metabase/collections/components/CollectionContent/CollectionContentView.tsx index cf0245eefb0dc..bf90a83c621ce 100644 --- a/frontend/src/metabase/collections/components/CollectionContent/CollectionContentView.tsx +++ b/frontend/src/metabase/collections/components/CollectionContent/CollectionContentView.tsx @@ -7,6 +7,7 @@ import { t } from "ttag"; import _ from "underscore"; import ErrorBoundary from "metabase/ErrorBoundary"; +import { useGetCollectionQuery } from "metabase/api"; import { deletePermanently } from "metabase/archive/actions"; import { ArchivedEntityBanner } from "metabase/archive/components/ArchivedEntityBanner"; import { CollectionBulkActions } from "metabase/collections/components/CollectionBulkActions"; @@ -111,6 +112,9 @@ export const CollectionContentView = ({ ] = useToggle(false); const [uploadedFile, setUploadedFile] = useState(null); + // TODO: temp fix until we have the type field on effective_ancestors + const { data: trashCollection } = useGetCollectionQuery("trash"); + const saveFile = (file: File) => { setUploadedFile(file); openModelUploadModal(); @@ -245,7 +249,10 @@ export const CollectionContentView = ({ collection.effective_ancestors && _.last(collection.effective_ancestors); const canRestore = - !!parentCollection && isRootTrashCollection(parentCollection); + // !!parentCollection && isRootTrashCollection(parentCollection); + // TODO: temporarily use the fetched trashCollection to check if this is the root trash collection + // as we don't currently have the "type" field on the effective_ancestors + !!parentCollection && parentCollection.id === trashCollection?.id; return ( diff --git a/frontend/src/metabase/collections/components/CollectionLanding/CollectionLanding.tsx b/frontend/src/metabase/collections/components/CollectionLanding/CollectionLanding.tsx index 90680beeddb8c..e44420304b3fd 100644 --- a/frontend/src/metabase/collections/components/CollectionLanding/CollectionLanding.tsx +++ b/frontend/src/metabase/collections/components/CollectionLanding/CollectionLanding.tsx @@ -1,14 +1,15 @@ -import type { Location } from "history"; import type { ReactNode } from "react"; +import { useEffect } from "react"; +import { replace } from "react-router-redux"; -import { TRASH_COLLECTION } from "metabase/entities/collections"; +import { useGetCollectionQuery } from "metabase/api"; +import { useDispatch } from "metabase/lib/redux"; import { isNotNull } from "metabase/lib/types"; import { extractCollectionId } from "metabase/lib/urls"; import { CollectionContent } from "../CollectionContent"; export interface CollectionLandingProps { - location: Location; params: CollectionLandingParams; children?: ReactNode; } @@ -18,12 +19,22 @@ export interface CollectionLandingParams { } const CollectionLanding = ({ - location: { pathname }, params: { slug }, children, }: CollectionLandingProps) => { - const collectionId = - pathname === "/trash" ? TRASH_COLLECTION.id : extractCollectionId(slug); + const dispatch = useDispatch(); + const { data: trashCollection } = useGetCollectionQuery("trash"); + + const collectionId = extractCollectionId(slug); + + useEffect( + function redirectIfTrashCollection() { + if (trashCollection?.id === collectionId) { + dispatch(replace("/trash")); + } + }, + [dispatch, trashCollection?.id, collectionId], + ); if (!isNotNull(collectionId)) { return null; diff --git a/frontend/src/metabase/collections/components/TrashCollectionLanding/TrashCollectionLanding.tsx b/frontend/src/metabase/collections/components/TrashCollectionLanding/TrashCollectionLanding.tsx new file mode 100644 index 0000000000000..6db69b22d8c2b --- /dev/null +++ b/frontend/src/metabase/collections/components/TrashCollectionLanding/TrashCollectionLanding.tsx @@ -0,0 +1,14 @@ +import { useGetCollectionQuery } from "metabase/api"; +import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; + +import { CollectionContent } from "../CollectionContent"; + +export const TrashCollectionLanding = () => { + const { data, isLoading, error } = useGetCollectionQuery("trash"); + + return ( + + {data && } + + ); +}; diff --git a/frontend/src/metabase/collections/components/TrashCollectionLanding/index.ts b/frontend/src/metabase/collections/components/TrashCollectionLanding/index.ts new file mode 100644 index 0000000000000..8a2e4ac0d7b66 --- /dev/null +++ b/frontend/src/metabase/collections/components/TrashCollectionLanding/index.ts @@ -0,0 +1 @@ +export * from "./TrashCollectionLanding"; diff --git a/frontend/src/metabase/collections/utils.ts b/frontend/src/metabase/collections/utils.ts index 85878329b20ce..3c1a7125b7ff4 100644 --- a/frontend/src/metabase/collections/utils.ts +++ b/frontend/src/metabase/collections/utils.ts @@ -1,6 +1,5 @@ import { t } from "ttag"; -import { TRASH_COLLECTION } from "metabase/entities/collections/constants"; import { PLUGIN_COLLECTIONS } from "metabase/plugins"; import type { Collection, @@ -28,13 +27,13 @@ export function isPersonalCollection( } export function isRootTrashCollection( - collection: Pick, + collection: Pick, ): boolean { - return collection.id === TRASH_COLLECTION.id; + return collection?.type === "trash"; } export function isTrashedCollection( - collection: Pick, + collection: Pick, ): boolean { return isRootTrashCollection(collection) || collection.archived; } diff --git a/frontend/src/metabase/components/ItemsTable/BaseItemsTable.tsx b/frontend/src/metabase/components/ItemsTable/BaseItemsTable.tsx index a04b0e47a7f4b..712d5638050bf 100644 --- a/frontend/src/metabase/components/ItemsTable/BaseItemsTable.tsx +++ b/frontend/src/metabase/components/ItemsTable/BaseItemsTable.tsx @@ -270,7 +270,7 @@ const DefaultItemRenderer = ({ collection?.can_write && typeof onToggleSelected === "function"; const icon = item.getIcon(); - if (item.model === "card") { + if (item.model === "card" || item.archived) { icon.color = color("text-light"); } diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx index f45f0b5c8b2f2..1b8d45e180353 100644 --- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx +++ b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx @@ -272,7 +272,7 @@ function DashboardInner(props: DashboardProps) { const hasVisibleParameters = visibleParameters.length > 0; const canRestore = !!dashboard?.collection_id && - isRootTrashCollection({ id: dashboard?.collection_id }); + isRootTrashCollection({ type: dashboard?.collection?.type }); const shouldRenderAsNightMode = isNightMode && isFullscreen; diff --git a/frontend/src/metabase/entities/collections/constants.ts b/frontend/src/metabase/entities/collections/constants.ts index 8df61d83e626d..f8c44fe30852a 100644 --- a/frontend/src/metabase/entities/collections/constants.ts +++ b/frontend/src/metabase/entities/collections/constants.ts @@ -28,12 +28,3 @@ export const PERSONAL_COLLECTIONS = { can_write: false, is_personal: true, }; - -export const TRASH_COLLECTION = { - id: 13371339 as const, - name: t`Trash`, - icon: "trash" as const, - location: "/", - path: [ROOT_COLLECTION.id], - is_personal: false, -}; diff --git a/frontend/src/metabase/entities/collections/getInitialCollectionId.ts b/frontend/src/metabase/entities/collections/getInitialCollectionId.ts index aa646dfaee1ef..e87a25574ba37 100644 --- a/frontend/src/metabase/entities/collections/getInitialCollectionId.ts +++ b/frontend/src/metabase/entities/collections/getInitialCollectionId.ts @@ -1,13 +1,16 @@ import { createSelector } from "@reduxjs/toolkit"; import type { Location } from "history"; -import { canonicalCollectionId } from "metabase/collections/utils"; +import { + canonicalCollectionId, + isRootTrashCollection, +} from "metabase/collections/utils"; import * as Urls from "metabase/lib/urls/collections"; import { getUserPersonalCollectionId } from "metabase/selectors/user"; import type { Collection, CollectionId } from "metabase-types/api"; import type { State } from "metabase-types/store"; -import { ROOT_COLLECTION, TRASH_COLLECTION } from "./constants"; +import { ROOT_COLLECTION } from "./constants"; type Props = { collectionId?: Collection["id"]; @@ -53,7 +56,7 @@ const getInitialCollectionId = createSelector( (collections, personalCollectionId, ...collectionIds) => { const rootCollectionId = ROOT_COLLECTION.id as CollectionId; const validCollectionIds = collectionIds - .filter(id => id !== TRASH_COLLECTION.id) + .filter(id => !isRootTrashCollection(collections[id as CollectionId])) .concat(rootCollectionId) as CollectionId[]; if (personalCollectionId) { diff --git a/frontend/src/metabase/entities/collections/utils.ts b/frontend/src/metabase/entities/collections/utils.ts index 83437ed6b0e44..d8746d4cb942b 100644 --- a/frontend/src/metabase/entities/collections/utils.ts +++ b/frontend/src/metabase/entities/collections/utils.ts @@ -10,11 +10,7 @@ import type { IconName, IconProps } from "metabase/ui"; import type { Collection, CollectionContentModel } from "metabase-types/api"; import type { State } from "metabase-types/store"; -import { - ROOT_COLLECTION, - PERSONAL_COLLECTIONS, - TRASH_COLLECTION, -} from "./constants"; +import { ROOT_COLLECTION, PERSONAL_COLLECTIONS } from "./constants"; export function normalizedCollection(collection: Collection) { return isRootCollection(collection) ? ROOT_COLLECTION : collection; @@ -32,8 +28,8 @@ export function getCollectionIcon( return { name: "group" }; } - if (collection.id === TRASH_COLLECTION.id) { - return { name: TRASH_COLLECTION.icon }; + if (collection.type === "trash") { + return { name: "trash" }; } if (isRootPersonalCollection(collection)) { diff --git a/frontend/src/metabase/lib/urls/collections.ts b/frontend/src/metabase/lib/urls/collections.ts index 9c8dad585f675..7ae6d0af877a4 100644 --- a/frontend/src/metabase/lib/urls/collections.ts +++ b/frontend/src/metabase/lib/urls/collections.ts @@ -15,7 +15,7 @@ export const otherUsersPersonalCollections = () => "/collection/users"; type Collection = Pick< BaseCollection, - "id" | "name" | "originalName" | "personal_owner_id" + "id" | "name" | "originalName" | "personal_owner_id" | "type" >; function slugifyPersonalCollection(collection: Collection) { @@ -38,7 +38,9 @@ function slugifyPersonalCollection(collection: Collection) { return slug; } -export function collection(collection?: Pick) { +export function collection( + collection?: Pick, +) { const isSystemCollection = !collection || collection.id === null || typeof collection.id === "string"; diff --git a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer/MainNavbarContainer.tsx b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer/MainNavbarContainer.tsx index dd9374a6dd8bc..4cc6d019bfd89 100644 --- a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer/MainNavbarContainer.tsx +++ b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer/MainNavbarContainer.tsx @@ -3,6 +3,7 @@ import { useCallback, useMemo, useState } from "react"; import { connect } from "react-redux"; import _ from "underscore"; +import { useGetCollectionQuery } from "metabase/api"; import { logout } from "metabase/auth/actions"; import CreateCollectionModal from "metabase/collections/containers/CreateCollectionModal"; import { @@ -13,7 +14,6 @@ import Modal from "metabase/components/Modal"; import Bookmarks, { getOrderedBookmarks } from "metabase/entities/bookmarks"; import type { CollectionTreeItem } from "metabase/entities/collections"; import Collections, { - TRASH_COLLECTION, buildCollectionTree, getCollectionIcon, ROOT_COLLECTION, @@ -56,7 +56,6 @@ interface Props extends MainNavbarProps { bookmarks: Bookmark[]; collections: Collection[]; rootCollection: Collection; - trashCollection: Collection; hasDataAccess: boolean; hasOwnDatabase: boolean; allError: boolean; @@ -79,10 +78,7 @@ function MainNavbarContainer({ hasOwnDatabase, collections = [], rootCollection, - trashCollection, hasDataAccess, - allError, - allFetched, location, params, openNavbar, @@ -94,6 +90,12 @@ function MainNavbarContainer({ }: Props) { const [modal, setModal] = useState(null); + const { + data: trashCollection, + isLoading, + error, + } = useGetCollectionQuery("trash"); + const collectionTree = useMemo(() => { const preparedCollections = []; const userPersonalCollections = currentUserPersonalCollections( @@ -163,10 +165,12 @@ function MainNavbarContainer({ return null; }, [modal, closeModal, onChangeLocation]); + const allError = props.allError || !!error; if (allError) { return ; } + const allFetched = props.allFetched && !isLoading; if (!allFetched) { return ; } @@ -204,11 +208,6 @@ export default _.compose( entityAlias: "rootCollection", loadingAndErrorWrapper: false, }), - Collections.load({ - id: TRASH_COLLECTION.id, - entityAlias: "trashCollection", - loadingAndErrorWrapper: false, - }), Collections.loadList({ query: () => ({ tree: true, diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx index dd5e909e38797..d6e6db3d6c6a6 100644 --- a/frontend/src/metabase/query_builder/components/view/View.jsx +++ b/frontend/src/metabase/query_builder/components/view/View.jsx @@ -219,7 +219,7 @@ class View extends Component { const isNewQuestion = !isNative && Lib.sourceTableOrCardId(query) === null; const canRestore = - !!card.collection_id && isRootTrashCollection({ id: card.collection_id }); + !!card.collection && isRootTrashCollection(card.collection); return ( diff --git a/frontend/src/metabase/routes.jsx b/frontend/src/metabase/routes.jsx index 56452de5e5d8b..98ce055dc5903 100644 --- a/frontend/src/metabase/routes.jsx +++ b/frontend/src/metabase/routes.jsx @@ -11,6 +11,7 @@ import { Logout } from "metabase/auth/components/Logout"; import { ResetPassword } from "metabase/auth/components/ResetPassword"; import CollectionLanding from "metabase/collections/components/CollectionLanding"; import { MoveCollectionModal } from "metabase/collections/components/MoveCollectionModal"; +import { TrashCollectionLanding } from "metabase/collections/components/TrashCollectionLanding"; import ArchiveCollectionModal from "metabase/components/ArchiveCollectionModal"; import { Unauthorized } from "metabase/components/ErrorPages"; import NotFoundFallbackPage from "metabase/containers/NotFoundFallbackPage"; @@ -21,7 +22,6 @@ import { DashboardMoveModalConnected } from "metabase/dashboard/components/Dashb import { ArchiveDashboardModalConnected } from "metabase/dashboard/containers/ArchiveDashboardModal"; import { AutomaticDashboardAppConnected } from "metabase/dashboard/containers/AutomaticDashboardApp"; import { DashboardAppConnected } from "metabase/dashboard/containers/DashboardApp/DashboardApp"; -import { TRASH_COLLECTION } from "metabase/entities/collections"; import { ModalRoute } from "metabase/hoc/ModalRoute"; import { Route } from "metabase/hoc/Title"; import { HomePage } from "metabase/home/components/HomePage"; @@ -140,12 +140,11 @@ export const getRoutes = store => { {/* Send historical /archive route to trash - can remove in v52 */} - - diff --git a/frontend/test/__support__/server-mocks/collection.ts b/frontend/test/__support__/server-mocks/collection.ts index 4dee0191ca8e0..5d23062885ae7 100644 --- a/frontend/test/__support__/server-mocks/collection.ts +++ b/frontend/test/__support__/server-mocks/collection.ts @@ -1,10 +1,7 @@ import fetchMock from "fetch-mock"; import _ from "underscore"; -import { - ROOT_COLLECTION, - TRASH_COLLECTION, -} from "metabase/entities/collections"; +import { ROOT_COLLECTION } from "metabase/entities/collections"; import { SAVED_QUESTIONS_VIRTUAL_DB_ID, convertSavedQuestionToVirtualTable, @@ -20,6 +17,11 @@ import { createMockCollection } from "metabase-types/api/mocks"; import { PERMISSION_ERROR } from "./constants"; +const mockTrashCollection = createMockCollection({ + id: 20000000, + name: "Trash", +}); + export interface CollectionEndpoints { collections: Collection[]; rootCollection?: Collection; @@ -29,10 +31,10 @@ export interface CollectionEndpoints { export function setupCollectionsEndpoints({ collections, rootCollection = createMockCollection(ROOT_COLLECTION), - trashCollection = createMockCollection(TRASH_COLLECTION), + trashCollection = mockTrashCollection, }: CollectionEndpoints) { fetchMock.get("path:/api/collection/root", rootCollection); - fetchMock.get(`path:/api/collection/${TRASH_COLLECTION.id}`, trashCollection); + fetchMock.get(`path:/api/collection/trash`, trashCollection); fetchMock.get( { url: "path:/api/collection/tree",