sts-sponsors team mailing list archive

[Peter Makowski <mp+440225@xxxxxxxxxxxxxxxxxx>]

Peter Makowski has proposed merging ~petermakowski/maas-site-manager:setting-navigation-group into maas-site-manager:main.

Commit message:
add enrolment navigation group
- rename "Overview" to "Regions"

Requested reviews:
  MAAS Committers (maas-committers)

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

- add enrollment navigation group to enable navigation to all enrollment pages via the UI.
- rename "Overview" to "Regions" to match latest designs
- fix typo in "enrollment"


This is temporary until we get the full side navigation secondary panel implemented (the design still needs final approval).

QA steps required
Using the main side navigation:
- Click on "Tokens" 
- Make sure the correct page is displayed
- Click on "Requests"
- Make sure the correct page is displayed
-- 
Your team MAAS Committers is requested to review the proposed merge of ~petermakowski/maas-site-manager:setting-navigation-group into maas-site-manager:main.

diff --git a/frontend/src/components/Navigation/Navigation.test.tsx b/frontend/src/components/Navigation/Navigation.test.tsx
index ab2baed..e0609fc 100644
--- a/frontend/src/components/Navigation/Navigation.test.tsx
+++ b/frontend/src/components/Navigation/Navigation.test.tsx
@@ -1,6 +1,6 @@
 import { MemoryRouter } from "react-router-dom";
 
-import Navigation from "./Navigation";
+import Navigation, { navItems, navBottomItems } from "./Navigation";
 
 import { render, screen, userEvent } from "@/test-utils";
 
@@ -14,31 +14,22 @@ describe("Navigation", () => {
     expect(screen.getByRole("navigation")).toBeInTheDocument();
   });
 
