From ab45139e7e208c3f05be2dac2b71b0b969edeb18 Mon Sep 17 00:00:00 2001 From: Leon Date: Sat, 19 Jul 2025 10:42:11 +0200 Subject: [PATCH] fix: auth disabled routing and UI --- .../src/app/login/__tests__/page.test.tsx | 37 ++++++++++++++++++- frontend/src/app/login/page.tsx | 19 ++++++++-- frontend/src/components/letterfeed/Header.tsx | 12 +++--- .../letterfeed/__tests__/Header.test.tsx | 36 +++++++++++++++--- frontend/src/contexts/AuthContext.tsx | 5 ++- .../contexts/__tests__/AuthContext.test.tsx | 26 +++++++++---- 6 files changed, 111 insertions(+), 24 deletions(-) diff --git a/frontend/src/app/login/__tests__/page.test.tsx b/frontend/src/app/login/__tests__/page.test.tsx index 6b0fef9..6abebd3 100644 --- a/frontend/src/app/login/__tests__/page.test.tsx +++ b/frontend/src/app/login/__tests__/page.test.tsx @@ -3,27 +3,38 @@ import { render, screen, fireEvent, waitFor } from "@testing-library/react" import "@testing-library/jest-dom" import LoginPage from "@/app/login/page" import { useAuth } from "@/hooks/useAuth" +import { useRouter } from "next/navigation" import { toast } from "sonner" +// Mock the necessary hooks and modules jest.mock("@/hooks/useAuth") jest.mock("sonner", () => ({ toast: { error: jest.fn(), }, })) +jest.mock("next/navigation", () => ({ + useRouter: jest.fn(), +})) const mockedUseAuth = useAuth as jest.Mock const mockedToast = toast as jest.Mocked +const mockedUseRouter = useRouter as jest.Mock describe("LoginPage", () => { const login = jest.fn() beforeEach(() => { jest.clearAllMocks() - mockedUseAuth.mockReturnValue({ login }) + // Default mock for useAuth + mockedUseAuth.mockReturnValue({ + login, + isAuthenticated: false, + isLoading: false, + }) }) - it("renders the login page", () => { + it("renders the login page when not authenticated", () => { render() expect(screen.getByText("LetterFeed")).toBeInTheDocument() expect(screen.getByLabelText("Username")).toBeInTheDocument() @@ -31,6 +42,28 @@ describe("LoginPage", () => { expect(screen.getByRole("button", { name: "Sign In" })).toBeInTheDocument() }) + it("shows loading spinner when isLoading is true", () => { + mockedUseAuth.mockReturnValue({ + login, + isAuthenticated: false, + isLoading: true, + }) + render() + expect(screen.getByTestId("loading-spinner")).toBeInTheDocument() + }) + + it("redirects when authenticated", () => { + const push = jest.fn() + mockedUseRouter.mockReturnValue({ push }) + mockedUseAuth.mockReturnValue({ + login, + isAuthenticated: true, + isLoading: false, + }) + render() + expect(push).toHaveBeenCalledWith("/") + }) + it("allows typing in the username and password fields", () => { render() const usernameInput = screen.getByLabelText("Username") diff --git a/frontend/src/app/login/page.tsx b/frontend/src/app/login/page.tsx index ab5cd9b..e3f9b50 100644 --- a/frontend/src/app/login/page.tsx +++ b/frontend/src/app/login/page.tsx @@ -1,7 +1,7 @@ "use client" import type React from "react" -import { useState } from "react" +import { useEffect, useState } from "react" import { Button } from "@/components/ui/button" import { Card, CardContent } from "@/components/ui/card" import { Input } from "@/components/ui/input" @@ -9,11 +9,20 @@ import { Label } from "@/components/ui/label" import Image from "next/image" import { useAuth } from "@/hooks/useAuth" import { toast } from "sonner" +import { useRouter } from "next/navigation" +import { LoadingSpinner } from "@/components/letterfeed/LoadingSpinner" export default function LoginPage() { const [username, setUsername] = useState("") const [password, setPassword] = useState("") - const { login } = useAuth() + const { login, isAuthenticated, isLoading } = useAuth() + const router = useRouter() + + useEffect(() => { + if (!isLoading && isAuthenticated) { + router.push("/") + } + }, [isLoading, isAuthenticated, router]) const handleLogin = async (e: React.FormEvent) => { e.preventDefault() @@ -25,11 +34,15 @@ export default function LoginPage() { try { await login(username, password) - } catch { + } catch { // The error is already toasted by the API layer, } } + if (isLoading || (!isLoading && isAuthenticated)) { + return + } + return (
diff --git a/frontend/src/components/letterfeed/Header.tsx b/frontend/src/components/letterfeed/Header.tsx index 3c908cb..7e10b47 100644 --- a/frontend/src/components/letterfeed/Header.tsx +++ b/frontend/src/components/letterfeed/Header.tsx @@ -13,7 +13,7 @@ interface HeaderProps { } export function Header({ onOpenAddNewsletter, onOpenSettings }: HeaderProps) { - const { logout } = useAuth() + const { logout, isAuthEnabled } = useAuth() const handleProcessEmails = async () => { try { await processEmails() @@ -62,10 +62,12 @@ export function Header({ onOpenAddNewsletter, onOpenSettings }: HeaderProps) { Settings - + {isAuthEnabled && ( + + )}
) diff --git a/frontend/src/components/letterfeed/__tests__/Header.test.tsx b/frontend/src/components/letterfeed/__tests__/Header.test.tsx index 5d8f7ec..ce02f26 100644 --- a/frontend/src/components/letterfeed/__tests__/Header.test.tsx +++ b/frontend/src/components/letterfeed/__tests__/Header.test.tsx @@ -4,6 +4,7 @@ import { Toaster } from "@/components/ui/sonner" import { toast } from "sonner" import * as api from "@/lib/api" import { AuthProvider } from "@/contexts/AuthContext" +import { useAuth } from "@/hooks/useAuth" jest.mock("@/lib/api") jest.mock("next/navigation", () => ({ @@ -11,7 +12,10 @@ jest.mock("next/navigation", () => ({ push: jest.fn(), }), })) +jest.mock("@/hooks/useAuth") + const mockedApi = api as jest.Mocked +const mockedUseAuth = useAuth as jest.Mock // Mock the toast functions jest.mock("sonner", () => { @@ -28,6 +32,7 @@ jest.mock("sonner", () => { describe("Header", () => { const onOpenAddNewsletter = jest.fn() const onOpenSettings = jest.fn() + const logout = jest.fn() const consoleError = jest.spyOn(console, "error").mockImplementation(() => {}) const renderWithAuthProvider = (component: React.ReactElement) => { @@ -37,14 +42,18 @@ describe("Header", () => { beforeEach(() => { jest.clearAllMocks() consoleError.mockClear() + mockedUseAuth.mockReturnValue({ + logout, + isAuthEnabled: true, // Default to auth being enabled for most tests + }) }) afterAll(() => { consoleError.mockRestore() }) - it("renders the header with title and buttons", () => { - renderWithAuthProvider( + it("renders the header with title and buttons including logout", () => { + render(
{ expect(screen.getByText("Add Newsletter")).toBeInTheDocument() expect(screen.getByText("Settings")).toBeInTheDocument() expect(screen.getByText("Process Now")).toBeInTheDocument() + expect(screen.getByText("Logout")).toBeInTheDocument() + }) + + it("does not render the logout button if auth is disabled", () => { + mockedUseAuth.mockReturnValue({ + logout, + isAuthEnabled: false, + }) + render( +
+ ) + expect(screen.queryByText("Logout")).not.toBeInTheDocument() }) it('calls onOpenAddNewsletter when "Add Newsletter" button is clicked', () => { - renderWithAuthProvider( + render(
{ }) it('calls onOpenSettings when "Settings" button is clicked', () => { - renderWithAuthProvider( + render(
{ it('calls the process emails API when "Process Now" button is clicked and shows success toast', async () => { mockedApi.processEmails.mockResolvedValue({ message: "Success" }) - renderWithAuthProvider( + render( <>
{ it("shows an error toast if the process emails API call fails", async () => { mockedApi.processEmails.mockRejectedValue(new Error("Failed to process")) - renderWithAuthProvider( + render( <>
Promise logout: () => void isLoading: boolean @@ -15,6 +16,7 @@ export const AuthContext = createContext(undefined) export const AuthProvider = ({ children }: { children: ReactNode }) => { const [isAuthenticated, setIsAuthenticated] = useState(false) + const [isAuthEnabled, setIsAuthEnabled] = useState(false) const [isLoading, setIsLoading] = useState(true) const router = useRouter() @@ -23,6 +25,7 @@ export const AuthProvider = ({ children }: { children: ReactNode }) => { setIsLoading(true); try { const { auth_enabled } = await getAuthStatus(); + setIsAuthEnabled(auth_enabled); if (!auth_enabled) { setIsAuthenticated(true); } else { @@ -67,7 +70,7 @@ export const AuthProvider = ({ children }: { children: ReactNode }) => { } return ( - + {children} ) diff --git a/frontend/src/contexts/__tests__/AuthContext.test.tsx b/frontend/src/contexts/__tests__/AuthContext.test.tsx index f25794c..d188111 100644 --- a/frontend/src/contexts/__tests__/AuthContext.test.tsx +++ b/frontend/src/contexts/__tests__/AuthContext.test.tsx @@ -28,21 +28,27 @@ describe("AuthContext", () => { consoleError.mockRestore() }) - it("authenticates if auth is not enabled", async () => { + it("authenticates and sets auth enabled to false if auth is not enabled on server", async () => { mockedApi.getAuthStatus.mockResolvedValue({ auth_enabled: false }) render( {(value) => ( - - Is Authenticated: {value?.isAuthenticated.toString()} - +
+ + Is Authenticated: {value?.isAuthenticated.toString()} + + + Is Auth Enabled: {value?.isAuthEnabled.toString()} + +
)} ) await waitFor(() => { expect(screen.getByText("Is Authenticated: true")).toBeInTheDocument() + expect(screen.getByText("Is Auth Enabled: false")).toBeInTheDocument() }) }) @@ -54,15 +60,21 @@ describe("AuthContext", () => { {(value) => ( - - Is Authenticated: {value?.isAuthenticated.toString()} - +
+ + Is Authenticated: {value?.isAuthenticated.toString()} + + + Is Auth Enabled: {value?.isAuthEnabled.toString()} + +
)} ) await waitFor(() => { expect(screen.getByText("Is Authenticated: true")).toBeInTheDocument() + expect(screen.getByText("Is Auth Enabled: true")).toBeInTheDocument() }) })