From 75cfc6c928f354168a914d408801b6c1001c892d Mon Sep 17 00:00:00 2001 From: counterweight Date: Fri, 19 Dec 2025 11:08:19 +0100 Subject: [PATCH] some fixes and refactors --- backend/main.py | 30 ++--- frontend/app/api.ts | 75 ++++++++++++ frontend/app/audit/page.test.tsx | 10 +- frontend/app/audit/page.tsx | 72 ++++-------- frontend/app/auth-context.tsx | 64 +++++------ frontend/app/components/Header.tsx | 85 ++++++++++++++ frontend/app/hooks/useRequireAuth.ts | 64 +++++++++++ frontend/app/page.test.tsx | 14 ++- frontend/app/page.tsx | 68 +++-------- frontend/app/profile/page.tsx | 108 +++++------------- frontend/app/sum/page.tsx | 65 ++--------- frontend/e2e/counter.spec.ts | 5 +- frontend/e2e/profile.spec.ts | 25 ++-- frontend/test-results/.last-run.json | 7 +- .../error-context.md | 57 --------- .../error-context.md | 57 --------- 16 files changed, 381 insertions(+), 425 deletions(-) create mode 100644 frontend/app/api.ts create mode 100644 frontend/app/components/Header.tsx create mode 100644 frontend/app/hooks/useRequireAuth.ts delete mode 100644 frontend/test-results/profile-Profile---Validati-940e7-x-validation-error-and-save/error-context.md delete mode 100644 frontend/test-results/profile-Profile---Validati-e4785-am-handle-that-is-too-short/error-context.md diff --git a/backend/main.py b/backend/main.py index cae2d61..64c5706 100644 --- a/backend/main.py +++ b/backend/main.py @@ -1,6 +1,6 @@ from contextlib import asynccontextmanager from datetime import datetime -from typing import Any, Callable, Generic, TypeVar +from typing import Callable, Generic, TypeVar from fastapi import FastAPI, Depends, HTTPException, Response, status, Query from fastapi.middleware.cors import CORSMiddleware @@ -8,6 +8,20 @@ from pydantic import BaseModel from sqlalchemy import select, func, desc from sqlalchemy.ext.asyncio import AsyncSession +from auth import ( + ACCESS_TOKEN_EXPIRE_MINUTES, + COOKIE_NAME, + UserCreate, + UserLogin, + UserResponse, + get_password_hash, + get_user_by_email, + authenticate_user, + create_access_token, + get_current_user, + require_permission, + build_user_response, +) from database import engine, get_db, Base from models import Counter, User, SumRecord, CounterRecord, Permission, Role, ROLE_REGULAR from validation import validate_profile_fields @@ -47,20 +61,6 @@ async def paginate_with_user_email( records: list[R] = [row_mapper(record, email) for record, email in rows] return records, total, total_pages -from auth import ( - ACCESS_TOKEN_EXPIRE_MINUTES, - COOKIE_NAME, - UserCreate, - UserLogin, - UserResponse, - get_password_hash, - get_user_by_email, - authenticate_user, - create_access_token, - get_current_user, - require_permission, - build_user_response, -) @asynccontextmanager diff --git a/frontend/app/api.ts b/frontend/app/api.ts new file mode 100644 index 0000000..75cca5d --- /dev/null +++ b/frontend/app/api.ts @@ -0,0 +1,75 @@ +import { API_URL } from "./config"; + +/** + * Simple API client that centralizes fetch configuration. + * All requests include credentials and proper headers. + */ + +export class ApiError extends Error { + constructor( + message: string, + public status: number, + public data?: unknown + ) { + super(message); + this.name = "ApiError"; + } +} + +async function request( + endpoint: string, + options: RequestInit = {} +): Promise { + const url = `${API_URL}${endpoint}`; + + const headers: HeadersInit = { + ...options.headers, + }; + + if (options.body && typeof options.body === "string") { + headers["Content-Type"] = "application/json"; + } + + const res = await fetch(url, { + ...options, + headers, + credentials: "include", + }); + + if (!res.ok) { + let data: unknown; + try { + data = await res.json(); + } catch { + // Response wasn't JSON + } + throw new ApiError( + `Request failed: ${res.status}`, + res.status, + data + ); + } + + return res.json(); +} + +export const api = { + get(endpoint: string): Promise { + return request(endpoint); + }, + + post(endpoint: string, body?: unknown): Promise { + return request(endpoint, { + method: "POST", + body: body ? JSON.stringify(body) : undefined, + }); + }, + + put(endpoint: string, body?: unknown): Promise { + return request(endpoint, { + method: "PUT", + body: body ? JSON.stringify(body) : undefined, + }); + }, +}; + diff --git a/frontend/app/audit/page.test.tsx b/frontend/app/audit/page.test.tsx index dee1b8f..720b492 100644 --- a/frontend/app/audit/page.test.tsx +++ b/frontend/app/audit/page.test.tsx @@ -20,6 +20,9 @@ const mockLogout = vi.fn(); const mockHasPermission = vi.fn((permission: string) => mockUser?.permissions.includes(permission) ?? false ); +const mockHasRole = vi.fn((role: string) => + mockUser?.roles.includes(role) ?? false +); vi.mock("../auth-context", () => ({ useAuth: () => ({ @@ -27,6 +30,7 @@ vi.mock("../auth-context", () => ({ isLoading: mockIsLoading, logout: mockLogout, hasPermission: mockHasPermission, + hasRole: mockHasRole, }), Permission: { VIEW_COUNTER: "view_counter", @@ -52,6 +56,9 @@ beforeEach(() => { mockHasPermission.mockImplementation((permission: string) => mockUser?.permissions.includes(permission) ?? false ); + mockHasRole.mockImplementation((role: string) => + mockUser?.roles.includes(role) ?? false + ); // Default: successful empty response mockFetch.mockResolvedValue({ ok: true, @@ -114,7 +121,8 @@ describe("AuditPage", () => { render(); await waitFor(() => { - expect(screen.getByText("Failed to load counter records")).toBeTruthy(); + const errors = screen.getAllByText("Request failed: 500"); + expect(errors.length).toBeGreaterThan(0); }); }); diff --git a/frontend/app/audit/page.tsx b/frontend/app/audit/page.tsx index 576eab5..8ee9620 100644 --- a/frontend/app/audit/page.tsx +++ b/frontend/app/audit/page.tsx @@ -1,10 +1,11 @@ "use client"; import { useEffect, useState, useCallback } from "react"; -import { useRouter } from "next/navigation"; -import { useAuth, Permission } from "../auth-context"; -import { API_URL } from "../config"; +import { Permission } from "../auth-context"; +import { api } from "../api"; import { sharedStyles } from "../styles/shared"; +import { Header } from "../components/Header"; +import { useRequireAuth } from "../hooks/useRequireAuth"; interface CounterRecord { id: number; @@ -38,31 +39,17 @@ export default function AuditPage() { const [sumError, setSumError] = useState(null); const [counterPage, setCounterPage] = useState(1); const [sumPage, setSumPage] = useState(1); - const { user, isLoading, logout, hasPermission } = useAuth(); - const router = useRouter(); - - const canViewAudit = hasPermission(Permission.VIEW_AUDIT); - - useEffect(() => { - if (!isLoading) { - if (!user) { - router.push("/login"); - } else if (!canViewAudit) { - router.push("/"); - } - } - }, [isLoading, user, router, canViewAudit]); + const { user, isLoading, isAuthorized } = useRequireAuth({ + requiredPermission: Permission.VIEW_AUDIT, + fallbackRedirect: "/", + }); const fetchCounterRecords = useCallback(async (page: number) => { setCounterError(null); try { - const res = await fetch(`${API_URL}/api/audit/counter?page=${page}&per_page=10`, { - credentials: "include", - }); - if (!res.ok) { - throw new Error("Failed to load counter records"); - } - const data = await res.json(); + const data = await api.get>( + `/api/audit/counter?page=${page}&per_page=10` + ); setCounterData(data); } catch (err) { setCounterData(null); @@ -73,13 +60,9 @@ export default function AuditPage() { const fetchSumRecords = useCallback(async (page: number) => { setSumError(null); try { - const res = await fetch(`${API_URL}/api/audit/sum?page=${page}&per_page=10`, { - credentials: "include", - }); - if (!res.ok) { - throw new Error("Failed to load sum records"); - } - const data = await res.json(); + const data = await api.get>( + `/api/audit/sum?page=${page}&per_page=10` + ); setSumData(data); } catch (err) { setSumData(null); @@ -88,21 +71,16 @@ export default function AuditPage() { }, []); useEffect(() => { - if (user && canViewAudit) { + if (user && isAuthorized) { fetchCounterRecords(counterPage); } - }, [user, counterPage, canViewAudit, fetchCounterRecords]); + }, [user, counterPage, isAuthorized, fetchCounterRecords]); useEffect(() => { - if (user && canViewAudit) { + if (user && isAuthorized) { fetchSumRecords(sumPage); } - }, [user, sumPage, canViewAudit, fetchSumRecords]); - - const handleLogout = async () => { - await logout(); - router.push("/login"); - }; + }, [user, sumPage, isAuthorized, fetchSumRecords]); const formatDate = (dateStr: string) => { return new Date(dateStr).toLocaleString(); @@ -116,23 +94,13 @@ export default function AuditPage() { ); } - if (!user || !canViewAudit) { + if (!user || !isAuthorized) { return null; } return (
-
-
- Audit -
-
- {user.email} - -
-
+
diff --git a/frontend/app/auth-context.tsx b/frontend/app/auth-context.tsx index 705b143..a5ba690 100644 --- a/frontend/app/auth-context.tsx +++ b/frontend/app/auth-context.tsx @@ -2,7 +2,7 @@ import { createContext, useContext, useState, useEffect, useCallback, ReactNode } from "react"; -import { API_URL } from "./config"; +import { api, ApiError } from "./api"; // Permission constants matching backend export const Permission = { @@ -43,13 +43,8 @@ export function AuthProvider({ children }: { children: ReactNode }) { const checkAuth = async () => { try { - const res = await fetch(`${API_URL}/api/auth/me`, { - credentials: "include", - }); - if (res.ok) { - const userData = await res.json(); - setUser(userData); - } + const userData = await api.get("/api/auth/me"); + setUser(userData); } catch { // Not authenticated } finally { @@ -58,44 +53,37 @@ export function AuthProvider({ children }: { children: ReactNode }) { }; const login = async (email: string, password: string) => { - const res = await fetch(`${API_URL}/api/auth/login`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - credentials: "include", - body: JSON.stringify({ email, password }), - }); - - if (!res.ok) { - const error = await res.json(); - throw new Error(error.detail || "Login failed"); + try { + const userData = await api.post("/api/auth/login", { email, password }); + setUser(userData); + } catch (err) { + if (err instanceof ApiError) { + const data = err.data as { detail?: string }; + throw new Error(data?.detail || "Login failed"); + } + throw err; } - - const userData = await res.json(); - setUser(userData); }; const register = async (email: string, password: string) => { - const res = await fetch(`${API_URL}/api/auth/register`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - credentials: "include", - body: JSON.stringify({ email, password }), - }); - - if (!res.ok) { - const error = await res.json(); - throw new Error(error.detail || "Registration failed"); + try { + const userData = await api.post("/api/auth/register", { email, password }); + setUser(userData); + } catch (err) { + if (err instanceof ApiError) { + const data = err.data as { detail?: string }; + throw new Error(data?.detail || "Registration failed"); + } + throw err; } - - const userData = await res.json(); - setUser(userData); }; const logout = async () => { - await fetch(`${API_URL}/api/auth/logout`, { - method: "POST", - credentials: "include", - }); + try { + await api.post("/api/auth/logout"); + } catch { + // Ignore errors on logout + } setUser(null); }; diff --git a/frontend/app/components/Header.tsx b/frontend/app/components/Header.tsx new file mode 100644 index 0000000..e42a4eb --- /dev/null +++ b/frontend/app/components/Header.tsx @@ -0,0 +1,85 @@ +"use client"; + +import { useRouter } from "next/navigation"; +import { useAuth } from "../auth-context"; +import { sharedStyles } from "../styles/shared"; + +type PageId = "counter" | "sum" | "profile" | "audit"; + +interface HeaderProps { + currentPage: PageId; +} + +interface NavItem { + id: PageId; + label: string; + href: string; + regularOnly?: boolean; +} + +const NAV_ITEMS: NavItem[] = [ + { id: "counter", label: "Counter", href: "/" }, + { id: "sum", label: "Sum", href: "/sum" }, + { id: "profile", label: "My Profile", href: "/profile", regularOnly: true }, +]; + +export function Header({ currentPage }: HeaderProps) { + const { user, logout, hasRole } = useAuth(); + const router = useRouter(); + const isRegularUser = hasRole("regular"); + + const handleLogout = async () => { + await logout(); + router.push("/login"); + }; + + if (!user) return null; + + // For audit page (admin), show only the current page label + if (currentPage === "audit") { + return ( +
+
+ Audit +
+
+ {user.email} + +
+
+ ); + } + + // For regular pages, build nav with links + const visibleItems = NAV_ITEMS.filter( + (item) => !item.regularOnly || isRegularUser + ); + + return ( +
+
+ {visibleItems.map((item, index) => ( + + {index > 0 && •} + {item.id === currentPage ? ( + {item.label} + ) : ( + + {item.label} + + )} + + ))} +
+
+ {user.email} + +
+
+ ); +} + diff --git a/frontend/app/hooks/useRequireAuth.ts b/frontend/app/hooks/useRequireAuth.ts new file mode 100644 index 0000000..3266461 --- /dev/null +++ b/frontend/app/hooks/useRequireAuth.ts @@ -0,0 +1,64 @@ +"use client"; + +import { useEffect } from "react"; +import { useRouter } from "next/navigation"; +import { useAuth, PermissionType, Permission } from "../auth-context"; + +interface UseRequireAuthOptions { + /** Required permission to access the page */ + requiredPermission?: PermissionType; + /** Required role to access the page */ + requiredRole?: string; + /** Where to redirect if permission check fails (defaults to best available page) */ + fallbackRedirect?: string; +} + +interface UseRequireAuthResult { + user: ReturnType["user"]; + isLoading: boolean; + isAuthorized: boolean; +} + +/** + * Hook that handles authentication and authorization checks. + * Automatically redirects to login if not authenticated, + * or to a fallback page if missing required permissions. + */ +export function useRequireAuth(options: UseRequireAuthOptions = {}): UseRequireAuthResult { + const { requiredPermission, requiredRole, fallbackRedirect } = options; + const { user, isLoading, hasPermission, hasRole } = useAuth(); + const router = useRouter(); + + const isAuthorized = (() => { + if (!user) return false; + if (requiredPermission && !hasPermission(requiredPermission)) return false; + if (requiredRole && !hasRole(requiredRole)) return false; + return true; + })(); + + useEffect(() => { + if (isLoading) return; + + if (!user) { + router.push("/login"); + return; + } + + if (!isAuthorized) { + // Redirect to the most appropriate page based on permissions + const redirect = fallbackRedirect ?? ( + hasPermission(Permission.VIEW_AUDIT) ? "/audit" : + hasPermission(Permission.VIEW_COUNTER) ? "/" : + "/login" + ); + router.push(redirect); + } + }, [isLoading, user, isAuthorized, router, fallbackRedirect, hasPermission]); + + return { + user, + isLoading, + isAuthorized, + }; +} + diff --git a/frontend/app/page.test.tsx b/frontend/app/page.test.tsx index 1a3ff04..1c8d106 100644 --- a/frontend/app/page.test.tsx +++ b/frontend/app/page.test.tsx @@ -75,6 +75,7 @@ describe("Home - Authenticated", () => { test("renders user email in header", async () => { vi.spyOn(global, "fetch").mockResolvedValue({ + ok: true, json: () => Promise.resolve({ value: 42 }), } as Response); @@ -84,6 +85,7 @@ describe("Home - Authenticated", () => { test("renders sign out button", async () => { vi.spyOn(global, "fetch").mockResolvedValue({ + ok: true, json: () => Promise.resolve({ value: 42 }), } as Response); @@ -93,6 +95,7 @@ describe("Home - Authenticated", () => { test("clicking sign out calls logout and redirects", async () => { vi.spyOn(global, "fetch").mockResolvedValue({ + ok: true, json: () => Promise.resolve({ value: 42 }), } as Response); @@ -107,6 +110,7 @@ describe("Home - Authenticated", () => { test("renders counter value after fetch", async () => { vi.spyOn(global, "fetch").mockResolvedValue({ + ok: true, json: () => Promise.resolve({ value: 42 }), } as Response); @@ -118,6 +122,7 @@ describe("Home - Authenticated", () => { test("fetches counter with credentials", async () => { const fetchSpy = vi.spyOn(global, "fetch").mockResolvedValue({ + ok: true, json: () => Promise.resolve({ value: 0 }), } as Response); @@ -135,6 +140,7 @@ describe("Home - Authenticated", () => { test("renders increment button", async () => { vi.spyOn(global, "fetch").mockResolvedValue({ + ok: true, json: () => Promise.resolve({ value: 0 }), } as Response); @@ -145,8 +151,8 @@ describe("Home - Authenticated", () => { test("clicking increment button calls API with credentials", async () => { const fetchSpy = vi .spyOn(global, "fetch") - .mockResolvedValueOnce({ json: () => Promise.resolve({ value: 0 }) } as Response) - .mockResolvedValueOnce({ json: () => Promise.resolve({ value: 1 }) } as Response); + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ value: 0 }) } as Response) + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ value: 1 }) } as Response); render(); await waitFor(() => expect(screen.getByText("0")).toBeDefined()); @@ -166,8 +172,8 @@ describe("Home - Authenticated", () => { test("clicking increment updates displayed count", async () => { vi.spyOn(global, "fetch") - .mockResolvedValueOnce({ json: () => Promise.resolve({ value: 0 }) } as Response) - .mockResolvedValueOnce({ json: () => Promise.resolve({ value: 1 }) } as Response); + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ value: 0 }) } as Response) + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ value: 1 }) } as Response); render(); await waitFor(() => expect(screen.getByText("0")).toBeDefined()); diff --git a/frontend/app/page.tsx b/frontend/app/page.tsx index aecf66d..187fdeb 100644 --- a/frontend/app/page.tsx +++ b/frontend/app/page.tsx @@ -1,55 +1,31 @@ "use client"; import { useEffect, useState } from "react"; -import { useRouter } from "next/navigation"; -import { useAuth, Permission } from "./auth-context"; -import { API_URL } from "./config"; +import { Permission } from "./auth-context"; +import { api } from "./api"; import { sharedStyles } from "./styles/shared"; +import { Header } from "./components/Header"; +import { useRequireAuth } from "./hooks/useRequireAuth"; export default function Home() { const [count, setCount] = useState(null); - const { user, isLoading, logout, hasPermission, hasRole } = useAuth(); - const router = useRouter(); - - const canViewCounter = hasPermission(Permission.VIEW_COUNTER); - const isRegularUser = hasRole("regular"); + const { user, isLoading, isAuthorized } = useRequireAuth({ + requiredPermission: Permission.VIEW_COUNTER, + }); useEffect(() => { - if (!isLoading) { - if (!user) { - router.push("/login"); - } else if (!canViewCounter) { - // Redirect to audit if user has audit permission, otherwise to login - router.push(hasPermission(Permission.VIEW_AUDIT) ? "/audit" : "/login"); - } - } - }, [isLoading, user, router, canViewCounter, hasPermission]); - - useEffect(() => { - if (user) { - fetch(`${API_URL}/api/counter`, { - credentials: "include", - }) - .then((res) => res.json()) + if (user && isAuthorized) { + api.get<{ value: number }>("/api/counter") .then((data) => setCount(data.value)) .catch(() => setCount(null)); } - }, [user]); + }, [user, isAuthorized]); const increment = async () => { - const res = await fetch(`${API_URL}/api/counter/increment`, { - method: "POST", - credentials: "include", - }); - const data = await res.json(); + const data = await api.post<{ value: number }>("/api/counter/increment"); setCount(data.value); }; - const handleLogout = async () => { - await logout(); - router.push("/login"); - }; - if (isLoading) { return (
@@ -58,31 +34,13 @@ export default function Home() { ); } - if (!user || !canViewCounter) { + if (!user || !isAuthorized) { return null; } return (
-
-
- Counter - • - Sum - {isRegularUser && ( - <> - • - My Profile - - )} -
-
- {user.email} - -
-
+
diff --git a/frontend/app/profile/page.tsx b/frontend/app/profile/page.tsx index b2ee3d0..478b2bb 100644 --- a/frontend/app/profile/page.tsx +++ b/frontend/app/profile/page.tsx @@ -1,11 +1,11 @@ "use client"; import { useEffect, useState, useCallback, useRef } from "react"; -import { useRouter } from "next/navigation"; import { bech32 } from "bech32"; -import { useAuth } from "../auth-context"; -import { API_URL } from "../config"; +import { api, ApiError } from "../api"; import { sharedStyles } from "../styles/shared"; +import { Header } from "../components/Header"; +import { useRequireAuth } from "../hooks/useRequireAuth"; interface ProfileData { contact_email: string | null; @@ -111,8 +111,10 @@ function toFormData(data: ProfileData): FormData { } export default function ProfilePage() { - const { user, isLoading, logout, hasRole } = useAuth(); - const router = useRouter(); + const { user, isLoading, isAuthorized } = useRequireAuth({ + requiredRole: "regular", + fallbackRedirect: "/audit", + }); const [originalData, setOriginalData] = useState(null); const [formData, setFormData] = useState({ contact_email: "", @@ -126,8 +128,6 @@ export default function ProfilePage() { const [toast, setToast] = useState<{ message: string; type: "success" | "error" } | null>(null); const validationTimeoutRef = useRef(null); - const isRegularUser = hasRole("regular"); - // Check if form has changes const hasChanges = useCallback(() => { if (!originalData) return false; @@ -144,42 +144,25 @@ export default function ProfilePage() { return Object.keys(errors).length === 0; }, [errors]); - useEffect(() => { - if (!isLoading) { - if (!user) { - router.push("/login"); - } else if (!isRegularUser) { - router.push("/audit"); - } - } - }, [isLoading, user, router, isRegularUser]); - const fetchProfile = useCallback(async () => { try { - const res = await fetch(`${API_URL}/api/profile`, { - credentials: "include", - }); - if (res.ok) { - const data: ProfileData = await res.json(); - const formValues = toFormData(data); - setFormData(formValues); - setOriginalData(formValues); - } else { - setToast({ message: "Failed to load profile", type: "error" }); - } + const data = await api.get("/api/profile"); + const formValues = toFormData(data); + setFormData(formValues); + setOriginalData(formValues); } catch (err) { console.error("Profile load error:", err); - setToast({ message: "Network error. Please try again.", type: "error" }); + setToast({ message: "Failed to load profile", type: "error" }); } finally { setIsLoadingProfile(false); } }, []); useEffect(() => { - if (user && isRegularUser) { + if (user && isAuthorized) { fetchProfile(); } - }, [user, isRegularUser, fetchProfile]); + }, [user, isAuthorized, fetchProfile]); // Auto-dismiss toast after 3 seconds useEffect(() => { @@ -238,47 +221,32 @@ export default function ProfilePage() { setIsSubmitting(true); try { - const res = await fetch(`${API_URL}/api/profile`, { - method: "PUT", - headers: { "Content-Type": "application/json" }, - credentials: "include", - body: JSON.stringify({ - contact_email: formData.contact_email || null, - telegram: formData.telegram || null, - signal: formData.signal || null, - nostr_npub: formData.nostr_npub || null, - }), + const data = await api.put("/api/profile", { + contact_email: formData.contact_email || null, + telegram: formData.telegram || null, + signal: formData.signal || null, + nostr_npub: formData.nostr_npub || null, }); - - if (res.ok) { - const data: ProfileData = await res.json(); - const formValues = toFormData(data); - setFormData(formValues); - setOriginalData(formValues); - setToast({ message: "Profile saved successfully!", type: "success" }); - } else if (res.status === 422) { - // Handle validation errors from backend - const errorData = await res.json(); - if (errorData.detail?.field_errors) { + const formValues = toFormData(data); + setFormData(formValues); + setOriginalData(formValues); + setToast({ message: "Profile saved successfully!", type: "success" }); + } catch (err) { + console.error("Profile save error:", err); + if (err instanceof ApiError && err.status === 422) { + const errorData = err.data as { detail?: { field_errors?: FieldErrors } }; + if (errorData?.detail?.field_errors) { setErrors(errorData.detail.field_errors); } setToast({ message: "Please fix the errors below", type: "error" }); } else { - setToast({ message: "Failed to save profile", type: "error" }); + setToast({ message: "Network error. Please try again.", type: "error" }); } - } catch (err) { - console.error("Profile save error:", err); - setToast({ message: "Network error. Please try again.", type: "error" }); } finally { setIsSubmitting(false); } }; - const handleLogout = async () => { - await logout(); - router.push("/login"); - }; - if (isLoading || isLoadingProfile) { return (
@@ -287,7 +255,7 @@ export default function ProfilePage() { ); } - if (!user || !isRegularUser) { + if (!user || !isAuthorized) { return null; } @@ -307,21 +275,7 @@ export default function ProfilePage() {
)} -
-
- Counter - • - Sum - • - My Profile -
-
- {user.email} - -
-
+
diff --git a/frontend/app/sum/page.tsx b/frontend/app/sum/page.tsx index b372ef2..c644a0f 100644 --- a/frontend/app/sum/page.tsx +++ b/frontend/app/sum/page.tsx @@ -1,10 +1,11 @@ "use client"; -import { useEffect, useState } from "react"; -import { useRouter } from "next/navigation"; -import { useAuth, Permission } from "../auth-context"; -import { API_URL } from "../config"; +import { useState } from "react"; +import { Permission } from "../auth-context"; +import { api } from "../api"; import { sharedStyles } from "../styles/shared"; +import { Header } from "../components/Header"; +import { useRequireAuth } from "../hooks/useRequireAuth"; export default function SumPage() { const [a, setA] = useState(""); @@ -12,21 +13,9 @@ export default function SumPage() { const [result, setResult] = useState(null); const [showResult, setShowResult] = useState(false); const [error, setError] = useState(null); - const { user, isLoading, logout, hasPermission, hasRole } = useAuth(); - const router = useRouter(); - - const canUseSum = hasPermission(Permission.USE_SUM); - const isRegularUser = hasRole("regular"); - - useEffect(() => { - if (!isLoading) { - if (!user) { - router.push("/login"); - } else if (!canUseSum) { - router.push(hasPermission(Permission.VIEW_AUDIT) ? "/audit" : "/login"); - } - } - }, [isLoading, user, router, canUseSum, hasPermission]); + const { user, isLoading, isAuthorized } = useRequireAuth({ + requiredPermission: Permission.USE_SUM, + }); const handleSum = async () => { const numA = parseFloat(a) || 0; @@ -34,16 +23,7 @@ export default function SumPage() { setError(null); try { - const res = await fetch(`${API_URL}/api/sum`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - credentials: "include", - body: JSON.stringify({ a: numA, b: numB }), - }); - if (!res.ok) { - throw new Error("Calculation failed"); - } - const data = await res.json(); + const data = await api.post<{ result: number }>("/api/sum", { a: numA, b: numB }); setResult(data.result); setShowResult(true); } catch (err) { @@ -59,11 +39,6 @@ export default function SumPage() { setError(null); }; - const handleLogout = async () => { - await logout(); - router.push("/login"); - }; - if (isLoading) { return (
@@ -72,31 +47,13 @@ export default function SumPage() { ); } - if (!user || !canUseSum) { + if (!user || !isAuthorized) { return null; } return (
-
-
- Counter - • - Sum - {isRegularUser && ( - <> - • - My Profile - - )} -
-
- {user.email} - -
-
+
diff --git a/frontend/e2e/counter.spec.ts b/frontend/e2e/counter.spec.ts index fd36afd..20a3bd7 100644 --- a/frontend/e2e/counter.spec.ts +++ b/frontend/e2e/counter.spec.ts @@ -100,7 +100,10 @@ test.describe("Counter - Authenticated", () => { // Second user increments await page2.click("text=Increment"); - await expect(page2.locator("h1")).toHaveText(String(page2InitialValue + 1)); + // Wait for counter to update - use >= because parallel tests may also increment + await expect(page2.locator("h1")).not.toHaveText(String(page2InitialValue)); + const page2AfterIncrement = Number(await page2.locator("h1").textContent()); + expect(page2AfterIncrement).toBeGreaterThanOrEqual(page2InitialValue + 1); // First user reloads and sees the increment (value should be >= what page2 has) await page.reload(); diff --git a/frontend/e2e/profile.spec.ts b/frontend/e2e/profile.spec.ts index dd4736c..bd90d26 100644 --- a/frontend/e2e/profile.spec.ts +++ b/frontend/e2e/profile.spec.ts @@ -237,17 +237,17 @@ test.describe("Profile - Validation", () => { await expect(page.locator("#telegram")).toHaveValue("@testhandle"); }); - test("shows error for telegram handle that is too short", async ({ page }) => { + test("shows error for telegram handle with no characters after @", async ({ page }) => { await page.goto("/profile"); - // Enter telegram with @ but too short (needs 5+ chars) - await page.fill("#telegram", "@ab"); + // Enter telegram with @ but nothing after (needs at least 1 char) + await page.fill("#telegram", "@"); // Wait for debounced validation await page.waitForTimeout(600); - // Should show error about length - await expect(page.getByText(/at least 5 characters/i)).toBeVisible(); + // Should show error about needing at least one character + await expect(page.getByText(/at least one character after @/i)).toBeVisible(); // Save button should be disabled const saveButton = page.getByRole("button", { name: /save changes/i }); @@ -271,15 +271,22 @@ test.describe("Profile - Validation", () => { test("can fix validation error and save", async ({ page }) => { await page.goto("/profile"); - // Enter invalid telegram - await page.fill("#telegram", "noat"); - await expect(page.getByText(/must start with @/i)).toBeVisible(); + // Enter invalid telegram (just @ with no handle) + await page.fill("#telegram", "@"); + + // Wait for debounced validation + await page.waitForTimeout(600); + + await expect(page.getByText(/at least one character after @/i)).toBeVisible(); // Fix it await page.fill("#telegram", "@validhandle"); + // Wait for debounced validation + await page.waitForTimeout(600); + // Error should disappear - await expect(page.getByText(/must start with @/i)).not.toBeVisible(); + await expect(page.getByText(/at least one character after @/i)).not.toBeVisible(); // Should be able to save const saveButton = page.getByRole("button", { name: /save changes/i }); diff --git a/frontend/test-results/.last-run.json b/frontend/test-results/.last-run.json index 56cd5cc..cbcc1fb 100644 --- a/frontend/test-results/.last-run.json +++ b/frontend/test-results/.last-run.json @@ -1,7 +1,4 @@ { - "status": "failed", - "failedTests": [ - "e8b79b4ee550a37632f1-b6f4d12ec6021e7a3bc8", - "e8b79b4ee550a37632f1-600f6ae7070fb14ef7f9" - ] + "status": "passed", + "failedTests": [] } \ No newline at end of file diff --git a/frontend/test-results/profile-Profile---Validati-940e7-x-validation-error-and-save/error-context.md b/frontend/test-results/profile-Profile---Validati-940e7-x-validation-error-and-save/error-context.md deleted file mode 100644 index b455bdb..0000000 --- a/frontend/test-results/profile-Profile---Validati-940e7-x-validation-error-and-save/error-context.md +++ /dev/null @@ -1,57 +0,0 @@ -# Page snapshot - -```yaml -- generic [ref=e1]: - - main [ref=e2]: - - generic [ref=e3]: - - generic [ref=e4]: - - link "Counter" [ref=e5] [cursor=pointer]: - - /url: / - - generic [ref=e6]: • - - link "Sum" [ref=e7] [cursor=pointer]: - - /url: /sum - - generic [ref=e8]: • - - generic [ref=e9]: My Profile - - generic [ref=e10]: - - generic [ref=e11]: user@example.com - - button "Sign out" [ref=e12] [cursor=pointer] - - generic [ref=e14]: - - generic [ref=e15]: - - heading "My Profile" [level=1] [ref=e16] - - paragraph [ref=e17]: Manage your contact information - - generic [ref=e18]: - - generic [ref=e19]: - - generic [ref=e20]: - - text: Login Email - - generic [ref=e21]: Read only - - textbox [disabled] [ref=e22]: user@example.com - - generic [ref=e23]: This is your login email and cannot be changed here. - - paragraph [ref=e25]: Contact Details - - paragraph [ref=e26]: These are for communication purposes only — they won't affect your login. - - generic [ref=e27]: - - generic [ref=e28]: Contact Email - - textbox "Contact Email" [ref=e29]: - - /placeholder: alternate@example.com - - generic [ref=e30]: - - generic [ref=e31]: Telegram - - textbox "Telegram" [active] [ref=e32]: - - /placeholder: "@username" - - text: "@noat" - - generic [ref=e33]: - - generic [ref=e34]: Signal - - textbox "Signal" [ref=e35]: - - /placeholder: username.01 - - generic [ref=e36]: - - generic [ref=e37]: Nostr (npub) - - textbox "Nostr (npub)" [ref=e38]: - - /placeholder: npub1... - - button "Save Changes" [ref=e39] [cursor=pointer] - - status [ref=e40]: - - generic [ref=e41]: - - img [ref=e43] - - generic [ref=e45]: - - text: Static route - - button "Hide static indicator" [ref=e46] [cursor=pointer]: - - img [ref=e47] - - alert [ref=e50] -``` \ No newline at end of file diff --git a/frontend/test-results/profile-Profile---Validati-e4785-am-handle-that-is-too-short/error-context.md b/frontend/test-results/profile-Profile---Validati-e4785-am-handle-that-is-too-short/error-context.md deleted file mode 100644 index 595df88..0000000 --- a/frontend/test-results/profile-Profile---Validati-e4785-am-handle-that-is-too-short/error-context.md +++ /dev/null @@ -1,57 +0,0 @@ -# Page snapshot - -```yaml -- generic [ref=e1]: - - main [ref=e2]: - - generic [ref=e3]: - - generic [ref=e4]: - - link "Counter" [ref=e5] [cursor=pointer]: - - /url: / - - generic [ref=e6]: • - - link "Sum" [ref=e7] [cursor=pointer]: - - /url: /sum - - generic [ref=e8]: • - - generic [ref=e9]: My Profile - - generic [ref=e10]: - - generic [ref=e11]: user@example.com - - button "Sign out" [ref=e12] [cursor=pointer] - - generic [ref=e14]: - - generic [ref=e15]: - - heading "My Profile" [level=1] [ref=e16] - - paragraph [ref=e17]: Manage your contact information - - generic [ref=e18]: - - generic [ref=e19]: - - generic [ref=e20]: - - text: Login Email - - generic [ref=e21]: Read only - - textbox [disabled] [ref=e22]: user@example.com - - generic [ref=e23]: This is your login email and cannot be changed here. - - paragraph [ref=e25]: Contact Details - - paragraph [ref=e26]: These are for communication purposes only — they won't affect your login. - - generic [ref=e27]: - - generic [ref=e28]: Contact Email - - textbox "Contact Email" [ref=e29]: - - /placeholder: alternate@example.com - - generic [ref=e30]: - - generic [ref=e31]: Telegram - - textbox "Telegram" [active] [ref=e32]: - - /placeholder: "@username" - - text: "@ab" - - generic [ref=e33]: - - generic [ref=e34]: Signal - - textbox "Signal" [ref=e35]: - - /placeholder: username.01 - - generic [ref=e36]: - - generic [ref=e37]: Nostr (npub) - - textbox "Nostr (npub)" [ref=e38]: - - /placeholder: npub1... - - button "Save Changes" [ref=e39] [cursor=pointer] - - status [ref=e40]: - - generic [ref=e41]: - - img [ref=e43] - - generic [ref=e45]: - - text: Static route - - button "Hide static indicator" [ref=e46] [cursor=pointer]: - - img [ref=e47] - - alert [ref=e50] -``` \ No newline at end of file