New Collection Picker (#38315)

* Entity Picker Squash

rough table selection. dont fire me plz

make stuff prettier

make mysql work in table picker

make a tabbed entity picker

reorganize stuff

WIP collectionPicker

collection picker works

NextedItemPicker Styling

more collectionpicker work

make collectionpicker work

Single Picker UI

adding scroll left

Tabbed UI

add search

fix key?

Splitting up components

yolo animation

isolate search input

reorg and fix some types

nested item picker unit test

types are much better

scroll left when needed on mount

handle snippets and some other stuff

Modal unit tests

Modal transition, default options adjustments, fixing endless searching

allow all personal collections

fix some types

createNewCollection

disable collection creation for read-only collections

using entity loaders

goodbye refs

started working on types. Still some stuff to do

I give up. Ryan help plz

wip

wip

more wip

fix stuff

cool stuff

cool stuff

cool stuff

more stuff

fix backwards traversal

use effective_location

testing helpers

# Conflicts:
#	frontend/src/metabase/common/components/EntityPicker/components/NestedItemPicker/NestedItemPicker.tsx

dont use entity picker for move modal yet

add a logical ui location

takes a location like `/1/2/3/4/` and returns a "logical" ui location
that includes only collections that the user can see.

For instance, in the above location, if the user lacked permissions to
collection 3, they could still see collection 4 but would see it as a
child of 2, so it's effective ui location would be `/1/2/4/`

remove location from other model types

ie cards, snippets, etc would have a nil location. just remove

test for ui logical location

switch to existing `effective_location`

had reinvented the wheel. There's already an effective_location which
does exactly what we want.

Add can-write? for UI concerns

want it for collection picker UI info (remove or show greyed out). Tests
are failing but want for them to start

{:test 35, :pass 270, :fail 14, :error 0, :type :summary}

better personal collection pathing

no root permissions

Basic version of Models in Browse Data (#37707)

* Add Models and Databases tabs
* Models are organized by collection
* Exclude items in personal collections
* Simplify BrowseHeader

dashboard spec passing

Initial collection selected

infinite scroll right, fixing rebase issue

native and models e2e

getting better. remember to look at moderator icon styling

adding search result filter, collection lookup for new collection modal

use correct icons for special things

top folder for snippets

Endless API requests 

virtualize lists

e2e tests are.... green??

now green e2e plz

UI adjustments, empty collection state

Lots of moving around. Types should be fairly happy, and started writing unit tests

happy-ish unit tests?

fix backend tests

remove test/table pickers

nicer loader

use personal endpoint

remove questionPicker

use generic types where possible

resolving import order stuff

some type stuff sorted

 type checks 



more prettier

fix lint error

Nick is embarassed

types back to green

fix rebase conflicts

* new collection modal polish, bad nav link stuff

* removing TFolder

* gathering collection picker components

* auto selecting newly created collections

* handle confirm on Enter

* formCollectionPicker cleanup

* border on selected search result

* NavLink variants

* virtualize search results

* fixing icon color on variants

* empty collection and search results state

* default options spread, removing commented tests, actionButtons

* moving New Collection Dialog, localizing header

* make entityPickerModal generic

* cleanup

* update unit tests

* update e2e tests

* fix type exports

* fix 1 more unit test

* last test fixes plz

* better loading behaviors

* improved x-scrolling

* remove imperative handle on scroll, scroll to selected item on mount

* fix virtualizer type

* scroll after measuring

* clean up your hooks man

* smooth out loading behavior

* happy little typescript types

* update collection id type

* cancel pending search requests

* handle whitespace search

* cleanup

* more cleanup

* lets null coalesce instead

* fix type

* small type fix

* Make a more versatile and easy-to-use list virtualizer

* better mock

* fix rebase conflicts

* even more cleanup

* ok raffi was right about the scrolling

* test fixes

* submit button niceties

* wider new collection button

* make jsdom happy

* lighter navlink variant

---------

Co-authored-by: Ryan Laurie <iethree@gmail.com>

e2e/support/helpers/e2e-collection-helpers.js

@@ -83,3 +83,17 @@ export const moveOpenedCollectionTo = newParent => {
   // Make sure modal closed
   modal().should("not.exist");
 };
+
+export function pickEntity({ path, select }) {
+  if (path) {
+    cy.findByTestId("nested-item-picker").within(() => {
+      for (const [index, name] of path.entries()) {
+        cy.findByTestId(`item-picker-level-${index}`).findByText(name).click();
+      }
+    });
+  }
+
+  if (select) {
+    cy.findByTestId("entity-picker-modal").button("Select").click();
+  }
+}

e2e/support/helpers/e2e-ui-elements-helpers.js

@@ -33,6 +33,14 @@ export function modal() {
   return cy.get([MODAL_SELECTOR, LEGACY_MODAL_SELECTOR].join(","));
 }
 
+export function entityPickerModal() {
+  return cy.findByTestId("entity-picker-modal");
+}
+
+export function collectionOnTheGoModal() {
+  return cy.findByTestId("create-collection-on-the-go");
+}
+
 export function sidebar() {
   return cy.get("main aside");
 }

e2e/test/scenarios/collections/collections.cy.spec.js

@@ -22,6 +22,8 @@ import {
   openUnpinnedItemMenu,
   getPinnedSection,
   moveOpenedCollectionTo,
+  pickEntity,
+  entityPickerModal,
 } from "e2e/support/helpers";
 
 import { displaySidebarChildOf } from "./helpers/e2e-collections-sidebar.js";
@@ -60,15 +62,17 @@ describe("scenarios > collection defaults", () => {
           "Test collection",
         );
         cy.findByLabelText("Description").type("Test collection description");
-        cy.findByTestId("select-button").findByText("Our analytics").click();
+        cy.findByTestId("collection-picker-button")
+          .findByText("Our analytics")
+          .click();
       });
 
-      popover().within(() => {
-        cy.findByText(`Collection ${COLLECTIONS_COUNT}`).click();
+      pickEntity({
+        path: ["Our analytics", `Collection ${COLLECTIONS_COUNT}`],
+        select: true,
       });
 
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.findByText("Create").click();
+      cy.findByTestId("new-collection-modal").button("Create").click();
 
       cy.findByTestId("collection-name-heading").should(
         "have.text",
@@ -366,13 +370,13 @@ describe("scenarios > collection defaults", () => {
         // Click to choose which collection should this question be saved to
         // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
         cy.findByText(revokedUsersPersonalCollectionName).click();
-        popover().within(() => {
-          cy.findByText(/Collections/i);
-          cy.findByText(/My personal collection/i);
-          cy.findByText("Parent").should("not.exist");
-          cy.log("Reported failing from v0.34.3");
-          cy.findByText("Child");
-        });
+        pickEntity({ path: [revokedUsersPersonalCollectionName] });
+        pickEntity({ path: ["Collections", "Child"] });
+        entityPickerModal().button("Select").should("be.enabled");
+        cy.log("Reported failing from v0.34.3");
+        cy.findByTestId("entity-picker-modal")
+          .findByText("Parent")
+          .should("not.exist");
       });
     });
 
@@ -575,7 +579,7 @@ describe("scenarios > collection defaults", () => {
 
       cy.findByTestId("new-collection-modal").then(modal => {
         cy.findByText("Collection it's saved in").should("be.visible");
-        cy.findByTestId("select-button")
+        cy.findByTestId("collection-picker-button")
           .findByText("Third collection")
           .should("be.visible");
       });

e2e/test/scenarios/collections/instance-analytics.cy.spec.js

@@ -73,7 +73,9 @@ describeEE("scenarios > Metabase Analytics Collection (AuditV2) ", () => {
         cy.findByTestId("qb-header").findByText("Save").click();
 
         cy.findByTestId("save-question-modal").within(modal => {
-          cy.findByTestId("select-button").findByText("Custom reports");
+          cy.findByTestId("collection-picker-button").findByText(
+            "Custom reports",
+          );
           cy.findByText("Save").click();
         });
 

e2e/test/scenarios/collections/permissions.cy.spec.js

@@ -25,6 +25,7 @@ const PERMISSIONS = {
 
 describe("collection permissions", () => {
   beforeEach(() => {
+    cy.intercept("GET", "/api/search*").as("search");
     restore();
   });
 
@@ -50,7 +51,7 @@ describe("collection permissions", () => {
                   });
                   // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
                   cy.findByText("Dashboard").click();
-                  cy.findByTestId("select-button").findByText(
+                  cy.findByLabelText(/Which collection/).findByText(
                     "Second collection",
                   );
                 });
@@ -64,7 +65,7 @@ describe("collection permissions", () => {
                       cy.icon("add").click();
                     });
                     popover().findByText("Dashboard").click();
-                    cy.findByTestId("select-button").findByText(
+                    cy.findByLabelText(/Which collection/).findByText(
                       "Our analytics",
                     );
                   });
@@ -406,15 +407,15 @@ describe("collection permissions", () => {
                   ).click();
                 });
 
-                popover().within(() => {
-                  cy.findByText("My personal collection");
+                cy.findByLabelText("Select a collection").within(() => {
+                  cy.findByText("Read Only Tableton's Personal Collection");
                   // Test will fail on this step first
                   cy.findByText("First collection").should("not.exist");
                   // This is the second step that makes sure not even search returns collections with read-only access
-                  cy.icon("search").click();
-                  cy.findByPlaceholderText("Search")
-                    .click()
-                    .type("third{Enter}");
+                  cy.findByPlaceholderText("Search…").type("third{Enter}");
+
+                  cy.wait("@search");
+                  cy.findByText(/Loading/i).should("not.exist");
                   cy.findByText("Third collection").should("not.exist");
                 });
               });
@@ -432,7 +433,7 @@ describe("collection permissions", () => {
     // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
     cy.findByText("Save").click();
 
-    cy.findByTestId("select-button").findByText("Our analytics");
+    cy.findByLabelText(/Which collection/).findByText("Our analytics");
   });
 
   it("should load the collection permissions admin pages", () => {

e2e/test/scenarios/dashboard/dashboard.cy.spec.js

@@ -40,6 +40,8 @@ import {
   assertDashboardFixedWidth,
   assertDashboardFullWidth,
   createDashboardWithTabs,
+  entityPickerModal,
+  collectionOnTheGoModal,
 } from "e2e/support/helpers";
 import { GRID_WIDTH } from "metabase/lib/dashboard_grid";
 import {
@@ -147,25 +149,32 @@ describe("scenarios > dashboard", () => {
         cy.findByTestId("new-dashboard-modal").then(modal => {
           cy.findByRole("heading", { name: "New dashboard" });
           cy.findByLabelText("Name").type(NEW_DASHBOARD).blur();
-          cy.findByTestId("select-button")
+          cy.findByTestId("collection-picker-button")
             .should("have.text", "Our analytics")
             .click();
         });
-        popover().findByText("New collection").click({ force: true });
+        entityPickerModal()
+          .findByText("Create a new collection")
+          .click({ force: true });
         const NEW_COLLECTION = "Bar";
-
-        cy.findByTestId("new-collection-modal").then(modal => {
-          cy.findByRole("heading", { name: "New collection" });
-          cy.findByPlaceholderText("My new fantastic collection")
+        collectionOnTheGoModal().within(() => {
+          cy.findByText("Create a new collection");
+          cy.findByPlaceholderText(/My new collection/)
             .type(NEW_COLLECTION)
             .blur();
-          cy.button("Create").click();
+          cy.findByText("Create").click();
           cy.wait("@createCollection");
         });
-
-        cy.findByTestId("new-dashboard-modal").then(modal => {
+        entityPickerModal().within(() => {
+          cy.findByText(NEW_COLLECTION).click();
+          cy.button("Select").click();
+        });
+        modal().within(() => {
           cy.findByText("New dashboard");
-          cy.findByTestId("select-button").should("have.text", NEW_COLLECTION);
+          cy.findByTestId("collection-picker-button").should(
+            "have.text",
+            NEW_COLLECTION,
+          );
           cy.button("Create").click();
         });
 

e2e/test/scenarios/models/create.cy.spec.js

@@ -52,7 +52,10 @@ describe("scenarios > models > create", () => {
       cy.contains("button", "Save").click();
     });
     cy.findByTestId("save-question-modal").within(() => {
-      cy.findByTestId("select-button").should("have.text", "Third collection");
+      cy.findByLabelText(/Which collection should this go in/).should(
+        "have.text",
+        "Third collection",
+      );
     });
   });
 
@@ -70,8 +73,11 @@ describe("scenarios > models > create", () => {
       cy.contains("button", "Save").click();
     });
 
-    cy.findByTestId("save-question-modal").within(modal => {
-      cy.findByTestId("select-button").should("have.text", "Third collection");
+    cy.findByTestId("save-question-modal").within(() => {
+      cy.findByLabelText(/Which collection should this go in/).should(
+        "have.text",
+        "Third collection",
+      );
     });
   });
 });

e2e/test/scenarios/native/native.cy.spec.js

@@ -46,7 +46,10 @@ describe("scenarios > question > native", () => {
       cy.findByText("Save").click();
     });
     cy.findByTestId("save-question-modal").within(() => {
-      cy.findByTestId("select-button").should("have.text", "Third collection");
+      cy.findByLabelText(/Which collection should this go in/).should(
+        "have.text",
+        "Third collection",
+      );
     });
   });
 

e2e/test/scenarios/native/snippets/snippet-permissions.cy.spec.js

@@ -8,6 +8,7 @@ import {
   rightSidebar,
   setTokenFeatures,
   isOSS,
+  entityPickerModal,
 } from "e2e/support/helpers";
 
 const { ALL_USERS_GROUP } = USER_GROUPS;
@@ -108,8 +109,10 @@ describeEE("scenarios > question > snippets (EE)", () => {
 
     // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
     modal().within(() => cy.findByText("Top folder").click());
-    // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-    popover().within(() => cy.findByText("my favorite snippets").click());
+    entityPickerModal().within(() => {
+      cy.findByText("my favorite snippets").click();
+      cy.findByText("Select").click();
+    });
     cy.intercept("/api/collection/root/items?namespace=snippets").as(
       "updateList",
     );

e2e/test/scenarios/onboarding/navbar/new-menu.cy.spec.js

@@ -33,7 +33,7 @@ describe("metabase > scenarios > navbar > new menu", () => {
     });
 
     cy.findByTestId("new-collection-modal").then(modal => {
-      cy.findByTestId("select-button").findByText("Our analytics");
+      cy.findByTestId("collection-picker-button").findByText("Our analytics");
 
       cy.findByPlaceholderText("My new fantastic collection").type(
         "Test collection",

e2e/test/scenarios/permissions/reproductions/22727-readonly-collection-offered-on-save.cy.spec.js

@@ -37,7 +37,7 @@ describe("issue 22727", () => {
       cy.findByText(/^Replace original qeustion/).should("not.exist");
 
       // This part is an actual repro for https://github.com/metabase/metabase/issues/22727
-      cy.findByTestId("select-button-content")
+      cy.findByLabelText(/Which collection should this go in/)
         .invoke("text")
         .should("not.eq", "Our analytics");
     });

e2e/test/scenarios/permissions/reproductions/23981-root-collection-breadcrumbs.cy.spec.js

 import { SAMPLE_DB_ID, USERS, USER_GROUPS } from "e2e/support/cypress_data";
 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
 import {
-  popover,
   restore,
   visitQuestionAdhoc,
   getFullName,
+  entityPickerModal,
 } from "e2e/support/helpers";
 
 const { ALL_USERS_GROUP } = USER_GROUPS;
@@ -43,9 +43,11 @@ describe("issue 23981", () => {
     // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
     cy.findByText(`${getFullName(nocollection)}'s Personal Collection`).click();
 
-    popover().within(() => {
+    entityPickerModal().within(() => {
       cy.findByText("Our analytics").should("not.exist");
-      cy.findByText("Collections").should("be.visible");
+      cy.log('ensure that "Collections" is not selectable');
+      cy.findByText("Collections").should("be.visible").click();
+      cy.button("Select").should("be.disabled");
     });
   });
 });

e2e/test/scenarios/question/new.cy.spec.js

@@ -18,6 +18,10 @@ import {
   setTokenFeatures,
   describeOSS,
   queryBuilderHeader,
+  entityPickerModal,
+  collectionOnTheGoModal,
+  modal,
+  pickEntity,
 } from "e2e/support/helpers";
 
 const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE;
@@ -276,8 +280,11 @@ describe("scenarios > question > new", () => {
     cy.findByTestId("qb-header").within(() => {
       cy.findByText("Save").click();
     });
-    cy.findByTestId("save-question-modal").within(modal => {
-      cy.findByTestId("select-button").should("have.text", "Third collection");
+    cy.findByTestId("save-question-modal").within(() => {
+      cy.findByLabelText(/Which collection/).should(
+        "have.text",
+        "Third collection",
+      );
     });
   });
 
@@ -291,24 +298,25 @@ describe("scenarios > question > new", () => {
       cy.findByText("Orders").click();
     });
     cy.findByTestId("qb-header").findByText("Save").click();
-
-    cy.findByTestId("save-question-modal").then(modal => {
-      cy.findByTestId("select-button").click();
-    });
-
-    popover().findByText("New collection").click();
+    cy.findByTestId("save-question-modal")
+      .findByLabelText(/Which collection/)
+      .click();
+    entityPickerModal().findByText("Create a new collection").click();
 
     const NEW_COLLECTION = "Foo";
-    cy.findByTestId("new-collection-modal").then(modal => {
-      cy.findByPlaceholderText("My new fantastic collection").type(
-        NEW_COLLECTION,
-      );
+    collectionOnTheGoModal().within(() => {
+      cy.findByLabelText(/Give it a name/).type(NEW_COLLECTION);
       cy.findByText("Create").click();
     });
-
-    cy.findByTestId("save-question-modal").then(modal => {
-      cy.findByTestId("select-button").should("have.text", NEW_COLLECTION);
-      cy.button("Save").click();
+    entityPickerModal().findByText("Foo").click();
+    entityPickerModal().findByText("Select").click();
+    cy.findByTestId("save-question-modal").within(() => {
+      cy.findByText("Save new question");
+      cy.findByLabelText(/Which collection/).should(
+        "have.text",
+        NEW_COLLECTION,
+      );
+      cy.findByText("Save").click();
     });
 
     cy.get("header").findByText(NEW_COLLECTION);
@@ -322,6 +330,7 @@ describe("scenarios > question > new", () => {
       name: "Dashboard in root collection",
     };
     const myPersonalCollection = "My personal collection";
+    const myPersonalCollectionName = "Bobby Tables's Personal Collection";
 
     beforeEach(() => {
       cy.intercept("POST", "/api/card").as("createQuestion");
@@ -341,19 +350,20 @@ describe("scenarios > question > new", () => {
       });
 
       queryBuilderHeader().button("Save").click();
-      cy.findByTestId("save-question-modal").within(modal => {
-        cy.findByTestId("select-button").click();
-      });
+      cy.findByTestId("save-question-modal")
+        .findByLabelText(/Which collection/)
+        .click();
 
-      popover().findByText("My personal collection").click();
+      pickEntity({ path: [myPersonalCollectionName], select: true });
 
-      cy.findByTestId("save-question-modal").within(modal => {
-        cy.findByText("Save").click();
-        cy.wait("@createQuestion");
-      });
+      cy.findByTestId("save-question-modal").button("Save").click();
+      cy.wait("@createQuestion");
 
-      cy.get("#QuestionSavedModal").within(() => {
-        cy.findByText("Yes please!").click();
+      cy.findByTestId("save-question-modal").should("not.exist");
+
+      modal().within(() => {
+        cy.findByText(/add this to a dashboard/i);
+        cy.button("Yes please!").click();
       });
 
       cy.get("#AddToDashSelectDashModal").within(() => {

e2e/test/scenarios/question/question-management.cy.spec.js

@@ -392,7 +392,8 @@ describe(
 
                 cy.get(".Modal").within(() => {
                   cy.findByText("Create a new dashboard").click();
-                  cy.findByTestId("select-button").findByText(
+                  cy.findByLabelText(/Which collection/).should(
+                    "contain.text",
                     personalCollection,
                   );
                   cy.findByLabelText("Name").type("Foo", { delay: 0 });

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

@@ -56,7 +56,8 @@ export interface Collection {
   personal_owner_id?: UserId;
   is_personal?: boolean;
 
-  location?: string;
+  location: string | null;
+  effective_location?: string; // location path containing only those collections that the user has permission to access
   effective_ancestors?: Collection[];
 
   here?: CollectionContentModel[];
@@ -96,6 +97,8 @@ export interface CollectionItem {
   type?: string;
   can_write?: boolean;
   "last-edit-info"?: LastEditInfo;
+  location?: string;
+  effective_location?: string;
   getIcon: () => { name: IconName };
   getUrl: (opts?: Record<string, unknown>) => string;
   setArchived?: (isArchived: boolean) => void;
@@ -108,6 +111,7 @@ export interface CollectionListQuery {
   archived?: boolean;
   "exclude-other-user-collections"?: boolean;
   "exclude-archived"?: boolean;
+  "personal-only"?: boolean;
   namespace?: string;
   tree?: boolean;
 }

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

@@ -28,3 +28,14 @@ export const createMockCollectionItem = (
   getUrl: () => "/question/1",
   ...opts,
 });
+
+export const createMockCollectionItemFromCollection = (
+  opts?: Partial<Collection>,
+): CollectionItem =>
+  createMockCollectionItem({
+    ...opts,
+    id: opts?.id as number,
+    model: "collection",
+    type: undefined,
+    location: opts?.location || "/",
+  });

frontend/src/metabase-types/api/mocks/search.ts

@@ -23,6 +23,7 @@ export const createMockSearchResult = (
     archived: null,
     collection,
     collection_position: null,
+    can_write: null,
     table_id: 1,
     table_name: null,
     bookmark: null,

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

@@ -2,6 +2,7 @@ import type { UserId } from "metabase-types/api/user";
 
 import type { CardId } from "./card";
 import type { Collection, CollectionId } from "./collection";
+import type { DashboardId } from "./dashboard";
 import type { DatabaseId, InitialSyncStatus } from "./database";
 import type { FieldReference } from "./query";
 import type { TableId } from "./table";
@@ -54,8 +55,15 @@ export type CollectionEssentials = Pick<
   "id" | "name" | "authority_level"
 >;
 
+export type SearchResultId =
+  | CollectionId
+  | CardId
+  | DatabaseId
+  | TableId
+  | DashboardId;
+
 export interface SearchResult {
-  id: number;
+  id: SearchResultId;
   name: string;
   model: SearchModelType;
   description: string | null;
@@ -85,6 +93,7 @@ export interface SearchResult {
   creator_id: UserId | null;
   creator_common_name: string | null;
   created_at: string | null;
+  can_write: boolean | null;
 }
 
 export interface SearchListQuery {
@@ -96,4 +105,5 @@ export interface SearchListQuery {
   offset?: number;
   collection?: CollectionId;
   filter_items_in_personal_collection?: "only" | "exclude";
+  namespace?: "snippets";
 }

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

@@ -29,7 +29,7 @@ export interface User extends BaseUser {
   is_installer: boolean;
   has_invited_second_user: boolean;
   has_question_and_dashboard: boolean;
-  personal_collection_id: number;
+  personal_collection_id: CollectionId;
   sso_source: "saml" | null;
   custom_homepage: {
     dashboard_id: DashboardId;

frontend/src/metabase/browse/analytics.ts

 import { trackSchemaEvent } from "metabase/lib/analytics";
+import type { SearchResultId } from "metabase-types/api";
 
-export const trackModelClick = (modelId: number) =>
+export const trackModelClick = (modelId: SearchResultId) =>
   trackSchemaEvent("browse_data", "1-0-0", {
     event: "browse_data_model_clicked",
     model_id: modelId,