Adding pagination to All Personal Collections page (#36255)

* Adding pagination to All Personal Collections page

* adding unit test

* test adjustments

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

@@ -62,15 +62,13 @@ describe("personal collections", () => {
       });
 
       cy.visit("/collection/root");
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.findByText("Your personal collection");
+      cy.findByRole("tree").findByText("Your personal collection");
       navigationSidebar().within(() => {
         cy.icon("ellipsis").click();
       });
       popover().findByText("Other users' personal collections").click();
       cy.location("pathname").should("eq", "/collection/users");
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.findByText(/All personal collections/i);
+      cy.findByTestId("browsercrumbs").findByText(/All personal collections/i);
       Object.values(USERS).forEach(user => {
         const FULL_NAME = `${user.first_name} ${user.last_name}`;
         cy.findByText(FULL_NAME);

e2e/test/scenarios/onboarding/urls.cy.spec.js

@@ -106,18 +106,6 @@ describe("URLs", () => {
       cy.location("pathname").should("eq", "/collection/users");
     });
 
-    it("should slugify users' personal collection URLs", () => {
-      cy.visit("/collection/users");
-      // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-      cy.findByText(getFullName(normal)).click();
-      cy.location("pathname").should(
-        "eq",
-        `/collection/${NORMAL_PERSONAL_COLLECTION_ID}-${getUsersPersonalCollectionSlug(
-          normal,
-        )}`,
-      );
-    });
-
     it("should open slugified URLs correctly", () => {
       cy.visit(`/collection/${FIRST_COLLECTION_ID}-first-collection`);
       cy.findByTestId("collection-name-heading").should(

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

@@ -31,6 +31,7 @@ export const createMockUserListResult = (
   last_name: "Tableton",
   common_name: "Testy Tableton",
   email: "user@metabase.test",
+  personal_collection_id: 2,
   ...opts,
 });
 

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

+import type { CollectionId } from "./collection";
 import type { DashboardId } from "./dashboard";
 
 export type UserId = number;
@@ -40,6 +41,11 @@ export interface UserListResult {
   last_name: string | null;
   common_name: string;
   email: string;
+  personal_collection_id: CollectionId;
+}
+
+export interface UserListMetadata {
+  total: number;
 }
 
 // Used when hydrating `creator` property
@@ -57,5 +63,7 @@ export type UserInfo = Pick<
 >;
 
 export type UserListQuery = {
-  recipients: boolean;
+  recipients?: boolean;
+  limit?: number;
+  offset?: number;
 };

frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.ts

@@ -4,11 +4,15 @@ import type {
   UseEntityListQueryResult,
 } from "metabase/common/hooks/use-entity-list-query";
 import { useEntityListQuery } from "metabase/common/hooks/use-entity-list-query";
-import type { UserListQuery, UserListResult } from "metabase-types/api";
+import type {
+  UserListQuery,
+  UserListResult,
+  UserListMetadata,
+} from "metabase-types/api";
 
 export const useUserListQuery = (
   props: UseEntityListQueryProps<UserListQuery> = {},
-): UseEntityListQueryResult<UserListResult> => {
+): UseEntityListQueryResult<UserListResult, UserListMetadata> => {
   return useEntityListQuery(props, {
     fetchList: Users.actions.fetchList,
     getList: Users.selectors.getList,

frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.unit.spec.tsx

@@ -9,12 +9,12 @@ import {
   waitForLoaderToBeRemoved,
   within,
 } from "__support__/ui";
-import { createMockUserInfo } from "metabase-types/api/mocks";
+import { createMockUserListResult } from "metabase-types/api/mocks";
 import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper/LoadingAndErrorWrapper";
 
 import { useUserListQuery } from "./use-user-list-query";
 
-const TEST_USER = createMockUserInfo();
+const TEST_USER = createMockUserListResult();
 
 type TestComponentProps = { getRecipients?: boolean };
 

frontend/src/metabase/components/BrowserCrumbs/BrowserCrumbs.jsx

@@ -15,7 +15,7 @@ const Crumb = ({ children }) => (
 );
 
 const BrowserCrumbs = ({ crumbs, analyticsContext }) => (
-  <BrowserCrumbsRoot>
+  <BrowserCrumbsRoot data-testid="browsercrumbs">
     {crumbs
       .filter(c => c)
       .map((crumb, index, crumbs) => (

frontend/src/metabase/containers/UserCollectionList.jsx

-/* eslint-disable react/prop-types */
-import { connect } from "react-redux";
-
-import * as Urls from "metabase/lib/urls";
-import { color } from "metabase/lib/colors";
-
-import Card from "metabase/components/Card";
-import { Icon } from "metabase/core/components/Icon";
-import { Grid } from "metabase/components/Grid";
-import Link from "metabase/core/components/Link";
-import BrowserCrumbs from "metabase/components/BrowserCrumbs";
-
-import User from "metabase/entities/users";
-import Collection, {
-  ROOT_COLLECTION,
-  PERSONAL_COLLECTIONS,
-} from "metabase/entities/collections";
-import {
-  CardContent,
-  ListGridItem,
-  ListHeader,
-  ListRoot,
-} from "./UserCollectionList.styled";
-
-function mapStateToProps(state) {
-  return {
-    collectionsById: state.entities.collections,
-  };
-}
-
-const UserCollectionList = ({ collectionsById }) => (
-  <ListRoot>
-    <ListHeader>
-      <BrowserCrumbs
-        crumbs={[
-          { title: ROOT_COLLECTION.name, to: Urls.collection({ id: "root" }) },
-          { title: PERSONAL_COLLECTIONS.name },
-        ]}
-      />
-    </ListHeader>
-    <User.ListLoader>
-      {({ list }) => {
-        return (
-          <div>
-            <Grid>
-              {
-                // map through all users that have logged in at least once
-                // which gives them a personal collection ID
-                list.map(
-                  user =>
-                    user.personal_collection_id && (
-                      <ListGridItem key={user.personal_collection_id}>
-                        <Link
-                          to={Urls.collection(
-                            collectionsById[user.personal_collection_id],
-                          )}
-                        >
-                          <Card className="p2" hoverable>
-                            <CardContent>
-                              <Icon
-                                name="person"
-                                classname="mr1"
-                                color={color("text-medium")}
-                                size={18}
-                              />
-                              <h3>{user.common_name}</h3>
-                            </CardContent>
-                          </Card>
-                        </Link>
-                      </ListGridItem>
-                    ),
-                )
-              }
-            </Grid>
-          </div>
-        );
-      }}
-    </User.ListLoader>
-  </ListRoot>
-);
-
-export default Collection.loadList()(
-  connect(mapStateToProps)(UserCollectionList),
-);

frontend/src/metabase/containers/UserCollectionList.styled.tsx

 import styled from "@emotion/styled";
 import { GridItem } from "metabase/components/Grid";
+import { color } from "metabase/lib/colors";
 
 export const ListRoot = styled.div`
   padding: 0 4rem;
@@ -11,6 +12,10 @@ export const ListHeader = styled.div`
 
 export const ListGridItem = styled(GridItem)`
   width: 33.33%;
+
+  &:hover {
+    color: ${color("brand")};
+  }
 `;
 
 export const CardContent = styled.div`

frontend/src/metabase/containers/UserCollectionList.tsx

+import * as Urls from "metabase/lib/urls";
+import { color } from "metabase/lib/colors";
+
+import Card from "metabase/components/Card";
+import { Icon } from "metabase/core/components/Icon";
+import { Grid } from "metabase/components/Grid";
+import Link from "metabase/core/components/Link";
+import BrowserCrumbs from "metabase/components/BrowserCrumbs";
+
+import { useUserListQuery } from "metabase/common/hooks/use-user-list-query";
+import PaginationControls from "metabase/components/PaginationControls";
+
+import { Box, Flex, Loader } from "metabase/ui";
+import {
+  ROOT_COLLECTION,
+  PERSONAL_COLLECTIONS,
+} from "metabase/entities/collections";
+
+import { usePeopleQuery } from "metabase/admin/people/hooks/use-people-query";
+import {
+  CardContent,
+  ListGridItem,
+  ListHeader,
+} from "./UserCollectionList.styled";
+
+const PAGE_SIZE = 27;
+
+export const UserCollectionList = () => {
+  const { query, handleNextPage, handlePreviousPage } =
+    usePeopleQuery(PAGE_SIZE);
+
+  const {
+    data: users = [],
+    isLoading,
+    metadata,
+  } = useUserListQuery({
+    query: {
+      limit: query.pageSize,
+      offset: query.pageSize * query.page,
+    },
+  });
+
+  return (
+    <Flex direction="column" p="1.5rem" h="100%">
+      <ListHeader>
+        <BrowserCrumbs
+          crumbs={[
+            {
+              title: ROOT_COLLECTION.name,
+              to: Urls.collection({ id: "root", name: "" }),
+            },
+            { title: PERSONAL_COLLECTIONS.name },
+          ]}
+          analyticsContext="user-collections"
+        />
+      </ListHeader>
+      <Box style={{ flexGrow: 1, overflowY: "auto" }} pr="0.5rem">
+        {isLoading ? (
+          <Flex justify="center" align="center" h="100%">
+            <Loader size="lg" />
+          </Flex>
+        ) : (
+          <Grid>
+            {users.map(
+              user =>
+                user.personal_collection_id && (
+                  <ListGridItem
+                    key={user.personal_collection_id}
+                    role="list-item"
+                  >
+                    <Link to={`/collection/${user.personal_collection_id}`}>
+                      <Card className="p2" hoverable>
+                        <CardContent>
+                          <Icon
+                            name="person"
+                            className="mr1"
+                            color={color("text-medium")}
+                            size={18}
+                          />
+                          <h3>{user.common_name}</h3>
+                        </CardContent>
+                      </Card>
+                    </Link>
+                  </ListGridItem>
+                ),
+            )}
+          </Grid>
+        )}
+      </Box>
+      <Flex justify="end">
+        <PaginationControls
+          page={query.page}
+          pageSize={PAGE_SIZE}
+          total={metadata?.total}
+          itemsLength={PAGE_SIZE}
+          onNextPage={handleNextPage}
+          onPreviousPage={handlePreviousPage}
+        />
+      </Flex>
+    </Flex>
+  );
+};

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

+import fetchMock from "fetch-mock";
+import userEvent from "@testing-library/user-event";
+import { renderWithProviders, screen } from "__support__/ui";
+import { createMockUser } from "metabase-types/api/mocks";
+import { UserCollectionList } from "./UserCollectionList";
+
+const MockUsers = new Array(100).fill(0).map((_, index) =>
+  createMockUser({
+    id: index,
+    common_name: `big boi ${index}`,
+    personal_collection_id: index + 2,
+  }),
+);
+
+const setup = () => {
+  fetchMock.get("path:/api/user", (_: any, __: any, request: Request) => {
+    const params = new URL(request.url).searchParams;
+    const limit = parseInt(params.get("limit") ?? "0");
+    const offset = parseInt(params.get("offset") ?? "0");
+    return MockUsers.slice(offset, offset + limit);
+  });
+  renderWithProviders(<UserCollectionList />);
+};
+
+describe("UserCollectionList", () => {
+  it("should show pages of users", async () => {
+    setup();
+
+    expect(await screen.findByText("big boi 0")).toBeInTheDocument();
+    expect(await screen.findAllByRole("list-item")).toHaveLength(27);
+
+    expect(await screen.findByText("1 - 27")).toBeInTheDocument();
+
+    expect(await screen.findByTestId("previous-page-btn")).toBeDisabled();
+    userEvent.click(await screen.findByTestId("next-page-btn"));
+
+    expect(await screen.findByText("28 - 54")).toBeInTheDocument();
+
+    expect(await screen.findByText("big boi 29")).toBeInTheDocument();
+    expect(await screen.findByTestId("previous-page-btn")).toBeEnabled();
+  });
+});

frontend/src/metabase/nav/containers/MainNavbar/MainNavbarContainer/MainNavbarView.tsx

@@ -127,6 +127,7 @@ function MainNavbarView({
             onSelect={onItemSelect}
             TreeNode={SidebarCollectionLink}
             role="tree"
+            aria-label="collection-tree"
           />
         </SidebarSection>
 

frontend/src/metabase/routes.jsx

@@ -33,7 +33,7 @@ import QueryBuilder from "metabase/query_builder/containers/QueryBuilder";
 import MoveCollectionModal from "metabase/collections/containers/MoveCollectionModal";
 import ArchiveCollectionModal from "metabase/components/ArchiveCollectionModal";
 import CollectionPermissionsModal from "metabase/admin/permissions/components/CollectionPermissionsModal/CollectionPermissionsModal";
-import UserCollectionList from "metabase/containers/UserCollectionList";
+import { UserCollectionList } from "metabase/containers/UserCollectionList";
 
 import PulseEditApp from "metabase/pulse/containers/PulseEditApp";
 import { Setup } from "metabase/setup/components/Setup";