sts-sponsors team mailing list archive

[Peter Makowski <mp+442091@xxxxxxxxxxxxxxxxxx>]

Peter Makowski has proposed merging ~petermakowski/maas-site-manager:refactor-auth-context-useReducer into maas-site-manager:main.

Commit message:
refactor: auth context useReducer
- split context providers into AppContext and AuthContext


Requested reviews:
  MAAS Committers (maas-committers)

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

refactor auth context to use useReducer
next step will be extracting async logic to middleware

QA Steps
Verify you can login to site manager and logout successfully 
-- 
Your team MAAS Committers is requested to review the proposed merge of ~petermakowski/maas-site-manager:refactor-auth-context-useReducer into maas-site-manager:main.

diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx
index cee9a8b..51431fb 100644
--- a/frontend/src/App.tsx
+++ b/frontend/src/App.tsx
@@ -2,10 +2,9 @@ import "./App.scss";
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
 import { createBrowserRouter, RouterProvider } from "react-router-dom";
 
-import { AppContextProvider, AuthContextProvider } from "./context";
-import routes from "./routes";
-
 import apiClient from "@/api";
+import { AppContextProvider, AuthContextProvider } from "@/context";
+import routes from "@/routes";
 
 const queryClient = new QueryClient();
 const router = createBrowserRouter(routes);
diff --git a/frontend/src/components/MainLayout/MainLayout.tsx b/frontend/src/components/MainLayout/MainLayout.tsx
index ee4e970..3347511 100644
--- a/frontend/src/components/MainLayout/MainLayout.tsx
+++ b/frontend/src/components/MainLayout/MainLayout.tsx
@@ -5,13 +5,12 @@ import { Col, Row, useOnEscapePressed, usePrevious } from "@canonical/react-comp
 import classNames from "classnames";
 import { matchPath, Outlet, useLocation } from "react-router-dom";
 
-import SecondaryNavigation from "../SecondaryNavigation";
-
 import { routesConfig } from "@/base/routesConfig";
 import type { RoutePath } from "@/base/routesConfig";
 import DocumentTitle from "@/components/DocumentTitle/DocumentTitle";
 import Navigation from "@/components/Navigation";
 import RemoveRegions from "@/components/RemoveRegions";
+import SecondaryNavigation from "@/components/SecondaryNavigation";
 import { useAppContext, useAuthContext } from "@/context";
 import TokensCreate from "@/pages/tokens/create";
 
diff --git a/frontend/src/context/AppContext.tsx b/frontend/src/context/AppContext.tsx
new file mode 100644
index 0000000..c7c41ec
--- /dev/null
+++ b/frontend/src/context/AppContext.tsx
@@ -0,0 +1,26 @@
+import { createContext, useContext, useState } from "react";
+
+import type { OnChangeFn, RowSelectionState } from "@tanstack/react-table";
+
+export const AppContext = createContext<{
+  rowSelection: RowSelectionState;
+  setRowSelection: OnChangeFn<RowSelectionState>;
+  sidebar: "removeRegions" | "createToken" | null;
+  setSidebar: (sidebar: "removeRegions" | "createToken" | null) => void;
+}>({
+  rowSelection: {},
+  setRowSelection: () => ({}),
+  sidebar: null,
+  setSidebar: () => null,
+});
+
+export const AppContextProvider = ({ children }: { children: React.ReactNode }) => {
+  const [rowSelection, setRowSelection] = useState<RowSelectionState>({});
+  const [sidebar, setSidebar] = useState<"removeRegions" | "createToken" | null>(null);
+
+  return (
+    <AppContext.Provider value={{ rowSelection, setRowSelection, sidebar, setSidebar }}>{children}</AppContext.Provider>
+  );
+};
+
+export const useAppContext = () => useContext(AppContext);
diff --git a/frontend/src/context.tsx b/frontend/src/context/AuthContext.tsx
similarity index 51%
rename from frontend/src/context.tsx
rename to frontend/src/context/AuthContext.tsx
index 9cd0980..69387ba 100644
--- a/frontend/src/context.tsx
+++ b/frontend/src/context/AuthContext.tsx
@@ -1,40 +1,16 @@
-import { createContext, useContext, useEffect, useState } from "react";
+import type { Reducer } from "react";
+import React, { createContext, useContext, useReducer } from "react";
 
