Make Trash Usable - Dynamic Trash Collection Id (#42532)

* wip

* fixes hardcoded reference to trash id in sidebar

* remove TRAHS_COLLECTION

* fixes line and e2e tests

* fix invert logic mistake and fixes lint

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

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

frontend/src/metabase/archive/actions.ts

@@ -1,17 +1,15 @@
 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(
   "metabase/archive/DELETE_PERMANENTLY",
   // 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.` }));
   },
 );

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<File | null>(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 (
           <CollectionRoot {...dropzoneProps}>

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;

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 (
+    <LoadingAndErrorWrapper loading={isLoading} error={error} noWrapper>
+      {data && <CollectionContent collectionId={data.id} />}
+    </LoadingAndErrorWrapper>
+  );
+};

frontend/src/metabase/collections/components/TrashCollectionLanding/index.ts

@@ -0,0 +1 @@
+export * from "./TrashCollectionLanding";

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, "id">,
+  collection: Pick<Collection, "type">,
 ): boolean {
-  return collection.id === TRASH_COLLECTION.id;
+  return collection?.type === "trash";
 }
 
 export function isTrashedCollection(
-  collection: Pick<Collection, "id" | "archived">,
+  collection: Pick<Collection, "type" | "archived">,
 ): boolean {
   return isRootTrashCollection(collection) || collection.archived;
 }

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

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;
 

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,
-};

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

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

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<Collection, "id" | "name">) {
+export function collection(
+  collection?: Pick<Collection, "id" | "type" | "name">,
+) {
   const isSystemCollection =
     !collection || collection.id === null || typeof collection.id === "string";
 

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<NavbarModal>(null);
 
+  const {
+    data: trashCollection,
+    isLoading,
+    error,
+  } = useGetCollectionQuery("trash");
+
   const collectionTree = useMemo<CollectionTreeItem[]>(() => {
     const preparedCollections = [];
     const userPersonalCollections = currentUserPersonalCollections(
@@ -163,10 +165,12 @@ function MainNavbarContainer({
     return null;
   }, [modal, closeModal, onChangeLocation]);
 
+  const allError = props.allError || !!error;
   if (allError) {
     return <NavbarErrorView />;
   }
 
+  const allFetched = props.allFetched && !isLoading;
   if (!allFetched) {
     return <NavbarLoadingView />;
   }
@@ -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,

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 (
       <QueryBuilderViewHeaderContainer>

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 => {
           <Route path="search" title={t`Search`} component={SearchApp} />
           {/* Send historical /archive route to trash - can remove in v52 */}
           <Redirect path="archive" to="trash" replace />
-          <Redirect
-            path={`/collection/${TRASH_COLLECTION.id}-trash`}
-            to="trash"
-            replace
+          <Route
+            path="trash"
+            title={t`Trash`}
+            component={TrashCollectionLanding}
           />
-          <Route path="trash" title={t`Trash`} component={CollectionLanding} />
 
           <Route path="collection/users" component={IsAdmin}>
             <IndexRoute component={UserCollectionList} />