-  it("can highlight an active URL", () => {
-    render(
-      <MemoryRouter initialEntries={[{ pathname: "/sites", key: "testKey" }]}>
-        <Navigation />
-      </MemoryRouter>,
-    );
-
-    const currentMenuItem = screen.getAllByRole("link", { current: "page" })[0];
-    expect(currentMenuItem).toBeInTheDocument();
-    expect(currentMenuItem).toHaveTextContent("Overview");
-  });
-
-  it("highlights 'Overview' when active", () => {
-    render(
-      <MemoryRouter initialEntries={[{ pathname: "/sites", key: "testKey" }]}>
-        <Navigation />
-      </MemoryRouter>,
-    );
-
-    const currentMenuItem = screen.getAllByRole("link", { current: "page" })[0];
-    expect(currentMenuItem).toBeInTheDocument();
-    expect(currentMenuItem).toHaveTextContent("Overview");
+  [...navItems, ...navBottomItems].forEach(({ label, url }) => {
+    it(`highlights ${label} navigation item when active`, () => {
+      render(
+        <MemoryRouter initialEntries={[{ pathname: url, key: "testKey" }]}>
+          <Navigation />
+        </MemoryRouter>,
+      );
+
+      const currentMenuItem = screen.getByRole("link", { current: "page", name: label });
+      expect(currentMenuItem).toBeInTheDocument();
+    });
   });
 
-  it("highlights 'Settings' when active", () => {
+  // TODO: enable once side navigation secondary panel is implemented
+  // https://warthogs.atlassian.net/browse/MAASENG-1508
+  it.skip("highlights 'Settings' when active", () => {
     const { rerender } = render(
       <MemoryRouter initialEntries={[{ pathname: "/tokens", key: "testKey" }]}>
         <Navigation />
diff --git a/frontend/src/components/Navigation/Navigation.tsx b/frontend/src/components/Navigation/Navigation.tsx
index 1aa767f..4d1fea9 100644
--- a/frontend/src/components/Navigation/Navigation.tsx
+++ b/frontend/src/components/Navigation/Navigation.tsx
@@ -6,21 +6,26 @@ import useLocalStorageState from "use-local-storage-state";
 import NavigationBanner from "./NavigationBanner";
 import NavigationCollapseToggle from "./NavigationCollapseToggle";
 import NavigationItems from "./NavigationItems";
-import type { NavItem } from "./types";
+import type { NavLink } from "./types";
 
-const navItems: NavItem[] = [
+export const navItems: NavLink[] = [
   {
-    label: "Overview",
+    label: "Regions",
     url: "/sites",
     icon: "maas",
   },
 ];
 
+export const navBottomItems: NavLink[] = [
+  { label: "Tokens", url: "/tokens" },
+  { label: "Requests", url: "/requests" },
+];
+
 const navItemsBottom = [
   {
-    label: "Settings",
-    url: "/tokens",
-    icon: "settings",
+    groupTitle: "Enrolment",
+    groupIcon: "settings",
+    navLinks: navBottomItems,
     highlight: ["/tokens", "/users", "/requests"],
   },
 ];
diff --git a/frontend/src/components/TokensCreate/TokensCreate.test.tsx b/frontend/src/components/TokensCreate/TokensCreate.test.tsx
index 7edb4c5..022f7b6 100644
--- a/frontend/src/components/TokensCreate/TokensCreate.test.tsx
+++ b/frontend/src/components/TokensCreate/TokensCreate.test.tsx
@@ -33,7 +33,7 @@ afterAll(() => {
 describe("TokensCreate", () => {
   it("renders the form", async () => {
     renderWithMemoryRouter(<TokensCreate />);
-    expect(screen.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeInTheDocument();
+    expect(screen.getByRole("form", { name: /Generate new enrolment tokens/i })).toBeInTheDocument();
   });
 
   it("if not all required fields have been entered the submit button is disabled", async () => {
diff --git a/frontend/src/components/TokensCreate/TokensCreate.tsx b/frontend/src/components/TokensCreate/TokensCreate.tsx
index ea6a9ab..b93a741 100644
--- a/frontend/src/components/TokensCreate/TokensCreate.tsx
+++ b/frontend/src/components/TokensCreate/TokensCreate.tsx
@@ -59,7 +59,7 @@ const TokensCreate = () => {
   return (
     <div className="tokens-create">
       <h3 className="tokens-create__heading p-heading--4" id={headingId}>
-        Generate new enrollment tokens
+        Generate new enrolment tokens
       </h3>
       {tokensMutation.isError && (
         <Notification severity="negative">There was an error generating the token(s).</Notification>
diff --git a/frontend/tests/tokens.spec.ts b/frontend/tests/tokens.spec.ts
index 778602c..9d4a1c7 100644
--- a/frontend/tests/tokens.spec.ts
+++ b/frontend/tests/tokens.spec.ts
@@ -6,14 +6,14 @@ test.beforeEach(async ({ page }) => {
 
 test("can open and close token generate form", async ({ page }) => {
   await page.getByRole("button", { name: /Generate tokens/i }).click();
-  expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeVisible();
+  expect(page.getByRole("form", { name: /Generate new enrolment tokens/i })).toBeVisible();
   await page.getByRole("button", { name: /Cancel/i }).click();
-  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeHidden();
+  await expect(page.getByRole("form", { name: /Generate new enrolment tokens/i })).toBeHidden();
 });
 
 test("token create form is closed when navigating away", async ({ page }) => {
   await page.getByRole("button", { name: /Generate tokens/i }).click();
-  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeVisible();
+  await expect(page.getByRole("form", { name: /Generate new enrolment tokens/i })).toBeVisible();
 
   const mobileBanner = await page.getByRole("banner", { name: /navigation/i }).isVisible();
   if (mobileBanner) {
@@ -26,11 +26,11 @@ test("token create form is closed when navigating away", async ({ page }) => {
   }
   await page
     .getByRole("navigation", { name: /main/i })
-    .getByRole("link", { name: /Overview/ })
+    .getByRole("link", { name: /Regions/ })
     .click();
 
   await page.goBack();
-  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeHidden();
+  await expect(page.getByRole("form", { name: /Generate new enrolment tokens/i })).toBeHidden();
 });
 
 test("closes and clears the form after creating the token", async ({ page }) => {
@@ -38,16 +38,16 @@ test("closes and clears the form after creating the token", async ({ page }) => 
   await page.getByRole("textbox", { name: /Amount of tokens to generate/i }).type("1");
   await page.getByRole("textbox", { name: /Expiration time/i }).type("1 week");
   await page
-    .getByRole("form", { name: /Generate new enrollment tokens/i })
+    .getByRole("form", { name: /Generate new enrolment tokens/i })
     .getByRole("button", { name: /Generate tokens/i })
     .click();
   await page.waitForResponse((resp) => resp.url().includes("/tokens") && resp.status() === 200);
-  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeHidden();
+  await expect(page.getByRole("form", { name: /Generate new enrolment tokens/i })).toBeHidden();
   // check that the form has been reset
   await page.getByRole("button", { name: /Generate tokens/i }).click();
   await expect(
     page
-      .getByRole("form", { name: /Generate new enrollment tokens/i })
+      .getByRole("form", { name: /Generate new enrolment tokens/i })
       .getByRole("button", { name: /Generate tokens/i }),
   ).toBeDisabled();
 });