← Back to team overview

sts-sponsors team mailing list archive

[Merge] ~petermakowski/maas-site-manager:update-api-shapes into maas-site-manager:main

 

Peter Makowski has proposed merging ~petermakowski/maas-site-manager:update-api-shapes into maas-site-manager:main.

Commit message:
update API data shapes

Requested reviews:
  MAAS Committers (maas-committers)

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

- update API data shapes
- update pagination to start at index 1

QA Steps
Go to sites, tokens, and requests pages and make sure they load correctly using the mock API
-- 
Your team MAAS Committers is requested to review the proposed merge of ~petermakowski/maas-site-manager:update-api-shapes into maas-site-manager:main.
diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts
index 8f0ae7a..c124177 100644
--- a/frontend/src/api/types.ts
+++ b/frontend/src/api/types.ts
@@ -18,12 +18,10 @@ export type Site = {
   url: string; // <full URL including protocol>,
   connection: Stats["connection"];
   last_seen: string; // <ISO 8601 date>,
-  address: {
-    countrycode: string; // <alpha2 country code>,
-    city: string;
-    zip: string;
-    street: string;
-  };
+  country: string; // <alpha2 country code>,
+  city: string;
+  zip: string;
+  street: string;
   timezone: string; // IANA time zone name,
   stats: Stats;
 };
@@ -40,7 +38,7 @@ export type SitesQueryResult = PaginatedQueryResult<Site>;
 export type Token = {
   id: string;
   site_id: Site["id"] | null;
-  token: string;
+  value: string;
   expires: string; //<ISO 8601 date string>,
   created: string; //<ISO 8601 date string>
 };
