Hide elements in create new dashboard modal (#35277)

* Hide public collections in create a new dashboard modal

* Hide "+ New collection option" in FormCollectionPicker

* Hide public collections in create a new collection modal

* Add unit tests

* Update frontend/src/metabase/containers/AddToDashSelectDashModal/AddToDashSelectDashModal.unit.spec.tsx

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>

* Review: Make test name more explicit to avoid confusion

* Review: Removed unused `async`

* Review: Move more common task to `beforeEach`

---------

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>

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

@@ -52,6 +52,7 @@ export interface CreateCollectionFormOwnProps {
   collectionId?: Collection["id"]; // can be used by `getInitialCollectionId`
   onCreate?: (collection: Collection) => void;
   onCancel?: () => void;
+  showOnlyPersonalCollections?: boolean;
 }
 
 interface CreateCollectionFormStateProps {
@@ -89,6 +90,7 @@ function CreateCollectionForm({
   handleCreateCollection,
   onCreate,
   onCancel,
+  showOnlyPersonalCollections,
 }: Props) {
   const initialValues = useMemo(
     () => ({
@@ -131,6 +133,7 @@ function CreateCollectionForm({
           <FormCollectionPicker
             name="parent_id"
             title={t`Collection it's saved in`}
+            showOnlyPersonalCollections={showOnlyPersonalCollections}
           />
           <FormAuthorityLevelFieldContainer
             collectionParentId={values.parent_id}

frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.tsx

@@ -20,6 +20,7 @@ import { isValidCollectionId } from "metabase/collections/utils";
 
 import type { CollectionId } from "metabase-types/api";
 
+import { useSelector } from "metabase/lib/redux";
 import {
   PopoverItemPicker,
   MIN_POPOVER_WIDTH,
@@ -33,6 +34,7 @@ export interface FormCollectionPickerProps
   type?: "collections" | "snippet-collections";
   initialOpenCollectionId?: CollectionId;
   onOpenCollectionChange?: (collectionId: CollectionId) => void;
+  showOnlyPersonalCollections?: boolean;
 }
 
 function ItemName({
@@ -58,6 +60,7 @@ function FormCollectionPicker({
   type = "collections",
   initialOpenCollectionId,
   onOpenCollectionChange,
+  showOnlyPersonalCollections,
 }: FormCollectionPickerProps) {
   const id = useUniqueId();
   const [{ value }, { error, touched }, { setValue }] = useField(name);
@@ -96,6 +99,15 @@ function FormCollectionPicker({
 
   const [openCollectionId, setOpenCollectionId] =
     useState<CollectionId>("root");
+  const openCollection = useSelector(state =>
+    Collections.selectors.getObject(state, {
+      entityId: openCollectionId,
+    }),
+  );
+
+  const isOpenCollectionInPersonalCollection = openCollection?.is_personal;
+  const showCreateNewCollectionOption =
+    !showOnlyPersonalCollections || isOpenCollectionInPersonalCollection;
 
   const renderContent = useCallback(
     ({ closePopover }) => {
@@ -120,18 +132,26 @@ function FormCollectionPicker({
             onOpenCollectionChange?.(id);
             setOpenCollectionId(id);
           }}
+          showOnlyPersonalCollections={showOnlyPersonalCollections}
         >
-          <CreateCollectionOnTheGoButton openCollectionId={openCollectionId} />
+          {showCreateNewCollectionOption && (
+            <CreateCollectionOnTheGoButton
+              showOnlyPersonalCollections={showOnlyPersonalCollections}
+              openCollectionId={openCollectionId}
+            />
+          )}
         </PopoverItemPicker>
       );
     },
     [
-      value,
       type,
+      value,
       width,
-      setValue,
       initialOpenCollectionId,
+      showOnlyPersonalCollections,
+      showCreateNewCollectionOption,
       openCollectionId,
+      setValue,
       onOpenCollectionChange,
     ],
   );

frontend/src/metabase/containers/AddToDashSelectDashModal/AddToDashSelectDashModal.tsx

@@ -69,8 +69,8 @@ const AddToDashSelectDashModal = ({
     }),
   );
   const isOpenCollectionInPersonalCollection = openCollection?.is_personal;
-  const hideCreateNewDashboardOption =
-    isQuestionInPersonalCollection && !isOpenCollectionInPersonalCollection;
+  const showCreateNewDashboardOption =
+    !isQuestionInPersonalCollection || isOpenCollectionInPersonalCollection;
 
   const navigateToDashboard: Required<CreateDashboardFormOwnProps>["onCreate"] =
     dashboard => {
@@ -92,6 +92,7 @@ const AddToDashSelectDashModal = ({
   if (shouldCreateDashboard) {
     return (
       <CreateDashboardModal
+        showOnlyPersonalCollections={isQuestionInPersonalCollection}
         collectionId={card.collection_id}
         onCreate={navigateToDashboard}
         onClose={() => setShouldCreateDashboard(false)}
@@ -124,7 +125,7 @@ const AddToDashSelectDashModal = ({
         collectionId={initialOpenCollectionId}
         value={mostRecentlyViewedDashboardQuery.data?.id}
       />
-      {!hideCreateNewDashboardOption && (
+      {showCreateNewDashboardOption && (
         <Link onClick={() => setShouldCreateDashboard(true)} to="">
           <LinkContent>
             <Icon name="add" className="mx1" />

frontend/src/metabase/containers/AddToDashSelectDashModal/AddToDashSelectDashModal.unit.spec.tsx

@@ -8,7 +8,7 @@ import {
   setupDashboardCollectionItemsEndpoint,
   setupSearchEndpoints,
 } from "__support__/server-mocks";
-import { renderWithProviders, screen, waitFor } from "__support__/ui";
+import { renderWithProviders, screen, waitFor, within } from "__support__/ui";
 import {
   createMockCard,
   createMockCollection,
@@ -353,7 +353,6 @@ describe("AddToDashSelectDashModal", () => {
           await setup({
             collections: COLLECTIONS,
           });
-
           expect(
             screen.getByRole("heading", {
               name: PERSONAL_COLLECTION.name,
@@ -367,9 +366,13 @@ describe("AddToDashSelectDashModal", () => {
         });
 
         describe('"Create a new dashboard" option', () => {
-          it('should render "Create a new dashboard" option when opening the root collection (public collection)', async () => {
-            await setup();
+          beforeEach(async () => {
+            await setup({
+              collections: COLLECTIONS,
+            });
+          });
 
+          it('should render "Create a new dashboard" option when opening the root collection (public collection)', async () => {
             expect(
               screen.getByRole("heading", {
                 name: "Create a new dashboard",
@@ -378,8 +381,6 @@ describe("AddToDashSelectDashModal", () => {
           });
 
           it('should render "Create a new dashboard" option when opening public subcollections', async () => {
-            await setup();
-
             userEvent.click(
               screen.getByRole("heading", {
                 name: COLLECTION.name,
@@ -406,8 +407,6 @@ describe("AddToDashSelectDashModal", () => {
           });
 
           it('should render "Create a new dashboard" option when opening personal subcollections', async () => {
-            await setup();
-
             userEvent.click(
               screen.getByRole("heading", {
                 name: PERSONAL_COLLECTION.name,
@@ -432,6 +431,89 @@ describe("AddToDashSelectDashModal", () => {
               }),
             ).toBeInTheDocument();
           });
+
+          describe('when "Create a new dashboard" option is clicked', () => {
+            beforeEach(() => {
+              userEvent.click(
+                screen.getByRole("heading", {
+                  name: "Create a new dashboard",
+                }),
+              );
+              // Open "Create a new dashboard" modal
+              userEvent.click(screen.getByTestId("select-button"));
+            });
+
+            it("should render all collections", async () => {
+              expect(
+                screen.getByRole("heading", { name: "New dashboard" }),
+              ).toBeInTheDocument();
+              const popover = screen.getByRole("tooltip");
+              expect(popover).toBeInTheDocument();
+              expect(
+                await within(popover).findByRole("heading", {
+                  name: "Our analytics",
+                }),
+              ).toBeInTheDocument();
+              expect(
+                within(popover).getByRole("heading", {
+                  name: COLLECTION.name,
+                }),
+              ).toBeInTheDocument();
+              expect(
+                within(popover).getByRole("heading", {
+                  name: PERSONAL_COLLECTION.name,
+                }),
+              ).toBeInTheDocument();
+            });
+
+            it('should render "New collection" option', async () => {
+              expect(
+                screen.getByRole("heading", { name: "New dashboard" }),
+              ).toBeInTheDocument();
+
+              const popover = screen.getByRole("tooltip");
+              expect(popover).toBeInTheDocument();
+
+              expect(
+                await within(popover).findByText("New collection"),
+              ).toBeInTheDocument();
+            });
+
+            describe('when "New collection" option is clicked', () => {
+              beforeEach(async () => {
+                const popover = screen.getByRole("tooltip");
+
+                userEvent.click(
+                  await within(popover).findByText("New collection"),
+                );
+              });
+
+              it("should render all collections", async () => {
+                expect(
+                  screen.getByRole("heading", { name: "New collection" }),
+                ).toBeInTheDocument();
+                userEvent.click(screen.getByTestId("select-button"));
+
+                const popover = screen.getByRole("tooltip");
+                expect(popover).toBeInTheDocument();
+                expect(
+                  await within(popover).findByRole("heading", {
+                    name: "Our analytics",
+                  }),
+                ).toBeInTheDocument();
+                expect(
+                  within(popover).getByRole("heading", {
+                    name: COLLECTION.name,
+                  }),
+                ).toBeInTheDocument();
+                expect(
+                  within(popover).getByRole("heading", {
+                    name: PERSONAL_COLLECTION.name,
+                  }),
+                ).toBeInTheDocument();
+              });
+            });
+          });
         });
 
         describe("whether we should render dashboards in a collection", () => {
@@ -556,11 +638,13 @@ describe("AddToDashSelectDashModal", () => {
         });
 
         describe('"Create a new dashboard" option', () => {
-          it('should not render "Create a new dashboard" option when opening the root collection (public collection)', async () => {
+          beforeEach(async () => {
             await setup({
               card: CARD_IN_PERSONAL_COLLECTION,
             });
+          });
 
+          it('should not render "Create a new dashboard" option when opening the root collection (public collection)', () => {
             expect(
               screen.queryByRole("heading", {
                 name: "Create a new dashboard",
@@ -568,11 +652,7 @@ describe("AddToDashSelectDashModal", () => {
             ).not.toBeInTheDocument();
           });
 
-          it('should render "Create a new dashboard" option when opening personal subcollections', async () => {
-            await setup({
-              card: CARD_IN_PERSONAL_COLLECTION,
-            });
-
+          it('should render "Create a new dashboard" option when opening personal subcollections', () => {
             userEvent.click(
               screen.getByRole("heading", {
                 name: PERSONAL_COLLECTION.name,
@@ -597,6 +677,103 @@ describe("AddToDashSelectDashModal", () => {
               }),
             ).toBeInTheDocument();
           });
+
+          describe('when "Create a new dashboard" option is clicked when opening personal collections', () => {
+            beforeEach(() => {
+              // "Create a new dashboard" option only renders when opening personal collections
+              userEvent.click(
+                screen.getByRole("heading", {
+                  name: PERSONAL_COLLECTION.name,
+                }),
+              );
+              userEvent.click(
+                screen.getByRole("heading", {
+                  name: "Create a new dashboard",
+                }),
+              );
+            });
+
+            it("should render only personal collections", async () => {
+              expect(
+                screen.getByRole("heading", { name: "New dashboard" }),
+              ).toBeInTheDocument();
+              userEvent.click(screen.getByTestId("select-button"));
+
+              const popover = screen.getByRole("tooltip");
+              expect(popover).toBeInTheDocument();
+              expect(
+                await within(popover).findByRole("heading", {
+                  name: PERSONAL_COLLECTION.name,
+                }),
+              ).toBeInTheDocument();
+              expect(
+                within(popover).queryByRole("heading", {
+                  name: "Our analytics",
+                }),
+              ).not.toBeInTheDocument();
+              expect(
+                within(popover).queryByRole("heading", {
+                  name: COLLECTION.name,
+                }),
+              ).not.toBeInTheDocument();
+            });
+
+            it('should not render "New collection" option when opening the root collection (public collection)', async () => {
+              expect(
+                screen.getByRole("heading", { name: "New dashboard" }),
+              ).toBeInTheDocument();
+              userEvent.click(screen.getByTestId("select-button"));
+
+              const popover = screen.getByRole("tooltip");
+              expect(popover).toBeInTheDocument();
+              expect(
+                await within(popover).findByRole("heading", {
+                  name: PERSONAL_COLLECTION.name,
+                }),
+              ).toBeInTheDocument();
+              expect(
+                within(popover).queryByText("New collection"),
+              ).not.toBeInTheDocument();
+            });
+
+            describe('when "New collection" option is clicked when opening personal collections', () => {
+              beforeEach(async () => {
+                userEvent.click(screen.getByTestId("select-button"));
+                const popover = screen.getByRole("tooltip");
+
+                // "New collection" option only renders when opening personal collections
+                userEvent.click(
+                  await within(popover).findByTestId("expand-btn"),
+                );
+                userEvent.click(within(popover).getByText("New collection"));
+              });
+
+              it("should render only personal collections", async () => {
+                expect(
+                  screen.getByRole("heading", { name: "New collection" }),
+                ).toBeInTheDocument();
+                userEvent.click(screen.getByTestId("select-button"));
+
+                const popover = screen.getByRole("tooltip");
+                expect(popover).toBeInTheDocument();
+                expect(
+                  await within(popover).findByRole("heading", {
+                    name: PERSONAL_COLLECTION.name,
+                  }),
+                ).toBeInTheDocument();
+                expect(
+                  within(popover).queryByRole("heading", {
+                    name: "Our analytics",
+                  }),
+                ).not.toBeInTheDocument();
+                expect(
+                  within(popover).queryByRole("heading", {
+                    name: COLLECTION.name,
+                  }),
+                ).not.toBeInTheDocument();
+              });
+            });
+          });
         });
 
         describe("whether we should render dashboards in a collection", () => {

frontend/src/metabase/containers/CreateCollectionOnTheGo.tsx

@@ -16,6 +16,7 @@ interface State {
   enabled?: boolean;
   resumedValues?: Values;
   openCollectionId?: CollectionId;
+  showOnlyPersonalCollections?: boolean;
 }
 
 const Context = createContext<{
@@ -33,7 +34,12 @@ export function CreateCollectionOnTheGo({
     (newState: State) => setState({ ...state, ...newState }),
     [state, setState],
   );
-  const { enabled, resumedValues, openCollectionId } = state;
+  const {
+    enabled,
+    resumedValues,
+    openCollectionId,
+    showOnlyPersonalCollections,
+  } = state;
   return enabled ? (
     <CreateCollectionModal
       collectionId={openCollectionId}
@@ -44,6 +50,7 @@ export function CreateCollectionOnTheGo({
           resumedValues: { ...resumedValues, collection_id: collection.id },
         });
       }}
+      showOnlyPersonalCollections={showOnlyPersonalCollections}
     />
   ) : (
     <Context.Provider value={{ canCreateNew: true, updateState }}>
@@ -52,11 +59,15 @@ export function CreateCollectionOnTheGo({
   );
 }
 
+interface CreateCollectionOnTheGoButtonProps {
+  openCollectionId?: CollectionId;
+  showOnlyPersonalCollections?: boolean;
+}
+
 export function CreateCollectionOnTheGoButton({
   openCollectionId,
-}: {
-  openCollectionId?: CollectionId;
-}) {
+  showOnlyPersonalCollections,
+}: CreateCollectionOnTheGoButtonProps) {
   const { canCreateNew, updateState } = useContext(Context);
   const formik = useFormikContext<Values>();
   return canCreateNew && formik ? (
@@ -68,6 +79,7 @@ export function CreateCollectionOnTheGoButton({
           enabled: true,
           resumedValues: formik.values,
           openCollectionId,
+          showOnlyPersonalCollections,
         })
       }
     >

frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx

@@ -43,6 +43,7 @@ export interface CreateDashboardFormOwnProps {
   onCreate?: (dashboard: Dashboard) => void;
   onCancel?: () => void;
   initialValues?: CreateDashboardProperties | null;
+  showOnlyPersonalCollections?: boolean;
 }
 
 interface CreateDashboardFormStateProps {
@@ -78,6 +79,7 @@ function CreateDashboardForm({
   onCreate,
   onCancel,
   initialValues,
+  showOnlyPersonalCollections,
 }: Props) {
   const computedInitialValues = useMemo(
     () => ({
@@ -120,6 +122,7 @@ function CreateDashboardForm({
           <FormCollectionPicker
             name="collection_id"
             title={t`Which collection should this go in?`}
+            showOnlyPersonalCollections={showOnlyPersonalCollections}
           />
           <FormFooter>
             <FormErrorMessage inline />