From 55b4b7cc25c9d794a2822065321ace0924efb8b1 Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Sat, 16 May 2026 16:02:51 -0400 Subject: [PATCH 01/14] refactor(user-context): switch profile loading to Suspense Replace useEffect/useState fetch with a memoized promise consumed via React 19's use() hook. Drops isLoading/error from the context surface; errors propagate to the nearest ErrorBoundary, loading to the nearest Suspense fallback. Header no longer needs an isLoading branch and is wrapped in Suspense to handle the pending state during initial load. Tests updated to render through Suspense + ErrorBoundary instead of asserting on isLoading/error fields. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/(web)/layout.tsx | 5 +- src/components/layout/header.tsx | 5 +- src/contexts/user-context.test.tsx | 104 ++++++++++++++++++++--------- src/contexts/user-context.tsx | 83 +++++++++++------------ 4 files changed, 118 insertions(+), 79 deletions(-) diff --git a/src/app/(web)/layout.tsx b/src/app/(web)/layout.tsx index 47ca85e9..552edc6b 100644 --- a/src/app/(web)/layout.tsx +++ b/src/app/(web)/layout.tsx @@ -1,4 +1,5 @@ import type { Metadata, Viewport } from "next"; +import { Suspense } from "react"; import { Geist, Geist_Mono } from "next/font/google"; import { Providers } from "@/app/providers"; import { AuthWrapper, Header, DynamicBreadcrumb } from "@/components/layout"; @@ -34,7 +35,9 @@ export default async function WebLayout({
-
+ }> +
+
diff --git a/src/components/layout/header.tsx b/src/components/layout/header.tsx index d0b9425b..bf045789 100644 --- a/src/components/layout/header.tsx +++ b/src/components/layout/header.tsx @@ -10,7 +10,7 @@ import { useAppSession, useUser } from "@/contexts"; export function Header() { const [sidebarOpen, setSidebarOpen] = useState(false); - const { userProfile, isLoading } = useUser(); + const { userProfile } = useUser(); const session = useAppSession(); return ( @@ -33,7 +33,7 @@ export function Header() { {/* Right side - User avatar */}
- {!isLoading && userProfile ? ( + {userProfile ? ( diff --git a/src/contexts/user-context.test.tsx b/src/contexts/user-context.test.tsx index c19785a7..6e70a594 100644 --- a/src/contexts/user-context.test.tsx +++ b/src/contexts/user-context.test.tsx @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { renderHook, waitFor, act } from '@testing-library/react'; -import { ReactNode } from 'react'; +import { render, renderHook, screen, waitFor, act } from '@testing-library/react'; +import { Component, ReactNode, Suspense } from 'react'; const { mockUseSession, mockGetCurrentUserProfile } = vi.hoisted(() => ({ mockUseSession: vi.fn(), @@ -19,10 +19,46 @@ vi.mock('@/components/shared-actions/user', () => ({ import { UserProvider, useUser } from './user-context'; -function createWrapper() { - return function Wrapper({ children }: { children: ReactNode }) { - return {children}; - }; +function ProfileProbe({ + onRefresh, +}: { + onRefresh?: (fn: () => void) => void; +}) { + const { userProfile, refreshUserProfile } = useUser(); + if (onRefresh) onRefresh(refreshUserProfile); + return ( + {userProfile?.First_Name ?? 'none'} + ); +} + +class Boundary extends Component< + { children: ReactNode }, + { error: Error | null } +> { + state = { error: null as Error | null }; + static getDerivedStateFromError(error: Error) { + return { error }; + } + render() { + if (this.state.error) { + return
{this.state.error.message}
; + } + return this.props.children; + } +} + +async function renderWithProvider(ui: ReactNode) { + let result!: ReturnType; + await act(async () => { + result = render( + + + loading
}>{ui} + + + ); + }); + return result; } describe('UserContext', () => { @@ -32,7 +68,6 @@ describe('UserContext', () => { describe('useUser', () => { it('should throw when used outside UserProvider', () => { - // Suppress console.error for this test const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); expect(() => { @@ -58,62 +93,58 @@ describe('UserContext', () => { }); mockGetCurrentUserProfile.mockResolvedValueOnce(mockProfile); - const { result } = renderHook(() => useUser(), { wrapper: createWrapper() }); + await renderWithProvider(); await waitFor(() => { - expect(result.current.isLoading).toBe(false); + expect(screen.getByTestId('name')).toHaveTextContent('John'); }); - - expect(result.current.userProfile).toEqual(mockProfile); - expect(result.current.error).toBeNull(); expect(mockGetCurrentUserProfile).toHaveBeenCalledWith('guid-123'); }); - it('should set null profile when no session', async () => { + it('should resolve to null profile when no session', async () => { mockUseSession.mockReturnValue({ data: null, isPending: false, }); - const { result } = renderHook(() => useUser(), { wrapper: createWrapper() }); + await renderWithProvider(); await waitFor(() => { - expect(result.current.isLoading).toBe(false); + expect(screen.getByTestId('name')).toHaveTextContent('none'); }); - - expect(result.current.userProfile).toBeNull(); expect(mockGetCurrentUserProfile).not.toHaveBeenCalled(); }); - it('should not fetch profile when session has no userGuid', () => { + it('should not fetch profile when session has no userGuid', async () => { mockUseSession.mockReturnValue({ data: { user: { id: 'internal-id' } }, isPending: false, }); - const { result } = renderHook(() => useUser(), { wrapper: createWrapper() }); + await renderWithProvider(); - // When session exists but has no userGuid, neither effect branch triggers - expect(result.current.userProfile).toBeNull(); + await waitFor(() => { + expect(screen.getByTestId('name')).toHaveTextContent('none'); + }); expect(mockGetCurrentUserProfile).not.toHaveBeenCalled(); }); - it('should handle profile load error', async () => { + it('should propagate profile load error to ErrorBoundary', async () => { mockUseSession.mockReturnValue({ data: { user: { id: 'internal-id', userGuid: 'guid-123' } }, isPending: false, }); mockGetCurrentUserProfile.mockRejectedValueOnce(new Error('Network error')); - const { result } = renderHook(() => useUser(), { wrapper: createWrapper() }); + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await renderWithProvider(); await waitFor(() => { - expect(result.current.isLoading).toBe(false); + expect(screen.getByTestId('err')).toHaveTextContent('Network error'); }); - expect(result.current.userProfile).toBeNull(); - expect(result.current.error).toBeInstanceOf(Error); - expect(result.current.error?.message).toBe('Network error'); + spy.mockRestore(); }); it('should refresh profile when refreshUserProfile is called', async () => { @@ -128,17 +159,28 @@ describe('UserContext', () => { .mockResolvedValueOnce(mockProfile) .mockResolvedValueOnce(updatedProfile); - const { result } = renderHook(() => useUser(), { wrapper: createWrapper() }); + const refreshRef: { current: (() => void) | null } = { current: null }; + + await renderWithProvider( + { + refreshRef.current = fn; + }} + /> + ); await waitFor(() => { - expect(result.current.userProfile).toEqual(mockProfile); + expect(screen.getByTestId('name')).toHaveTextContent('John'); }); await act(async () => { - await result.current.refreshUserProfile(); + refreshRef.current?.(); + }); + + await waitFor(() => { + expect(screen.getByTestId('name')).toHaveTextContent('Jane'); }); - expect(result.current.userProfile).toEqual(updatedProfile); expect(mockGetCurrentUserProfile).toHaveBeenCalledTimes(2); }); }); diff --git a/src/contexts/user-context.tsx b/src/contexts/user-context.tsx index f2523abf..772358e5 100644 --- a/src/contexts/user-context.tsx +++ b/src/contexts/user-context.tsx @@ -1,15 +1,22 @@ "use client"; -import { createContext, useContext, useState, useEffect, useCallback, ReactNode } from "react"; +import { + createContext, + useContext, + useState, + useMemo, + useCallback, + useTransition, + use, + ReactNode, +} from "react"; import { authClient } from "@/lib/auth-client"; import { MPUserProfile } from "@/lib/providers/ministry-platform/types"; import { getCurrentUserProfile } from "@/components/shared-actions/user"; interface UserContextValue { - userProfile: MPUserProfile | null; - isLoading: boolean; - error: Error | null; - refreshUserProfile: () => Promise; + userProfilePromise: Promise; + refreshUserProfile: () => void; } const UserContext = createContext(undefined); @@ -20,58 +27,46 @@ interface UserProviderProps { export function UserProvider({ children }: UserProviderProps) { const { data: session, isPending } = authClient.useSession(); - const [userProfile, setUserProfile] = useState(null); - const [isLoading, setIsLoading] = useState(true); - const [error, setError] = useState(null); + const [refreshKey, setRefreshKey] = useState(0); + const [, startTransition] = useTransition(); // userGuid is the MP User_GUID stored as an additionalField on the Better Auth user. // Better Auth generates its own internal user.id, so we use userGuid for MP lookups. const userGuid = (session?.user as { userGuid?: string } | undefined)?.userGuid; - const loadUserProfile = useCallback(async () => { - if (!userGuid) { - setUserProfile(null); - setIsLoading(false); - return; - } + const userProfilePromise = useMemo>(() => { + if (isPending) return Promise.resolve(null); + if (!userGuid) return Promise.resolve(null); + // refreshKey is read to invalidate the memo on refresh, even though + // it isn't otherwise referenced in the body. + void refreshKey; + return getCurrentUserProfile(userGuid).then((p) => p ?? null); + }, [userGuid, isPending, refreshKey]); - try { - setIsLoading(true); - setError(null); - const profile = await getCurrentUserProfile(userGuid); - setUserProfile(profile ?? null); - } catch (err) { - setError(err instanceof Error ? err : new Error("Failed to load user profile")); - setUserProfile(null); - } finally { - setIsLoading(false); - } - }, [userGuid]); + const refreshUserProfile = useCallback(() => { + startTransition(() => { + setRefreshKey((k) => k + 1); + }); + }, []); - useEffect(() => { - if (!isPending && userGuid) { - loadUserProfile(); - } else if (!isPending && !session) { - setUserProfile(null); - setIsLoading(false); - } - }, [userGuid, isPending, loadUserProfile, session]); + const value = useMemo( + () => ({ userProfilePromise, refreshUserProfile }), + [userProfilePromise, refreshUserProfile] + ); - const refreshUserProfile = async () => { - await loadUserProfile(); - }; + return {children}; +} - return ( - - {children} - - ); +interface UseUserResult { + userProfile: MPUserProfile | null; + refreshUserProfile: () => void; } -export function useUser() { +export function useUser(): UseUserResult { const context = useContext(UserContext); if (context === undefined) { throw new Error("useUser must be used within a UserProvider"); } - return context; + const userProfile = use(context.userProfilePromise); + return { userProfile, refreshUserProfile: context.refreshUserProfile }; } From 1ffed425eaa875781484b553eac3335c5db7d6ba Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Sat, 16 May 2026 16:03:00 -0400 Subject: [PATCH 02/14] refactor(contact-lookup): stream details via Suspense and use() Hoist getContactDetails / getContactLogsByContactId calls into the server component as promises and pass them down. The client component unwraps with use(), letting Suspense own the loading UI and the route boundary own errors. Refresh now delegates to router.refresh() instead of a local re-fetch, so server-side data stays the source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/(web)/contactlookup/[guid]/page.tsx | 26 +++++- .../contact-lookup-details.tsx | 84 ++++--------------- 2 files changed, 40 insertions(+), 70 deletions(-) diff --git a/src/app/(web)/contactlookup/[guid]/page.tsx b/src/app/(web)/contactlookup/[guid]/page.tsx index d0dbb8d0..4b2dbafe 100644 --- a/src/app/(web)/contactlookup/[guid]/page.tsx +++ b/src/app/(web)/contactlookup/[guid]/page.tsx @@ -1,4 +1,9 @@ +import { Suspense } from "react"; import { ContactLookupDetails } from "@/components/contact-lookup-details"; +import { + getContactDetails, + getContactLogsByContactId, +} from "@/components/contact-lookup-details/actions"; interface ContactLookupDetailPageProps { params: Promise<{ @@ -11,9 +16,28 @@ export default async function ContactLookupDetailPage({ }: ContactLookupDetailPageProps) { const { guid } = await params; + const contactPromise = getContactDetails(guid); + const contactLogsPromise = contactPromise.then((c) => + c.Contact_ID ? getContactLogsByContactId(c.Contact_ID) : [] + ); + return (
- + +
+
+

Loading contact details...

+
+
+ } + > + +
); } diff --git a/src/components/contact-lookup-details/contact-lookup-details.tsx b/src/components/contact-lookup-details/contact-lookup-details.tsx index 3fa14658..6dfd9995 100644 --- a/src/components/contact-lookup-details/contact-lookup-details.tsx +++ b/src/components/contact-lookup-details/contact-lookup-details.tsx @@ -1,54 +1,26 @@ "use client"; -import React, { useState, useEffect } from "react"; +import React, { use } from "react"; import Image from "next/image"; -import { getContactDetails, getContactLogsByContactId } from "./actions"; -import { ContactLookupDetails as ContactLookupDetailsType, ContactLogDisplay } from "@/lib/dto"; +import { useRouter } from "next/navigation"; +import { + ContactLookupDetails as ContactLookupDetailsType, + ContactLogDisplay, +} from "@/lib/dto"; import { ContactLogs } from "@/components/contact-logs"; interface ContactLookupDetailsProps { - guid: string; + contactPromise: Promise; + contactLogsPromise: Promise; } export const ContactLookupDetails: React.FC = ({ - guid, + contactPromise, + contactLogsPromise, }) => { - const [contact, setContact] = useState(null); - const [contactLogs, setContactLogs] = useState([]); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); - - const fetchContactDetails = async () => { - try { - setLoading(true); - setError(null); - - const contactDetails = await getContactDetails(guid); - setContact(contactDetails); - - // Fetch contact logs - if (contactDetails.Contact_ID) { - const logs = await getContactLogsByContactId(contactDetails.Contact_ID); - setContactLogs(logs); - } - } catch (err) { - console.error("Error loading contact details:", err); - const errorMessage = - err instanceof Error - ? err.message - : "An error occurred while loading contact details"; - setError(errorMessage); - } finally { - setLoading(false); - } - }; - - useEffect(() => { - if (guid) { - fetchContactDetails(); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [guid]); + const contact = use(contactPromise); + const contactLogs = use(contactLogsPromise); + const router = useRouter(); const getDisplayName = (firstName?: string, nickname?: string) => { return nickname && nickname.trim() ? nickname : firstName; @@ -69,32 +41,6 @@ export const ContactLookupDetails: React.FC = ({ return `${process.env.NEXT_PUBLIC_MINISTRY_PLATFORM_FILE_URL}/${imageGuid}?$thumbnail=true`; }; - if (loading) { - return ( -
-
-
-

Loading contact details...

-
-
- ); - } - - if (error) { - return ( -
-
-
-

Error

-
-

{error}

-
-
-
-
- ); - } - if (!contact) { return (
@@ -214,13 +160,13 @@ export const ContactLookupDetails: React.FC = ({
- + router.refresh()} /> ); From 539aae374fe7d6184cb9588532e43ce7456c7c2b Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Sat, 16 May 2026 16:03:09 -0400 Subject: [PATCH 03/14] refactor(contact-logs): scope re-renders to log-type field with useWatch Replace useForm's watch() with a scoped useWatch on the contactLogType field so only the Select that consumes it re-renders, instead of the whole component re-rendering on every form value change. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/components/contact-logs/contact-logs.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/contact-logs/contact-logs.tsx b/src/components/contact-logs/contact-logs.tsx index 8ab840b7..10deed69 100644 --- a/src/components/contact-logs/contact-logs.tsx +++ b/src/components/contact-logs/contact-logs.tsx @@ -34,7 +34,7 @@ import { AlertDialogTitle, } from "@/components/ui/alert-dialog"; import { Plus, Save, Trash2 } from "lucide-react"; -import { useForm } from "react-hook-form"; +import { useForm, useWatch } from "react-hook-form"; import { zodResolver } from "@hookform/resolvers/zod"; import { z } from "zod"; @@ -104,7 +104,7 @@ export function ContactLogs({ formState: { errors }, reset, setValue, - watch, + control, } = useForm({ resolver: zodResolver(ContactLogFormSchema), defaultValues: { @@ -113,6 +113,8 @@ export function ContactLogs({ }, }); + const contactLogTypeValue = useWatch({ control, name: "contactLogType" }); + const onCreateLog = async (data: ContactLogFormData) => { try { setIsCreating(true); @@ -288,7 +290,7 @@ export function ContactLogs({