WD-11588 - refactor: Remove nested components in components/Login (#1762

)

src/components/LogIn/IdentityProviderForm/IdentityProviderForm.test.tsx

@@ -0,0 +1,42 @@
+import { screen, within } from "@testing-library/react";
+
+import { generalStateFactory } from "testing/factories/general";
+import { rootStateFactory } from "testing/factories/root";
+import { renderComponent } from "testing/utils";
+
+import { Label } from "../types";
+
+import IdentityProviderForm from "./IdentityProviderForm";
+
+describe("IdentityProviderForm", () => {
+  it("should render a 'connecting' message if the user is logged in", () => {
+    renderComponent(<IdentityProviderForm userIsLoggedIn={true} />);
+    expect(
+      within(screen.getByRole("button")).getByText("Connecting..."),
+    ).toBeInTheDocument();
+  });
+
+  it("should render a 'connecting' message if the user is not logged in and there is no visitURLs", () => {
+    const state = rootStateFactory.build({
+      general: generalStateFactory.withConfig().build({
+        visitURLs: null,
+      }),
+    });
+    renderComponent(<IdentityProviderForm userIsLoggedIn={false} />, { state });
+    expect(
+      within(screen.getByRole("button")).getByText("Connecting..."),
+    ).toBeInTheDocument();
+  });
+
+  it("should render a login message if the user is not logged in and there is visitURLs", () => {
+    const state = rootStateFactory.build({
+      general: generalStateFactory.withConfig().build({
+        visitURLs: ["I am a url"],
+      }),
+    });
+    renderComponent(<IdentityProviderForm userIsLoggedIn={false} />, { state });
+    expect(
+      screen.getByRole("link", { name: Label.LOGIN_TO_DASHBOARD }),
+    ).toBeInTheDocument();
+  });
+});

src/components/LogIn/IdentityProviderForm/IdentityProviderForm.tsx

@@ -0,0 +1,34 @@
+import { Spinner } from "@canonical/react-components";
+import { useSelector } from "react-redux";
+
+import AuthenticationButton from "components/AuthenticationButton";
+import type { RootState } from "store/store";
+
+import { Label } from "../types";
+
+type Props = {
+  userIsLoggedIn: boolean;
+};
+
+const IdentityProviderForm = ({ userIsLoggedIn }: Props) => {
+  const visitURL = useSelector((state: RootState) => {
+    if (!userIsLoggedIn) {
+      // This form only gets displayed on the main login page, at which point
+      // there can only be one authentication request, so just return the
+      // first one.
+      return state?.general?.visitURLs?.[0];
+    }
+  });
+
+  return visitURL ? (
+    <AuthenticationButton appearance="positive" visitURL={visitURL}>
+      {Label.LOGIN_TO_DASHBOARD}
+    </AuthenticationButton>
+  ) : (
+    <button className="p-button--neutral" disabled>
+      <Spinner text="Connecting..." />
+    </button>
+  );
+};
+
+export default IdentityProviderForm;

src/components/LogIn/IdentityProviderForm/index.ts

@@ -0,0 +1 @@
+export { default } from "./IdentityProviderForm";

src/components/LogIn/LogIn.test.tsx

