refactor(auth): simplify ProtectedRoute tests and enhance loading state handling

- Removed the renderWithRouter utility in ProtectedRoute tests, directly using the render function for clarity.
- Added a new test case to ensure that the ProtectedRoute does not redirect when hasCompletedSignup is null, addressing the loading state scenario.
- Updated ProtectedRoute component to wait for hasCompletedSignup to be determined before performing any redirects, improving user experience during authentication checks.

Author: tyler-dane

packages/web/src/auth/ProtectedRoute.test.tsx

@@ -1,7 +1,6 @@
-import { ReactElement } from "react";
-import { BrowserRouter } from "react-router-dom";
 import "@testing-library/jest-dom";
-import { render, waitFor } from "@testing-library/react";
+import { waitFor } from "@testing-library/react";
+import { render } from "@web/__tests__/__mocks__/mock.render";
 import { AUTH_FAILURE_REASONS } from "@web/common/constants/auth.constants";
 import { ROOT_ROUTES } from "@web/common/constants/routes";
 import { STORAGE_KEYS } from "@web/common/constants/storage.constants";
@@ -28,22 +27,6 @@ jest.mock("react-router-dom", () => ({
   useNavigate: () => mockNavigate,
 }));
 
-/**
- * Utility function to wrap a component in BrowserRouter with v7 future props
- */
-const renderWithRouter = (component: ReactElement) => {
-  return render(
-    <BrowserRouter
-      future={{
-        v7_startTransition: true,
-        v7_relativeSplatPath: true,
-      }}
-    >
-      {component}
-    </BrowserRouter>,
-  );
-};
-
 describe("ProtectedRoute", () => {
   beforeEach(() => {
     jest.clearAllMocks();
@@ -64,7 +47,7 @@ describe("ProtectedRoute", () => {
         markSignupCompleted: jest.fn(),
       });
 
-      renderWithRouter(
+      render(
         <ProtectedRoute>
           <div>Protected Content</div>
         </ProtectedRoute>,
@@ -87,7 +70,7 @@ describe("ProtectedRoute", () => {
         markSignupCompleted: jest.fn(),
       });
 
-      renderWithRouter(
+      render(
         <ProtectedRoute>
           <div>Protected Content</div>
         </ProtectedRoute>,
@@ -112,7 +95,7 @@ describe("ProtectedRoute", () => {
         markSignupCompleted: jest.fn(),
       });
 
-      renderWithRouter(
+      render(
         <ProtectedRoute>
           <div>Protected Content</div>
         </ProtectedRoute>,
@@ -137,7 +120,7 @@ describe("ProtectedRoute", () => {
         markSignupCompleted: jest.fn(),
       });
 
-      const { getByText } = renderWithRouter(
+      const { getByText } = render(
         <ProtectedRoute>
           <div>Protected Content</div>
         </ProtectedRoute>,
@@ -149,6 +132,31 @@ describe("ProtectedRoute", () => {
 
       expect(mockNavigate).not.toHaveBeenCalled();
     });
+
+    it("does not redirect when hasCompletedSignup is null (loading state)", async () => {
+      mockUseAuthCheck.mockReturnValue({
+        isAuthenticated: false,
+        isCheckingAuth: false,
+        isGoogleTokenActive: false,
+        isSessionActive: false,
+      });
+      mockUseHasCompletedSignup.mockReturnValue({
+        hasCompletedSignup: null,
+        markSignupCompleted: jest.fn(),
+      });
+
+      render(
+        <ProtectedRoute>
+          <div>Protected Content</div>
+        </ProtectedRoute>,
+      );
+
+      // Wait a bit to ensure redirect doesn't happen
+      await new Promise((resolve) => setTimeout(resolve, 100));
+
+      // Should not redirect while loading
+      expect(mockNavigate).not.toHaveBeenCalled();
+    });
   });
 
   describe("localStorage Integration", () => {
@@ -166,7 +174,7 @@ describe("ProtectedRoute", () => {
         markSignupCompleted: jest.fn(),
       });
 
-      renderWithRouter(
+      render(
         <ProtectedRoute>
           <div>Protected Content</div>
         </ProtectedRoute>,
@@ -194,7 +202,7 @@ describe("ProtectedRoute", () => {
         markSignupCompleted: jest.fn(),
       });
 
-      renderWithRouter(
+      render(
         <ProtectedRoute>
           <div>Protected Content</div>
         </ProtectedRoute>,

packages/web/src/auth/ProtectedRoute.tsx

@@ -16,6 +16,11 @@ export const ProtectedRoute = ({ children }: { children: ReactNode }) => {
   useEffect(() => {
     const handleAuthCheck = () => {
       if (isAuthenticated === false) {
+        // Wait for hasCompletedSignup to be determined (not null) before redirecting
+        if (hasCompletedSignup === null) {
+          return;
+        }
+
         // Check if user has completed signup to determine redirect destination
         if (hasCompletedSignup === true) {
           // User has completed signup before, redirect to /login

packages/web/src/views/Onboarding/OnboardingFlow.test.tsx

@@ -10,7 +10,7 @@ import { OnboardingFlow } from "./OnboardingFlow";
 
 // Mock navigate function
 const mockNavigate = jest.fn();
-const mockLocation = { pathname: ROOT_ROUTES.LOGIN };
+let mockLocationPathname = ROOT_ROUTES.LOGIN;
 
 // Mock dependencies
 jest.mock("@web/auth/useHasCompletedSignup");
@@ -19,7 +19,7 @@ jest.mock("@web/common/hooks/useIsMobile");
 jest.mock("react-router-dom", () => ({
   ...jest.requireActual("react-router-dom"),
   useNavigate: () => mockNavigate,
-  useLocation: () => mockLocation,
+  useLocation: () => ({ pathname: mockLocationPathname }),
 }));
 
 // Mock the Onboarding component to avoid complex rendering
@@ -67,7 +67,7 @@ describe("OnboardingFlow", () => {
       isSessionActive: false,
     });
     // Reset location to /login by default
-    mockLocation.pathname = ROOT_ROUTES.LOGIN;
+    mockLocationPathname = ROOT_ROUTES.LOGIN;
   });
 
   describe("New User Flow", () => {
@@ -240,7 +240,7 @@ describe("OnboardingFlow", () => {
   describe("Route-based Behavior", () => {
     describe("/onboarding route", () => {
       beforeEach(() => {
-        mockLocation.pathname = ROOT_ROUTES.ONBOARDING;
+        mockLocationPathname = ROOT_ROUTES.ONBOARDING;
       });
 
       it("ignores localStorage and always starts from beginning", () => {
@@ -323,7 +323,7 @@ describe("OnboardingFlow", () => {
 
     describe("/login route", () => {
       beforeEach(() => {
-        mockLocation.pathname = ROOT_ROUTES.LOGIN;
+        mockLocationPathname = ROOT_ROUTES.LOGIN;
       });
 
       it("preserves localStorage check behavior", () => {

packages/web/src/views/Onboarding/OnboardingFlow.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from "react";
+import { FC, useEffect, useState } from "react";
 import { useLocation, useNavigate } from "react-router-dom";
 import { useAuthCheck } from "@web/auth/useAuthCheck";
 import { useHasCompletedSignup } from "@web/auth/useHasCompletedSignup";
@@ -37,7 +37,7 @@ import { ReminderIntroOne } from "./steps/reminder/ReminderIntroOne";
 import { ReminderIntroTwo } from "./steps/reminder/ReminderIntroTwo";
 import { TasksToday } from "./steps/tasks/TasksToday/TasksToday";
 
-const _OnboardingFlow: React.FC = () => {
+const _OnboardingFlow: FC = () => {
   const navigate = useNavigate();
   const location = useLocation();
   const { setHideSteps } = useOnboarding();