sts-sponsors team mailing list archive

[Jones Ogolo <mp+442089@xxxxxxxxxxxxxxxxxx>]

Jones Ogolo has proposed merging ~jonesogolo/maas-site-manager:1593-update-sites-page-pagination into maas-site-manager:main.

Commit message:
Add pagination bar to sites table, replacing the previous pagination component.

Requested reviews:
  MAAS Committers (maas-committers)

For more details, see:
https://code.launchpad.net/~jonesogolo/maas-site-manager/+git/maas-site-manager/+merge/442089

QA steps:
- Goto '/sites'
- Ensure that you see a pagination bar tjat shows the sites count. e.g: Showing xx out of xxx MAAS Regions
- Test the pagination buttons, input and items per page selector to ensure it all works.
-- 
Your team MAAS Committers is requested to review the proposed merge of ~jonesogolo/maas-site-manager:1593-update-sites-page-pagination into maas-site-manager:main.

diff --git a/frontend/src/components/SitesList/SitesList.tsx b/frontend/src/components/SitesList/SitesList.tsx
index 39a8c26..9a6c366 100644
--- a/frontend/src/components/SitesList/SitesList.tsx
+++ b/frontend/src/components/SitesList/SitesList.tsx
@@ -1,29 +1,35 @@
 import { useEffect, useState } from "react";
 
-import { Pagination } from "@canonical/react-components";
-
 import SitesTable from "./SitesTable";
 
 import { useSitesQuery } from "@/hooks/api";
 import useDebounce from "@/hooks/useDebouncedValue";
+import usePagination from "@/hooks/usePagination";
 import { parseSearchTextToQueryParams } from "@/utils";
 
 const DEFAULT_PAGE_SIZE = 50;
 
 const SitesList = () => {
-  const [page, setPage] = useState(1);
-  const [size] = useState(DEFAULT_PAGE_SIZE);
+  const [totalDataCount, setTotalDataCount] = useState(0);
+  const { page, debouncedPage, size, handleNextClick, handlePreviousClick, handlePageSizeChange, setPage } =
+    usePagination(DEFAULT_PAGE_SIZE, totalDataCount);
   const [searchText, setSearchText] = useState("");
   const debounceSearchText = useDebounce(searchText);
 
   const { data, isLoading, isFetchedAfterMount } = useSitesQuery(
-    { page: `${page}`, size: `${size}` },
+    { page: `${debouncedPage}`, size: `${size}` },
     parseSearchTextToQueryParams(debounceSearchText),
   );
 
   useEffect(() => {
     setPage(1);
-  }, [searchText]);
+  }, [searchText, setPage]);
+
+  useEffect(() => {
+    if (data && "total" in data) {
+      setTotalDataCount(data.total);
+    }
+  }, [data]);
 
   return (
     <div>
@@ -31,16 +37,18 @@ const SitesList = () => {
         data={data}
         isFetchedAfterMount={isFetchedAfterMount}
         isLoading={isLoading}
-        setSearchText={setSearchText}
-      />
-      <Pagination
-        currentPage={page}
-        disabled={isLoading}
-        itemsPerPage={size}
-        paginate={(page) => {
-          setPage(page);
+        paginationProps={{
+          currentPage: page,
+          dataContext: "MAAS Regions",
+          handlePageSizeChange,
+          isLoading,
+          itemsPerPage: size,
+          onNextClick: handleNextClick,
+          onPreviousClick: handlePreviousClick,
+          setCurrentPage: setPage,
+          totalItems: data?.total || 0,
         }}
-        totalItems={data?.total || 0}
+        setSearchText={setSearchText}
       />
     </div>
   );
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
index 9b753ef..89f78d3 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
@@ -32,6 +32,17 @@ it("displays an empty sites table", () => {
       data={sitesQueryResultFactory.build()}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 1,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 0,
+      }}
       setSearchText={() => {}}
     />,
   );
@@ -46,6 +57,17 @@ it("displays rows with details for each site", () => {
       data={sitesQueryResultFactory.build({ items, total: 1, page: 1, size: 1 })}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 1,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 1,
+      }}
       setSearchText={() => {}}
     />,
   );
@@ -67,6 +89,17 @@ it("displays correctly paginated results", () => {
       data={sitesQueryResultFactory.build({ items, total: 100, page: 1, size: pageLength })}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: pageLength,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 100,
+      }}
       setSearchText={() => {}}
     />,
   );
@@ -85,6 +118,17 @@ it("displays correct local time", () => {
       data={sitesQueryResultFactory.build({ items: [item], total: 1, page: 1, size: 1 })}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 1,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 1,
+      }}
       setSearchText={() => {}}
     />,
   );
@@ -100,6 +144,17 @@ it("displays full name of the country", () => {
       data={sitesQueryResultFactory.build({ items: [item], total: 1, page: 1, size: 1 })}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 1,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 1,
+      }}
       setSearchText={() => {}}
     />,
   );
@@ -121,6 +176,17 @@ it("displays correct number of deployed machines", () => {
       data={sitesQueryResultFactory.build({ items: [item], total: 1, page: 1, size: 1 })}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 1,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 1,
+      }}
       setSearchText={() => {}}
     />,
   );
@@ -137,6 +203,17 @@ it("if name is not unique a warning is displayed.", async () => {
       data={sitesQueryResultFactory.build({ items: [itemUnique], total: 1, page: 1, size: 1 })}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 1,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 1,
+      }}
       setSearchText={() => {}}
     />,
   );
