Do not Move and Archive personal collections (#28229)

frontend/src/metabase-types/api/collection.ts

+import { UserId } from "./user";
+
 export type RegularCollectionId = number;
 
 export type CollectionId = RegularCollectionId | "root";
@@ -25,7 +27,7 @@ export interface Collection {
   authority_level?: "official" | null;
 
   parent_id?: CollectionId;
-  personal_owner_id?: number;
+  personal_owner_id?: UserId;
 
   location?: string;
   effective_ancestors?: Collection[];
@@ -38,7 +40,12 @@ export interface Collection {
   path?: CollectionId[];
 }
 
-type CollectionItemModel = "card" | "dataset" | "dashboard" | "pulse";
+type CollectionItemModel =
+  | "card"
+  | "dataset"
+  | "dashboard"
+  | "pulse"
+  | "collection";
 
 export interface CollectionItem<T = CollectionItemModel> {
   id: number;
@@ -49,6 +56,7 @@ export interface CollectionItem<T = CollectionItemModel> {
   collection_position?: number | null;
   collection_preview?: boolean | null;
   fully_parametrized?: boolean | null;
+  personal_owner_id?: UserId;
   getIcon: () => { name: string };
   getUrl: (opts?: Record<string, unknown>) => string;
   setArchived?: (isArchived: boolean) => void;

frontend/src/metabase/collections/components/ActionMenu/ActionMenu.tsx

@@ -3,9 +3,12 @@ import React, { useCallback } from "react";
 import { Bookmark, Collection, CollectionItem } from "metabase-types/api";
 import { ANALYTICS_CONTEXT } from "metabase/collections/constants";
 import {
+  canArchiveItem,
+  canMoveItem,
+  canPinItem,
+  canPreviewItem,
   isFullyParametrized,
   isItemPinned,
-  isItemQuestion,
   isPreviewEnabled,
   isPreviewShown,
 } from "metabase/collections/utils";
@@ -50,8 +53,10 @@ function ActionMenu({
   deleteBookmark,
 }: ActionMenuProps) {
   const isBookmarked = bookmarks && getIsBookmarked(item, bookmarks);
-  const isPreviewOptionShown =
-    isItemPinned(item) && isItemQuestion(item) && collection.can_write;
+  const canPin = canPinItem(item, collection);
+  const canPreview = canPreviewItem(item, collection);
+  const canMove = canMoveItem(item, collection);
+  const canArchive = canArchiveItem(item, collection);
 
   const handlePin = useCallback(() => {
     item.setPinned?.(!isItemPinned(item));
@@ -88,12 +93,12 @@ function ActionMenu({
         isBookmarked={isBookmarked}
         isPreviewShown={isPreviewShown(item)}
         isPreviewAvailable={isFullyParametrized(item)}
-        onPin={collection.can_write && item.setPinned ? handlePin : null}
-        onMove={collection.can_write && item.setCollection ? handleMove : null}
+        onPin={canPin ? handlePin : null}
+        onMove={canMove ? handleMove : null}
         onCopy={item.copy ? handleCopy : null}
-        onArchive={collection.can_write ? handleArchive : null}
+        onArchive={canArchive ? handleArchive : null}
         onToggleBookmark={handleToggleBookmark}
-        onTogglePreview={isPreviewOptionShown ? handleTogglePreview : null}
+        onTogglePreview={canPreview ? handleTogglePreview : null}
         analyticsContext={ANALYTICS_CONTEXT}
       />
     </EventSandbox>

frontend/src/metabase/collections/components/ActionMenu/ActionMenu.unit.spec.tsx

 import React from "react";
-import { render, screen } from "@testing-library/react";
+import { screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
+import { renderWithProviders } from "__support__/ui";
+import { Collection, CollectionItem } from "metabase-types/api";
 import {
   createMockCollection,
   createMockCollectionItem,
 } from "metabase-types/api/mocks";
-import ActionMenu, { ActionMenuProps } from "./ActionMenu";
+import ActionMenu from "./ActionMenu";
+
+interface SetupOpts {
+  item: CollectionItem;
+  collection?: Collection;
+}
+
+const setup = ({
+  item,
+  collection = createMockCollection({ can_write: true }),
+}: SetupOpts) => {
+  const onCopy = jest.fn();
+  const onMove = jest.fn();
+
+  renderWithProviders(
+    <ActionMenu
+      item={item}
+      collection={collection}
+      onCopy={onCopy}
+      onMove={onMove}
+    />,
+  );
+
+  return { onCopy, onMove };
+};
 
 describe("ActionMenu", () => {
   it("should show an option to hide preview for a pinned question", () => {
-    const props = getProps({
-      item: createMockCollectionItem({
-        model: "card",
-        collection_position: 1,
-        collection_preview: true,
-        setCollectionPreview: jest.fn(),
-      }),
-      collection: createMockCollection({
-        can_write: true,
-      }),
+    const item = createMockCollectionItem({
+      model: "card",
+      collection_position: 1,
+      collection_preview: true,
+      setCollectionPreview: jest.fn(),
     });
 
-    render(<ActionMenu {...props} />);
+    setup({ item });
+
     userEvent.click(screen.getByLabelText("ellipsis icon"));
     userEvent.click(screen.getByText("Don’t show visualization"));
 
-    expect(props.item.setCollectionPreview).toHaveBeenCalledWith(false);
+    expect(item.setCollectionPreview).toHaveBeenCalledWith(false);
   });
 
   it("should show an option to show preview for a pinned question", () => {
-    const props = getProps({
-      item: createMockCollectionItem({
-        model: "card",
-        collection_position: 1,
-        collection_preview: false,
-        setCollectionPreview: jest.fn(),
-      }),
-      collection: createMockCollection({
-        can_write: true,
-      }),
+    const item = createMockCollectionItem({
+      model: "card",
+      collection_position: 1,
+      collection_preview: false,
+      setCollectionPreview: jest.fn(),
     });
 
-    render(<ActionMenu {...props} />);
+    setup({ item });
+
     userEvent.click(screen.getByLabelText("ellipsis icon"));
     userEvent.click(screen.getByText("Show visualization"));
 
-    expect(props.item.setCollectionPreview).toHaveBeenCalledWith(true);
+    expect(item.setCollectionPreview).toHaveBeenCalledWith(true);
   });
 
   it("should not show an option to hide preview for a pinned model", () => {
-    const props = getProps({
+    setup({
       item: createMockCollectionItem({
         model: "dataset",
         collection_position: 1,
         setCollectionPreview: jest.fn(),
       }),
-      collection: createMockCollection({
-        can_write: true,
-      }),
     });
 
-    render(<ActionMenu {...props} />);
     userEvent.click(screen.getByLabelText("ellipsis icon"));
 
     expect(
       screen.queryByText("Don’t show visualization"),
     ).not.toBeInTheDocument();
   });
-});
 
-const getProps = (opts?: Partial<ActionMenuProps>): ActionMenuProps => ({
-  item: createMockCollectionItem(),
-  collection: createMockCollection(),
-  onCopy: jest.fn(),
-  onMove: jest.fn(),
-  ...opts,
+  it("should allow to move and archive regular collections", () => {
+    const item = createMockCollectionItem({
+      name: "Collection",
+      model: "collection",
+      setCollection: jest.fn(),
+      setArchived: jest.fn(),
+    });
+
+    const { onMove } = setup({ item });
+
+    userEvent.click(screen.getByLabelText("ellipsis icon"));
+    userEvent.click(screen.getByText("Move"));
+    expect(onMove).toHaveBeenCalledWith([item]);
+
+    userEvent.click(screen.getByLabelText("ellipsis icon"));
+    userEvent.click(screen.getByText("Archive"));
+    expect(item.setArchived).toHaveBeenCalledWith(true);
+  });
+
+  it("should not allow to move and archive personal collections", () => {
+    const item = createMockCollectionItem({
+      name: "My personal collection",
+      model: "collection",
+      personal_owner_id: 1,
+      setCollection: jest.fn(),
+      setArchived: jest.fn(),
+    });
+
+    setup({ item });
+
+    userEvent.click(screen.getByLabelText("ellipsis icon"));
+    expect(screen.queryByText("Move")).not.toBeInTheDocument();
+    expect(screen.queryByText("Archive")).not.toBeInTheDocument();
+  });
 });

frontend/src/metabase/collections/components/BulkActions.jsx

@@ -12,6 +12,7 @@ import CollectionMoveModal from "metabase/containers/CollectionMoveModal";
 import CollectionCopyEntityModal from "metabase/collections/components/CollectionCopyEntityModal";
 
 import { ANALYTICS_CONTEXT } from "metabase/collections/constants";
+import { canArchiveItem, canMoveItem } from "metabase/collections/utils";
 import {
   ActionBarContent,
   ActionBarText,
@@ -55,6 +56,7 @@ const SelectionControls = ({
 function BulkActions(props) {
   const {
     selected,
+    collection,
     selectedItems,
     selectedAction,
     onArchive,
@@ -64,6 +66,10 @@ function BulkActions(props) {
     onCopy,
     isNavbarOpen,
   } = props;
+
+  const canMove = selected.every(item => canMoveItem(item, collection));
+  const canArchive = selected.every(item => canArchiveItem(item, collection));
+
   return (
     <BulkActionBar showing={selected.length > 0} isNavbarOpen={isNavbarOpen}>
       {/* NOTE: these padding and grid sizes must be carefully matched
@@ -71,12 +77,8 @@ function BulkActions(props) {
       <ActionBarContent>
         <SelectionControls {...props} />
         <BulkActionControls
-          onArchive={
-            _.all(selected, item => item.setArchived) ? onArchive : null
-          }
-          onMove={
-            _.all(selected, item => item.setCollection) ? onMoveStart : null
-          }
+          onArchive={canArchive ? onArchive : null}
+          onMove={canMove ? onMoveStart : null}
         />
         <ActionBarText>
           {ngettext(

frontend/src/metabase/collections/containers/CollectionContent.jsx

@@ -283,6 +283,7 @@ function CollectionContent({
                       </div>
                       <BulkActions
                         selected={selected}
+                        collection={collection}
                         onSelectAll={handleSelectAll}
                         onSelectNone={clear}
                         onArchive={handleBulkArchive}

frontend/src/metabase/collections/utils.ts

@@ -66,6 +66,33 @@ export function isItemQuestion(item: CollectionItem) {
   return item.model === "card";
 }
 
+export function isItemCollection(item: CollectionItem) {
+  return item.model === "collection";
+}
+
+export function canPinItem(item: CollectionItem, collection: Collection) {
+  return collection.can_write && item.setPinned != null;
+}
+
+export function canPreviewItem(item: CollectionItem, collection: Collection) {
+  return collection.can_write && isItemPinned(item) && isItemQuestion(item);
+}
+
+export function canMoveItem(item: CollectionItem, collection: Collection) {
+  return (
+    collection.can_write &&
+    item.setCollection != null &&
+    !(isItemCollection(item) && isPersonalCollection(item))
+  );
+}
+
+export function canArchiveItem(item: CollectionItem, collection: Collection) {
+  return (
+    collection.can_write &&
+    !(isItemCollection(item) && isPersonalCollection(item))
+  );
+}
+
 export function isPreviewShown(item: CollectionItem) {
   return isPreviewEnabled(item) && isFullyParametrized(item);
 }

frontend/test/metabase/scenarios/collections/collections.cy.spec.js

@@ -10,6 +10,7 @@ import {
   closeNavigationSidebar,
   openCollectionMenu,
   visitCollection,
+  getFullName,
 } from "__support__/e2e/helpers";
 import { USERS, USER_GROUPS } from "__support__/e2e/cypress_data";
 import { displaySidebarChildOf } from "./helpers/e2e-collections-sidebar.js";
@@ -374,6 +375,18 @@ describe("scenarios > collection defaults", () => {
       });
     });
 
+    it("should not be able to move or archive a personal collection", () => {
+      cy.visit("/collection/root");
+
+      openEllipsisMenuFor(getPersonalCollectionName(USERS.admin));
+
+      popover().within(() => {
+        cy.findByText("Bookmark").should("be.visible");
+        cy.findByText("Move").should("not.exist");
+        cy.findByText("Archive").should("not.exist");
+      });
+    });
+
     describe("bulk actions", () => {
       describe("selection", () => {
         it("should be possible to apply bulk selection to all items (metabase#14705)", () => {
@@ -394,6 +407,19 @@ describe("scenarios > collection defaults", () => {
           cy.findByTestId("bulk-action-bar").should("not.be.visible");
         });
 
+        it("should not be possible to archive or move a personal collection via bulk actions", () => {
+          cy.visit("/collection/root");
+
+          selectItemUsingCheckbox(
+            getPersonalCollectionName(USERS.admin),
+            "person",
+          );
+
+          cy.findByText("1 item selected").should("be.visible");
+          cy.button("Move").should("be.disabled");
+          cy.button("Archive").should("be.disabled");
+        });
+
         function bulkSelectDeselectWorkflow() {
           cy.visit("/collection/root");
           selectItemUsingCheckbox("Orders");
@@ -478,6 +504,10 @@ describe("scenarios > collection defaults", () => {
   });
 });
 
+function getPersonalCollectionName(user) {
+  return `${getFullName(USERS.admin)}'s Personal Collection`;
+}
+
 function openEllipsisMenuFor(item) {
   cy.findByText(item)
     .closest("tr")