@@ -2,9 +2,6 @@ import { screen, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { vi } from "vitest";
 
-import { thunks as appThunks } from "store/app";
-import { actions as generalActions } from "store/general";
-import * as dashboardStore from "store/store";
 import { configFactory, generalStateFactory } from "testing/factories/general";
 import { rootStateFactory } from "testing/factories/root";
 import { renderComponent } from "testing/utils";
@@ -13,34 +10,10 @@ import LogIn from "./LogIn";
 import { ErrorResponse, Label } from "./types";
 
 describe("LogIn", () => {
-  const consoleError = console.error;
-
-  beforeEach(() => {
-    console.error = vi.fn();
-  });
-
   afterEach(() => {
-    console.error = consoleError;
     vi.restoreAllMocks();
   });
 
-  it("renders a 'connecting' message while connecting", () => {
-    const state = rootStateFactory.build({
-      general: generalStateFactory.withConfig().build({
-        config: configFactory.build({
-          identityProviderAvailable: true,
-        }),
-      }),
-    });
-    renderComponent(<LogIn>App content</LogIn>, { state });
-    expect(
-      within(screen.getByRole("button")).getByText("Connecting..."),
-    ).toBeInTheDocument();
-    const content = screen.getByText("App content");
-    expect(content).toBeInTheDocument();
-    expect(content).toHaveClass("app-content");
-  });
-
   it("does not display the login form if the user is logged in", () => {
     const state = rootStateFactory.build({
       general: generalStateFactory.withConfig().build({
@@ -58,6 +31,9 @@ describe("LogIn", () => {
     });
     renderComponent(<LogIn>App content</LogIn>, { state });
     expect(document.querySelector(".login")).not.toBeInTheDocument();
+    const content = screen.getByText("App content");
+    expect(content).toBeInTheDocument();
+    expect(content).toHaveClass("app-content");
   });
 
   it("renders an IdentityProvider login UI if the user is not logged in", () => {
@@ -71,11 +47,11 @@ describe("LogIn", () => {
     });
     renderComponent(<LogIn>App content</LogIn>, { state });
     expect(
-      screen.getByRole("link", { name: "Log in to the dashboard" }),
+      screen.getByRole("link", { name: Label.LOGIN_TO_DASHBOARD }),
     ).toBeInTheDocument();
-    const content = screen.getByText("App content");
-    expect(content).toBeInTheDocument();
-    expect(content).toHaveClass("app-content");
+    expect(
+      screen.queryByRole("textbox", { name: "Username" }),
+    ).not.toBeInTheDocument();
   });
 
   it("renders a UserPass login UI if the user is not logged in", () => {
@@ -88,11 +64,11 @@ describe("LogIn", () => {
     });
     renderComponent(<LogIn>App content</LogIn>, { state });
     expect(screen.getByRole("button")).toHaveTextContent(
-      "Log in to the dashboard",
+      Label.LOGIN_TO_DASHBOARD,
     );
-    const content = screen.getByText("App content");
-    expect(content).toBeInTheDocument();
-    expect(content).toHaveClass("app-content");
+    expect(
+      screen.getByRole("textbox", { name: "Username" }),
+    ).toBeInTheDocument();
   });
 
   it("renders a login error if one exists", () => {
@@ -142,50 +118,6 @@ describe("LogIn", () => {
     expect(screen.getByText(Label.INVALID_FIELD)).toBeInTheDocument();
   });
 
-  it("logs in", async () => {
-    // Mock the result of the thunk to be a normal action so that it can be tested
-    // for. This is necessary because we don't have a full store set up which
-    // can dispatch thunks (and we don't need to handle the thunk, just know it
-    // was dispatched).
-    vi.spyOn(appThunks, "connectAndStartPolling").mockImplementation(
-      vi.fn().mockReturnValue({
-        type: "connectAndStartPolling",
-        catch: vi.fn(),
-      }),
-    );
-    const mockUseAppDispatch = vi.fn().mockReturnValue({
-      then: vi.fn().mockReturnValue({ catch: vi.fn() }),
-    });
-    vi.spyOn(dashboardStore, "useAppDispatch").mockReturnValue(
-      mockUseAppDispatch,
-    );
-    const state = rootStateFactory.build({
-      general: generalStateFactory.withConfig().build({
-        config: configFactory.build({
-          identityProviderAvailable: false,
-        }),
-      }),
-    });
-    renderComponent(<LogIn>App content</LogIn>, { state });
-    await userEvent.type(
-      screen.getByRole("textbox", { name: "Username" }),
-      "eggman",
-    );
-    await userEvent.type(screen.getByLabelText("Password"), "verysecure123");
-    await userEvent.click(screen.getByRole("button"));
-    const storeAction = generalActions.storeUserPass({
-      wsControllerURL: "wss://controller.example.com",
-      credential: { user: "eggman", password: "verysecure123" },
-    });
-    expect(mockUseAppDispatch.mock.calls[0][0]).toMatchObject({
-      type: "general/cleanupLoginErrors",
-    });
-    expect(mockUseAppDispatch.mock.calls[1][0]).toMatchObject(storeAction);
-    expect(mockUseAppDispatch.mock.calls[2][0]).toMatchObject({
-      type: "connectAndStartPolling",
-    });
-  });
-
   it("displays authentication request notifications", async () => {
     const state = rootStateFactory.build({
       general: generalStateFactory.withConfig().build({
@@ -206,6 +138,9 @@ describe("LogIn", () => {
   });
 
   it("should remove the authentication request when clicking the authenticate button", async () => {
+    const consoleError = console.error;
+    console.error = vi.fn();
+
     const state = rootStateFactory.build({
       general: generalStateFactory.withConfig().build({
         config: configFactory.build({
@@ -223,32 +158,7 @@ describe("LogIn", () => {
       { pointerEventsCheck: 0 },
     );
     expect(screen.queryByTestId("toast-card")).not.toBeInTheDocument();
-  });
 
-  it("should display console error when trying to log in", async () => {
-    vi.spyOn(appThunks, "connectAndStartPolling").mockImplementation(
-      vi.fn().mockReturnValue({ type: "connectAndStartPolling" }),
-    );
-    vi.spyOn(dashboardStore, "useAppDispatch").mockImplementation(
-      vi
-        .fn()
-        .mockReturnValue((action: unknown) =>
-          action instanceof Object &&
-          "type" in action &&
-          action.type === "connectAndStartPolling"
-            ? Promise.reject(
-                new Error("Error while dispatching connectAndStartPolling!"),
-              )
-            : null,
-        ),
-    );
-
-    renderComponent(<LogIn>App content</LogIn>);
-    await userEvent.click(screen.getByRole("button"));
-    expect(appThunks.connectAndStartPolling).toHaveBeenCalledTimes(1);
-    expect(console.error).toHaveBeenCalledWith(
-      Label.POLLING_ERROR,
-      new Error("Error while dispatching connectAndStartPolling!"),
-    );
+    console.error = consoleError;
   });
 });

src/components/LogIn/LogIn.tsx

@@ -1,6 +1,4 @@
-import { Spinner } from "@canonical/react-components";
-import { unwrapResult } from "@reduxjs/toolkit";
-import type { FormEvent, PropsWithChildren } from "react";
+import type { PropsWithChildren } from "react";
 import { useEffect, useRef } from "react";
 import reactHotToast from "react-hot-toast";
 import { useSelector } from "react-redux";
@@ -9,9 +7,6 @@ import FadeUpIn from "animations/FadeUpIn";
 import AuthenticationButton from "components/AuthenticationButton";
 import Logo from "components/Logo";
 import ToastCard from "components/ToastCard";
-import bakery from "juju/bakery";
-import { thunks as appThunks } from "store/app";
-import { actions as generalActions } from "store/general";
 import {
   getConfig,
   getLoginError,
@@ -20,10 +15,11 @@ import {
   isLoggedIn,
   getIsJuju,
 } from "store/general/selectors";
-import type { RootState } from "store/store";
-import { useAppDispatch, useAppSelector } from "store/store";
+import { useAppSelector } from "store/store";
 
 import "./_login.scss";
+import IdentityProviderForm from "./IdentityProviderForm";
+import UserPassForm from "./UserPassForm";
 import { ErrorResponse, Label } from "./types";
 
 export default function LogIn({ children }: PropsWithChildren) {
@@ -121,81 +117,3 @@ function generateErrorMessage(loginError?: string | null) {
     </p>
   );
 }
-
-function IdentityProviderForm({ userIsLoggedIn }: { userIsLoggedIn: boolean }) {
-  const visitURL = useSelector((state: RootState) => {
-    if (!userIsLoggedIn) {
-      // This form only gets displayed on the main login page, at which point
-      // there can only be one authentication request, so just return the
-      // first one.
-      return state?.general?.visitURLs?.[0];
-    }
-  });
-
-  return <Button visitURL={visitURL}></Button>;
-}
-
-interface LoginElements extends HTMLFormControlsCollection {
-  username: HTMLInputElement;
-  password: HTMLInputElement;
-}
-
-function UserPassForm() {
-  const dispatch = useAppDispatch();
-  const focus = useRef<HTMLInputElement>(null);
-  const wsControllerURL = useAppSelector(getWSControllerURL);
-
-  function handleSubmit(
-    e: FormEvent<HTMLFormElement & { elements: LoginElements }>,
-  ) {
-    e.preventDefault();
-    const elements = e.currentTarget.elements;
-    const user = elements.username.value;
-    const password = elements.password.value;
-    dispatch(generalActions.cleanupLoginErrors());
-    dispatch(
-      generalActions.storeUserPass({
-        wsControllerURL,
-        credential: { user, password },
-      }),
-    );
-    if (bakery) {
-      // TODO: Consider displaying an error alert.
-      dispatch(appThunks.connectAndStartPolling())
-        .then(unwrapResult)
-        .catch((error) => console.error(Label.POLLING_ERROR, error));
-    }
-  }
-
-  useEffect(() => {
-    focus.current?.focus();
-  }, []);
-
-  return (
-    <form onSubmit={handleSubmit}>
-      <label htmlFor="username">Username</label>
-      <input type="text" name="username" id="username" ref={focus} />
-      <label htmlFor="password">Password</label>
-      <input type="password" name="password" id="password" />
-      <button className="p-button--positive" type="submit">
-        Log in to the dashboard
-      </button>
-    </form>
-  );
-}
-
-function Button({ visitURL }: { visitURL?: string | null }) {
-  if (visitURL) {
-    return (
-      <AuthenticationButton appearance="positive" visitURL={visitURL}>
-        Log in to the dashboard
-      </AuthenticationButton>
-    );
-  } else {
-    return (
-      <button className="p-button--neutral" disabled>
-        <Spinner text="Connecting..." />
-      </button>
-    );
-  }
-}