@@ -151,9 +228,47 @@ it("if name is not unique a warning is displayed.", async () => {
       data={sitesQueryResultFactory.build({ items: [itemNonUnique], total: 1, page: 1, size: 1 })}
       isFetchedAfterMount={true}
       isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 1,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 1,
+      }}
       setSearchText={() => {}}
     />,
   );
 
   expect(screen.getByRole("button", { name: /warning - name is not unique/i })).toBeInTheDocument();
 });
+
+it("displays a pagination bar with the table", () => {
+  const items = siteFactory.buildList(2);
+  renderWithMemoryRouter(
+    <SitesTable
+      data={sitesQueryResultFactory.build({ items, total: 2, page: 1, size: 10 })}
+      isFetchedAfterMount={true}
+      isLoading={false}
+      paginationProps={{
+        currentPage: 1,
+        dataContext: "MAAS Regions",
+        handlePageSizeChange: () => {},
+        isLoading: false,
+        itemsPerPage: 10,
+        onNextClick: () => {},
+        onPreviousClick: () => {},
+        setCurrentPage: () => {},
+        totalItems: 2,
+      }}
+      setSearchText={() => {}}
+    />,
+  );
+
+  expect(screen.getByText(/Showing 2 out of 2 MAAS Regions/i)).toBeInTheDocument();
+  expect(screen.getByRole("button", { name: /next page/i })).toBeInTheDocument();
+  expect(screen.getByRole("button", { name: /previous page/i })).toBeInTheDocument();
+});
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
index 2d26837..cc28b1a 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
@@ -14,6 +14,8 @@ import type { SitesQueryResult } from "@/api/types";
 import ExternalLink from "@/components/ExternalLink";
 import NoRegions from "@/components/NoRegions";
 import SelectAllCheckbox from "@/components/SelectAllCheckbox";
+import type { PaginationBarProps } from "@/components/base/PaginationBar/PaginationBar";
+import PaginationBar from "@/components/base/PaginationBar/PaginationBar";
 import TooltipButton from "@/components/base/TooltipButton/TooltipButton";
 import { isDev } from "@/constants";
 import { useAppContext } from "@/context";
@@ -34,8 +36,10 @@ const SitesTable = ({
   isFetchedAfterMount,
   isLoading,
   setSearchText,
+  paginationProps,
 }: Pick<UseSitesQueryResult, "data" | "isLoading" | "isFetchedAfterMount"> & {
   setSearchText: (text: string) => void;
+  paginationProps: PaginationBarProps;
 }) => {
   const [columnVisibility, setColumnVisibility] = useLocalStorageState("sitesTableColumnVisibility", {
     defaultValue: {},
@@ -225,6 +229,7 @@ const SitesTable = ({
         isLoading={isLoading}
         setSearchText={setSearchText}
       />
+      <PaginationBar {...{ ...paginationProps }} />
       <table aria-label="sites" className="sites-table">
         <thead>
           {table.getHeaderGroups().map((headerGroup) => (
diff --git a/frontend/src/components/base/PaginationBar/PaginationBar.tsx b/frontend/src/components/base/PaginationBar/PaginationBar.tsx
index 3c7395a..0c1eefe 100644
--- a/frontend/src/components/base/PaginationBar/PaginationBar.tsx
+++ b/frontend/src/components/base/PaginationBar/PaginationBar.tsx
@@ -6,7 +6,7 @@ import { Select } from "@canonical/react-components";
 import type { AppPaginationProps } from "@/components/base/TablePagination/TablePagination";
 import TablePagination from "@/components/base/TablePagination/TablePagination";
 
-type TokensTableControlProps = AppPaginationProps & {
+export type PaginationBarProps = AppPaginationProps & {
   handlePageSizeChange: (size: number) => void;
   dataContext: string;
   setCurrentPage: (page: number) => void;
@@ -23,7 +23,7 @@ const PaginationBar = ({
   dataContext,
   setCurrentPage,
   isLoading,
-}: TokensTableControlProps) => {
+}: PaginationBarProps) => {
   const pageCounts = useMemo(() => [20, 30, 50], []);
   const pageOptions = useMemo(
     () => pageCounts.map((pageCount) => ({ label: `${pageCount}/page`, value: pageCount })),
diff --git a/frontend/src/components/base/TablePagination/TablePagination.tsx b/frontend/src/components/base/TablePagination/TablePagination.tsx
index 070f187..49c8c27 100644
--- a/frontend/src/components/base/TablePagination/TablePagination.tsx
+++ b/frontend/src/components/base/TablePagination/TablePagination.tsx
@@ -37,6 +37,7 @@ const TablePagination = ({
         setCurrentPage(valueAsNumber);
       }
     } else {
+      setPageNumber(undefined);
       setError("Enter a page number.");
     }
   };
@@ -57,13 +58,14 @@ const TablePagination = ({
   const handlePreviousClick = () => {
     onPreviousClick();
   };
+  const noItems = totalItems === 0;
 
   return (
     <nav aria-label="pagination" className="table-pagination">
       <Button
         appearance="base"
         aria-label="previous page"
-        disabled={currentPage === 1 || isLoading}
+        disabled={currentPage === 1 || isLoading || noItems}
         hasIcon
         onClick={handlePreviousClick}
       >
@@ -85,7 +87,7 @@ const TablePagination = ({
       <Button
         appearance="base"
         aria-label="next page"
-        disabled={currentPage === totalPages || isLoading}
+        disabled={currentPage === totalPages || isLoading || noItems}
         hasIcon
         onClick={handleNextClick}
       >