-import type { OnChangeFn, RowSelectionState } from "@tanstack/react-table";
 import type { AxiosInstance } from "axios";
 import useLocalStorageState from "use-local-storage-state";
 
-import type { LoginError } from "@/hooks/api";
 import { useLoginMutation } from "@/hooks/api";
-
-export const AppContext = createContext<{
-  rowSelection: RowSelectionState;
-  setRowSelection: OnChangeFn<RowSelectionState>;
-  sidebar: "removeRegions" | "createToken" | null;
-  setSidebar: (sidebar: "removeRegions" | "createToken" | null) => void;
-}>({
-  rowSelection: {},
-  setRowSelection: () => ({}),
-  sidebar: null,
-  setSidebar: () => null,
-});
-
-export const AppContextProvider = ({ children }: { children: React.ReactNode }) => {
-  const [rowSelection, setRowSelection] = useState<RowSelectionState>({});
-  const [sidebar, setSidebar] = useState<"removeRegions" | "createToken" | null>(null);
-
-  return (
-    <AppContext.Provider value={{ rowSelection, setRowSelection, sidebar, setSidebar }}>{children}</AppContext.Provider>
-  );
-};
+import type { LoginError } from "@/hooks/api";
 
 type AuthStatus = "initial" | "authenticated" | "unauthorised";
 
 interface AuthContextType {
   status: AuthStatus;
-  setStatus: (status: AuthStatus) => void;
-  setAuthToken: (token: string) => void;
-  removeAuthToken: VoidFunction;
   login: ({ username, password }: { username: string; password: string }) => void;
   logout: (callback: VoidFunction) => void;
   isError: boolean;
@@ -43,15 +19,52 @@ interface AuthContextType {
 
 export const AuthContext = createContext<AuthContextType>({
   status: "initial",
-  setStatus: () => null,
-  setAuthToken: () => null,
-  removeAuthToken: () => null,
   login: () => null,
   logout: () => null,
   isError: false,
   failureReason: null,
 });
 
+export const actionTypes = {
+  LOGIN_SUCCESS: "LOGIN_SUCCESS",
+  LOGIN_ERROR: "LOGIN_ERROR",
+  LOGOUT: "LOGOUT",
+} as const;
+
+const status = {
+  AUTHENTICATED: "authenticated",
+  UNAUTHORISED: "unauthorised",
+} as const;
+
+type Status = (typeof status)[keyof typeof status];
+type ActionType = (typeof actionTypes)[keyof typeof actionTypes];
+type AuthToken = string | null;
+type AuthState = {
+  authToken: AuthToken;
+  status: Status;
+};
+type AuthAction =
+  | {
+      type: typeof actionTypes.LOGIN_SUCCESS;
+      payload: AuthToken;
+    }
+  | {
+      type: Exclude<ActionType, "LOGIN_SUCCESS">;
+    };
+
+const authReducer: Reducer<AuthState, AuthAction> = (state, action) => {
+  switch (action.type) {
+    case actionTypes.LOGIN_SUCCESS:
+      return { ...state, status: "authenticated", authToken: action.payload };
+    case actionTypes.LOGIN_ERROR:
+      return { ...state, status: "unauthorised", authToken: null };
+    case actionTypes.LOGOUT:
+      return { ...state, status: "unauthorised", authToken: null };
+    default:
+      return state;
+  }
+};
+
 export const AuthContextProvider = ({
   apiClient,
   children,
@@ -59,65 +72,70 @@ export const AuthContextProvider = ({
   apiClient: AxiosInstance;
   children: React.ReactNode;
 }) => {
-  const [authToken, setAuthToken, { removeItem: removeAuthToken }] = useLocalStorageState("jwtToken");
-  const [status, setStatus] = useState<AuthStatus>("initial");
+  const [persistedAuthToken, setPersistedAuthToken, { removeItem: removePersistedAuthToken }] =
+    useLocalStorageState<string>("jwtToken");
   const { mutateAsync, isError, failureReason } = useLoginMutation();
 
-  useEffect(() => {
-    if (!authToken) {
-      setStatus("unauthorised");
-    } else {
-      setStatus("authenticated");
-    }
-  }, [apiClient, authToken]);
+  const initialState: AuthState = {
+    authToken: persistedAuthToken ? persistedAuthToken : null,
+    status: persistedAuthToken ? "authenticated" : "unauthorised",
+  };
+
+  const [state, dispatch] = useReducer(authReducer, initialState);
 
   useEffect(() => {
-    if (authToken) {
+    if (state.status === "authenticated") {
       apiClient.interceptors.request.use(function (config) {
-        if (authToken) {
-          config.headers.Authorization = `Bearer ${authToken}`;
+        if (state.authToken) {
+          config.headers.Authorization = `Bearer ${state.authToken}`;
         }
         return config;
       });
     }
-  }, [apiClient, authToken]);
+  }, [apiClient.interceptors.request, state.authToken, state.status]);
 
   useEffect(() => {
     apiClient.interceptors.response.use(
       (response) => response,
       (error) => {
-        if (error.response.status === 401) {
-          removeAuthToken();
+        if (error?.response?.status === 401 || error?.status === 401) {
+          dispatch({ type: actionTypes.LOGOUT });
+          removePersistedAuthToken();
         }
         return Promise.reject(error);
       },
     );
-  }, [apiClient, removeAuthToken]);
+  }, [apiClient, removePersistedAuthToken]);
 
   const login = async ({ username, password }: { username: string; password: string }) => {
     try {
       const response = await mutateAsync({ username, password });
-      setAuthToken(response.access_token);
-      setStatus("authenticated");
+      dispatch({ type: actionTypes.LOGIN_SUCCESS, payload: response.access_token });
+      setPersistedAuthToken(response.access_token);
     } catch (error) {
-      setStatus("unauthorised");
+      dispatch({ type: actionTypes.LOGIN_ERROR });
     }
   };
 
   const logout = (callback: VoidFunction) => {
-    removeAuthToken();
-    setStatus("unauthorised");
+    dispatch({ type: actionTypes.LOGOUT });
+    removePersistedAuthToken();
     callback();
   };
 
   return (
     <AuthContext.Provider
-      value={{ status, setStatus, setAuthToken, removeAuthToken, login, logout, isError, failureReason }}
+      value={{
+        status: state.status,
+        login,
+        logout,
+        isError,
+        failureReason,
+      }}
     >
       {children}
     </AuthContext.Provider>
   );
 };
 
-export const useAppContext = () => useContext(AppContext);
 export const useAuthContext = () => useContext(AuthContext);
diff --git a/frontend/src/context/index.ts b/frontend/src/context/index.ts
new file mode 100644
index 0000000..d778afb
--- /dev/null
+++ b/frontend/src/context/index.ts
@@ -0,0 +1,2 @@
+export { AuthContext, AuthContextProvider, useAuthContext } from "./AuthContext";
+export { AppContext, AppContextProvider, useAppContext } from "./AppContext";
diff --git a/frontend/src/test-utils.tsx b/frontend/src/test-utils.tsx
index cabf08d..b247e2a 100644
--- a/frontend/src/test-utils.tsx
+++ b/frontend/src/test-utils.tsx
@@ -7,9 +7,8 @@ import { screen, render } from "@testing-library/react";
 import type { MemoryRouterProps } from "react-router-dom";
 import { MemoryRouter } from "react-router-dom";
 
-import { AppContextProvider, AuthContextProvider } from "./context";
-
 import apiClient from "@/api";
+import { AppContextProvider, AuthContextProvider } from "@/context";
 
 const queryClient = new QueryClient({
   defaultOptions: {