diff --git a/frontend/src/components/RequestsList/RequestsList.tsx b/frontend/src/components/RequestsList/RequestsList.tsx
index 313bf06..3f5be3c 100644
--- a/frontend/src/components/RequestsList/RequestsList.tsx
+++ b/frontend/src/components/RequestsList/RequestsList.tsx
@@ -30,7 +30,7 @@ const Requests: React.FC = () => {
       <Row>
         <Col size={12}>
           <PaginationBar
-            currentPage={page + 1}
+            currentPage={page}
             dataContext="open enrolment requests"
             handlePageSizeChange={handlePageSizeChange}
             isLoading={isLoading}
diff --git a/frontend/src/components/SitesList/SitesList.tsx b/frontend/src/components/SitesList/SitesList.tsx
index ef0a567..39a8c26 100644
--- a/frontend/src/components/SitesList/SitesList.tsx
+++ b/frontend/src/components/SitesList/SitesList.tsx
@@ -11,7 +11,7 @@ import { parseSearchTextToQueryParams } from "@/utils";
 const DEFAULT_PAGE_SIZE = 50;
 
 const SitesList = () => {
-  const [page, setPage] = useState(0);
+  const [page, setPage] = useState(1);
   const [size] = useState(DEFAULT_PAGE_SIZE);
   const [searchText, setSearchText] = useState("");
   const debounceSearchText = useDebounce(searchText);
@@ -22,7 +22,7 @@ const SitesList = () => {
   );
 
   useEffect(() => {
-    setPage(0);
+    setPage(1);
   }, [searchText]);
 
   return (
@@ -34,11 +34,11 @@ const SitesList = () => {
         setSearchText={setSearchText}
       />
       <Pagination
-        currentPage={page + 1}
+        currentPage={page}
         disabled={isLoading}
         itemsPerPage={size}
         paginate={(page) => {
-          setPage(page - 1);
+          setPage(page);
         }}
         totalItems={data?.total || 0}
       />
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
index c0cd2e3..70e7291 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
@@ -94,7 +94,7 @@ it("displays correct local time", () => {
 });
 
 it("displays full name of the country", () => {
-  const item = siteFactory.build({ address: { countrycode: "GB" } });
+  const item = siteFactory.build({ country: "GB" });
   renderWithMemoryRouter(
     <SitesTable
       data={sitesQueryResultFactory.build({ items: [item], total: 1, page: 1, size: 1 })}
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
index b3578fb..4d214ed 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
@@ -88,7 +88,7 @@ const SitesTable = ({
       },
       {
         id: "connection",
-        accessorFn: createAccessor(["connection", "last_seen"]),
+        accessorFn: createAccessor("stats"),
         header: () => (
           <>
             <div className="connection__text">connection</div>
@@ -96,13 +96,13 @@ const SitesTable = ({
           </>
         ),
         cell: ({ getValue }) => {
-          const { connection, last_seen } = getValue();
-          return connection ? <ConnectionInfo connection={connection} lastSeen={last_seen} /> : null;
+          const { stats } = getValue();
+          return stats ? <ConnectionInfo connection={stats.connection} lastSeen={stats.last_seen} /> : null;
         },
       },
       {
         id: "address",
-        accessorFn: createAccessor("address"),
+        accessorFn: createAccessor(["country", "city", "zip", "street"]),
         header: () => (
           <>
             <div>country</div>
@@ -110,11 +110,10 @@ const SitesTable = ({
           </>
         ),
         cell: ({ getValue }) => {
-          const { address } = getValue();
-          const { countrycode, city, zip, street } = address || {};
+          const { country, city, zip, street } = getValue();
           return (
             <>
-              <div>{countrycode ? getCountryName(countrycode) : ""}</div>
+              <div>{country ? getCountryName(country) : ""}</div>
               <div className="u-text--muted">
                 {street}, {city}, {zip}
               </div>
diff --git a/frontend/src/components/TokensList/TokensList.test.tsx b/frontend/src/components/TokensList/TokensList.test.tsx
index 631096a..8738cd4 100644
--- a/frontend/src/components/TokensList/TokensList.test.tsx
+++ b/frontend/src/components/TokensList/TokensList.test.tsx
@@ -39,7 +39,7 @@ it("should display table with tokens", async () => {
   expect(within(tableBody).getAllByRole("row")).toHaveLength(tokens.length);
   within(tableBody)
     .getAllByRole("row")
-    .forEach((row, idx) => expect(row).toHaveTextContent(new RegExp(tokens[idx].token, "i")));
+    .forEach((row, idx) => expect(row).toHaveTextContent(new RegExp(tokens[idx].value, "i")));
 });
 
 it("should display a token count description (default=50)", () => {
diff --git a/frontend/src/components/TokensList/TokensList.tsx b/frontend/src/components/TokensList/TokensList.tsx
index d2e6da9..aac942a 100644
--- a/frontend/src/components/TokensList/TokensList.tsx
+++ b/frontend/src/components/TokensList/TokensList.tsx
@@ -21,7 +21,10 @@ const TokensList = () => {
   const { page, debouncedPage, size, handleNextClick, handlePreviousClick, handlePageSizeChange, setPage } =
     usePagination(DEFAULT_PAGE_SIZE, totalDataCount);
 
-  const { data, isLoading, isFetchedAfterMount } = useTokensQuery({ page: `${debouncedPage}`, size: `${size}` });
+  const { data, isLoading, isFetchedAfterMount } = useTokensQuery({
+    page: `${debouncedPage}`,
+    size: `${size}`,
+  });
 
   useEffect(() => {
     if (data && "total" in data) {
@@ -90,7 +93,7 @@ const TokensList = () => {
           </Col>
         </Row>
         <PaginationBar
-          currentPage={page + 1}
+          currentPage={page}
           dataContext="tokens"
           handlePageSizeChange={handlePageSizeChange}
           isLoading={isLoading}
diff --git a/frontend/src/components/TokensList/components/TokensTable/TokensTable.test.tsx b/frontend/src/components/TokensList/components/TokensTable/TokensTable.test.tsx
index 5a12909..26b7b61 100644
--- a/frontend/src/components/TokensList/components/TokensTable/TokensTable.test.tsx
+++ b/frontend/src/components/TokensList/components/TokensTable/TokensTable.test.tsx
@@ -18,7 +18,7 @@ it("displays rows for each token", () => {
   expect(within(tableBody).getAllByRole("row")).toHaveLength(items.length);
   within(tableBody)
     .getAllByRole("row")
-    .forEach((row, idx) => expect(row).toHaveTextContent(new RegExp(items[idx].token, "i")));
+    .forEach((row, idx) => expect(row).toHaveTextContent(new RegExp(items[idx].value, "i")));
 });
 
 it("displays a copy button in each row", () => {
diff --git a/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx b/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
index a4c2b15..458fb01 100644
--- a/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
+++ b/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
@@ -60,13 +60,13 @@ const TokensTable = ({
       {
         id: "token",
         header: () => <div>Token</div>,
-        accessorFn: createAccessor("token"),
+        accessorFn: createAccessor("value"),
         cell: ({ getValue }) => {
-          const { token } = getValue();
+          const { value } = getValue();
           return (
-            <div className="token-cell" onClick={() => handleTokenCopy(token!)}>
-              <span className="token-text">{token}</span>
-              <CopyButton isCopied={isTokenCopied(token!)} value={token || ""} />
+            <div className="token-cell" onClick={() => handleTokenCopy(value!)}>
+              <span className="token-text">{value}</span>
+              <CopyButton isCopied={isTokenCopied(value!)} value={value || ""} />
             </div>
           );
         },
diff --git a/frontend/src/hooks/api.test.ts b/frontend/src/hooks/api.test.ts
index 4968ff4..9186e36 100644
--- a/frontend/src/hooks/api.test.ts
+++ b/frontend/src/hooks/api.test.ts
@@ -27,7 +27,7 @@ afterAll(() => {
 });
 
 it("should return sites", async () => {
-  const { result } = renderHook(() => useSitesQuery({ page: "0", size: "2" }), { wrapper: Providers });
+  const { result } = renderHook(() => useSitesQuery({ page: "1", size: "2" }), { wrapper: Providers });
 
   await waitFor(() => expect(result.current.isFetchedAfterMount).toBe(true));
 
@@ -35,7 +35,7 @@ it("should return sites", async () => {
 });
 
 it("should return tokens", async () => {
-  const { result } = renderHook(() => useTokensQuery({ page: "0", size: "2" }), { wrapper: Providers });
+  const { result } = renderHook(() => useTokensQuery({ page: "1", size: "2" }), { wrapper: Providers });
 
   await waitFor(() => expect(result.current.isFetchedAfterMount).toBe(true));
 
diff --git a/frontend/src/hooks/api.ts b/frontend/src/hooks/api.ts
index f050ab7..1f9c732 100644
--- a/frontend/src/hooks/api.ts
+++ b/frontend/src/hooks/api.ts
@@ -52,8 +52,8 @@ export const useRequestsQuery = ({ page, size }: GetEnrollmentRequestsQueryParam
 
 export const useRequestsCountQuery = () =>
   useQuery<EnrollmentRequestsQueryResult>({
-    queryKey: ["requests", "0", "1"],
-    queryFn: () => getEnrollmentRequests({ page: "0", size: "1" }),
+    queryKey: ["requests", "1", "1"],
+    queryFn: () => getEnrollmentRequests({ page: "1", size: "1" }),
     keepPreviousData: true,
     refetchInterval: defaultRefetchInterval,
   });
diff --git a/frontend/src/hooks/usePagination.test.ts b/frontend/src/hooks/usePagination.test.ts
index 2c4f6b8..a999c40 100644
--- a/frontend/src/hooks/usePagination.test.ts
+++ b/frontend/src/hooks/usePagination.test.ts
@@ -6,10 +6,10 @@ describe("usePagination", () => {
   const samplePageSize = 50;
   const sampleTotalCount = 200;
 
-  it("initializes the page to 0 when called", () => {
+  it("initializes the page to 1 when called", () => {
     const { result } = renderHook(() => usePagination(samplePageSize, sampleTotalCount));
 
-    expect(result.current.page).toBe(0);
+    expect(result.current.page).toBe(1);
   });
 
   it("next and previous page functions work correctly", () => {
@@ -19,13 +19,13 @@ describe("usePagination", () => {
       result.current.handleNextClick();
     });
 
-    expect(result.current.page).toBe(1);
+    expect(result.current.page).toBe(2);
 
     hookAct(() => {
       result.current.handlePreviousClick();
     });
 
-    expect(result.current.page).toBe(0);
+    expect(result.current.page).toBe(1);
   });
 
   it("should reset page count after page size is changed", () => {
@@ -36,6 +36,6 @@ describe("usePagination", () => {
       result.current.handlePageSizeChange(10);
     });
 
-    expect(result.current.page).toBe(0);
+    expect(result.current.page).toBe(1);
   });
 });
diff --git a/frontend/src/hooks/usePagination.ts b/frontend/src/hooks/usePagination.ts
index 5608eb7..7a0f66f 100644
--- a/frontend/src/hooks/usePagination.ts
+++ b/frontend/src/hooks/usePagination.ts
@@ -3,7 +3,7 @@ import { useState } from "react";
 import useDebouncedValue from "./useDebouncedValue";
 
 function usePagination(pageSize: number, totalItems: number) {
-  const [page, setPage] = useState(0);
+  const [page, setPage] = useState(1);
   const [size, setSize] = useState(pageSize);
   const debouncedPage = useDebouncedValue(page);
 
@@ -13,10 +13,10 @@ function usePagination(pageSize: number, totalItems: number) {
   };
 
   const handlePreviousClick = () => {
-    setPage((prev) => (prev === 0 ? 0 : prev - 1));
+    setPage((prev) => (prev === 1 ? 1 : prev - 1));
   };
 
-  const resetPageCount = () => setPage(0);
+  const resetPageCount = () => setPage(1);
 
   const handlePageSizeChange = (size: number) => {
     setSize(size);
diff --git a/frontend/src/mocks/factories.ts b/frontend/src/mocks/factories.ts
index 3b27de4..7121d20 100644
--- a/frontend/src/mocks/factories.ts
+++ b/frontend/src/mocks/factories.ts
@@ -40,12 +40,10 @@ export const siteFactory = Factory.define<Site>(({ sequence }) => {
     url: `http://${name}.${chance.tld()}`,
     connection: connectionFactory.build(),
     last_seen: new Date(chance.date({ year: 2023 })).toISOString(),
-    address: {
-      countrycode: chance.country(), // <alpha2 country code>,
-      city: chance.city(),
-      zip: chance.zip(),
-      street: chance.address(),
-    },
+    country: chance.country(), // <alpha2 country code>,
+    city: chance.city(),
+    zip: chance.zip(),
+    street: chance.address(),
     timezone: chance.timezone().utc[0],
     stats: statsFactory.build(),
   };
@@ -64,7 +62,7 @@ export const tokenFactory = Factory.define<Token>(({ sequence }) => {
   return {
     id: `${sequence}`,
     site_id: `${chance.integer({ min: 0, max: 100 })}`,
-    token: chance.hash({ length: 64 }),
+    value: chance.hash({ length: 64 }),
     expires: new Date(chance.date({ year: 2024 })).toISOString(), //<ISO 8601 date string>,
     created: new Date(chance.date({ year: 2023 })).toISOString(), //<ISO 8601 date string>
   };
diff --git a/frontend/src/mocks/resolvers.ts b/frontend/src/mocks/resolvers.ts
index 385d828..dc8e6d9 100644
--- a/frontend/src/mocks/resolvers.ts
+++ b/frontend/src/mocks/resolvers.ts
@@ -36,7 +36,7 @@ export const createMockSitesResolver =
     const searchParams = new URLSearchParams(req.url.search);
     const page = Number(searchParams.get("page"));
     const size = Number(searchParams.get("size"));
-    const itemsPage = sites.slice(page * Number(size), (page + 1) * size);
+    const itemsPage = sites.slice((page - 1) * size, page * size);
     const response = {
       items: itemsPage,
       page,
@@ -68,7 +68,7 @@ export const createMockGetTokensResolver =
     const searchParams = new URLSearchParams(req.url.search);
     const page = Number(searchParams.get("page"));
     const size = Number(searchParams.get("size"));
-    const itemsPage = tokens.slice(page * Number(size), (page + 1) * size);
+    const itemsPage = tokens.slice((page - 1) * size, page * size);
 
     const response = {
       items: itemsPage,
@@ -85,7 +85,7 @@ export const createMockGetEnrollmentRequestsResolver =
     const searchParams = new URLSearchParams(req.url.search);
     const page = Number(searchParams.get("page"));
     const size = Number(searchParams.get("size"));
-    const itemsPage = enrollmentRequests.slice(page * Number(size), (page + 1) * size);
+    const itemsPage = enrollmentRequests.slice((page - 1) * size, page * size);
 
     const response = {
       items: itemsPage,
diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts
index 002cb19..e5f5a0c 100644
--- a/frontend/src/utils.ts
+++ b/frontend/src/utils.ts
@@ -28,10 +28,10 @@ export const parseSearchTextToQueryParams = (text: string) => {
   return parsedText;
 };
 
-export const customParamSerializer = (params: Record<string, string>, queryText?: string) => {
+export const customParamSerializer = (params: Record<string, string | number>, queryText?: string) => {
   return (
     Object.entries(Object.assign({}, params))
-      .map(([key, value]) => `${key}=${value}`)
+      .map(([key, value]) => `${key}=${encodeURIComponent(value)}`)
       .join("&") + `${queryText ? "&" + queryText : ""}`
   );
 };

Follow ups