diff --git a/src/features/flyer/FlyerUploader.test.tsx b/src/features/flyer/FlyerUploader.test.tsx index 7c2e559e..4e8e8c59 100644 --- a/src/features/flyer/FlyerUploader.test.tsx +++ b/src/features/flyer/FlyerUploader.test.tsx @@ -88,12 +88,11 @@ describe('FlyerUploader', () => { console.log('--- [TEST LOG] ---: 4. File change event fired. Now AWAITING UI update.'); try { + console.log('--- [TEST LOG] ---: 5a. Awaiting screen.findByText("Checking...")'); await screen.findByText('Checking...'); - console.log('--- [TEST LOG] ---: 5. SUCCESS: UI updated to polling state.'); - console.log('--- [DEBUG] ---: DOM after findByText success:'); - screen.debug(); + console.log('--- [TEST LOG] ---: 5b. SUCCESS: UI updated to polling state.'); } catch (error) { - console.error('--- [TEST LOG] ---: 5. ERROR: findByText("Checking...") timed out.'); + console.error('--- [TEST LOG] ---: 5c. ERROR: findByText("Checking...") timed out.'); console.log('--- [DEBUG] ---: DOM at time of failure:'); screen.debug(); // Print the DOM when the error occurs throw error; // Re-throw the error to fail the test @@ -102,17 +101,19 @@ describe('FlyerUploader', () => { console.log('--- [TEST LOG] ---: 6. Asserting mock calls after UI update.'); expect(mockedAiApiClient.uploadAndProcessFlyer).toHaveBeenCalledWith(file, 'mock-checksum'); expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(1); - console.log('--- [TEST LOG] ---: 7. Verified upload and initial poll calls. Now AWAITING timer advancement.'); + console.log('--- [TEST LOG] ---: 7. Mocks verified. Advancing timers now...'); await act(async () => { - console.log('--- [TEST LOG] ---: 8. Advancing timers by 3000ms...'); + console.log('--- [TEST LOG] ---: 8a. vi.advanceTimersByTime(3000) starting...'); vi.advanceTimersByTime(3000); + console.log('--- [TEST LOG] ---: 8b. vi.advanceTimersByTime(3000) complete.'); }); - console.log('--- [TEST LOG] ---: 9. Timers advanced. Now AWAITING second poll.'); + console.log(`--- [TEST LOG] ---: 9. Act block finished. Now checking if getJobStatus was called again.`); try { await waitFor(() => { - console.log('--- [TEST LOG] ---: 10. waitFor check: checking for 2 getJobStatus calls...'); + const calls = mockedAiApiClient.getJobStatus.mock.calls.length; + console.log(`--- [TEST LOG] ---: 10. waitFor check: getJobStatus calls = ${calls}`); expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(2); }); console.log('--- [TEST LOG] ---: 11. SUCCESS: Second poll confirmed.'); @@ -160,8 +161,9 @@ describe('FlyerUploader', () => { console.log(`--- [TEST LOG] ---: 7. Timers advanced. Now AWAITING completion message.`); try { + console.log('--- [TEST LOG] ---: 8a. waitFor check: Waiting for completion text and job status count.'); await waitFor(() => { - console.log('--- [TEST LOG] ---: 8. waitFor check: checking for completion message...'); + console.log(`--- [TEST LOG] ---: 8b. waitFor interval: calls=${mockedAiApiClient.getJobStatus.mock.calls.length}`); expect(screen.getByText('Processing complete! Redirecting to flyer 42...')).toBeInTheDocument(); }); console.log('--- [TEST LOG] ---: 9. SUCCESS: Completion message found.'); @@ -285,14 +287,14 @@ describe('FlyerUploader', () => { try { console.log('--- [TEST LOG] ---: 8. AWAITING UI reset to idle state...'); expect(await screen.findByText(/click to select a file/i)).toBeInTheDocument(); - console.log('--- [TEST LOG] ---: 9. SUCCESS: UI has reset.'); + // Fix typo: queryText -> queryByText + expect(screen.queryByText('Analyzing...')).not.toBeInTheDocument(); + console.log('--- [TEST LOG] ---: 9. SUCCESS: UI has reset and message removed.'); } catch(error) { console.error('--- [TEST LOG] ---: 9. ERROR: findByText for idle state timed out.'); console.log('--- [DEBUG] ---: DOM at time of failure:'); screen.debug(); throw error; } - - expect(screen.queryByText('Analyzing...')).not.toBeInTheDocument(); }); }); \ No newline at end of file diff --git a/src/features/flyer/FlyerUploader.tsx b/src/features/flyer/FlyerUploader.tsx index 580fec27..9fee926a 100644 --- a/src/features/flyer/FlyerUploader.tsx +++ b/src/features/flyer/FlyerUploader.tsx @@ -200,11 +200,17 @@ export const FlyerUploader: React.FC = ({ onProcessingComple const borderColor = isDragging ? 'border-brand-primary' : 'border-gray-400 dark:border-gray-600'; const bgColor = isDragging ? 'bg-brand-light/50 dark:bg-brand-dark/20' : 'bg-gray-50/50 dark:bg-gray-800/20'; + // If processing, show the detailed status component. Otherwise, show the uploader. + console.debug(`[DEBUG] FlyerUploader: Rendering. State=${processingState}, Msg=${statusMessage}, Err=${!!errorMessage}`); + if (isProcessing || processingState === 'completed' || processingState === 'error') { return (
+ {/* Display the current status message to the user and the test runner */} + {statusMessage &&

{statusMessage}

} + {errorMessage && (

{errorMessage}

diff --git a/src/features/voice-assistant/VoiceAssistant.test.tsx b/src/features/voice-assistant/VoiceAssistant.test.tsx index ce552b2d..15aceab3 100644 --- a/src/features/voice-assistant/VoiceAssistant.test.tsx +++ b/src/features/voice-assistant/VoiceAssistant.test.tsx @@ -1,7 +1,7 @@ // src/features/voice-assistant/VoiceAssistant.test.tsx import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, Mock } from 'vitest'; import { VoiceAssistant } from './VoiceAssistant'; import * as aiApiClient from '../../services/aiApiClient'; @@ -50,6 +50,12 @@ describe('VoiceAssistant Component', () => { beforeEach(() => { vi.clearAllMocks(); + // FIX: The component's `startSession` function awaits `getUserMedia`. + // We must provide a mock resolved value for the promise to prevent the test + // from hanging or erroring, and to allow the async function to proceed. + (navigator.mediaDevices.getUserMedia as Mock).mockResolvedValue({ + getTracks: () => [{ stop: vi.fn() }], + }); }); it('should not render when isOpen is false', () => { @@ -85,11 +91,18 @@ describe('VoiceAssistant Component', () => { expect(mockOnClose).not.toHaveBeenCalled(); }); - it('should call startVoiceSession when the microphone button is clicked in idle state', () => { + // FIX: The test must be async to handle the async `startSession` function. + it('should call startVoiceSession when the microphone button is clicked in idle state', async () => { render(); const micButton = screen.getByRole('button', { name: /start voice session/i }); fireEvent.click(micButton); - expect(aiApiClient.startVoiceSession).toHaveBeenCalledTimes(1); + + // FIX: Because `startSession` is async (it awaits `getUserMedia`), the call + // to `startVoiceSession` within it does not happen immediately. We must + // wait for the asynchronous operations to complete before asserting the result. + await vi.waitFor(() => { + expect(aiApiClient.startVoiceSession).toHaveBeenCalledTimes(1); + }); }); it('should display history and current transcripts', () => { diff --git a/src/hooks/useAiAnalysis.test.ts b/src/hooks/useAiAnalysis.test.ts index 788b00a5..b9d67e88 100644 --- a/src/hooks/useAiAnalysis.test.ts +++ b/src/hooks/useAiAnalysis.test.ts @@ -5,6 +5,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { useAiAnalysis } from './useAiAnalysis'; import { useApi } from './useApi'; import { AnalysisType } from '../types'; +import { logger } from '../services/logger.client'; import type { Flyer, FlyerItem, MasterGroceryItem } from '../types'; // Removed ApiProvider import import { ApiProvider } from '../providers/ApiProvider'; // Updated import path for ApiProvider @@ -14,6 +15,7 @@ vi.mock('../services/logger.client', () => ({ logger: { info: vi.fn(), error: vi.fn(), + warn: vi.fn(), }, })); @@ -53,6 +55,7 @@ describe('useAiAnalysis Hook', () => { }; beforeEach(() => { + logger.info('--- NEW TEST RUN ---'); vi.clearAllMocks(); // Set a default return value for any call to useApi. @@ -90,8 +93,10 @@ describe('useAiAnalysis Hook', () => { }); it('should initialize with correct default states', () => { + logger.info('TEST: should initialize with correct default states'); const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Asserting initial state...'); expect(result.current.results).toEqual({}); expect(result.current.sources).toEqual({}); expect(result.current.loadingStates).toEqual({ @@ -104,54 +109,68 @@ describe('useAiAnalysis Hook', () => { expect(result.current.error).toBeNull(); expect(result.current.generatedImageUrl).toBeNull(); expect(result.current.isGeneratingImage).toBe(false); + logger.info('Initial state assertions passed.'); }); describe('runAnalysis', () => { it('should call the correct execute function for QUICK_INSIGHTS', async () => { + logger.info('TEST: should call execute for QUICK_INSIGHTS'); mockGetQuickInsights.execute.mockResolvedValue('Quick insights text'); const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Act: Running analysis for QUICK_INSIGHTS...'); await act(async () => { await result.current.runAnalysis(AnalysisType.QUICK_INSIGHTS); }); + logger.info('Assert: Checking if getQuickInsights.execute was called correctly.'); expect(mockGetQuickInsights.execute).toHaveBeenCalledWith(mockFlyerItems); }); it('should update results when quickInsightsData changes', () => { + logger.info('TEST: should update results when quickInsightsData changes'); const { result, rerender } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Arrange: Simulating useApi returning new data for QUICK_INSIGHTS.'); // Simulate useApi returning new data by re-rendering with a new mock value mockedUseApi.mockReset() .mockReturnValueOnce({ ...mockGetQuickInsights, data: 'New insights', reset: vi.fn() }) .mockReturnValue(mockGetDeepDive); // provide defaults for others + logger.info('Act: Re-rendering hook to simulate data update.'); rerender(); + logger.info('Assert: Checking if results state was updated.'); expect(result.current.results[AnalysisType.QUICK_INSIGHTS]).toBe('New insights'); }); it('should call the correct execute function for DEEP_DIVE', async () => { + logger.info('TEST: should call execute for DEEP_DIVE'); mockGetDeepDive.execute.mockResolvedValue('Deep dive text'); const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Act: Running analysis for DEEP_DIVE...'); await act(async () => { await result.current.runAnalysis(AnalysisType.DEEP_DIVE); }); + logger.info('Assert: Checking if getDeepDive.execute was called correctly.'); expect(mockGetDeepDive.execute).toHaveBeenCalledWith(mockFlyerItems); }); it('should update results and sources when webSearchData changes', () => { + logger.info('TEST: should update results and sources for WEB_SEARCH'); const mockResponse = { text: 'Web search text', sources: [{ web: { uri: 'http://a.com', title: 'Source A' } }] }; const { result, rerender } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Arrange: Simulating useApi returning new data for WEB_SEARCH.'); // Prepare mocks for the re-render. We must provide the full sequence of 6 calls. mockedUseApi.mockReset(); // Set default fallback first mockedUseApi.mockReturnValue({ execute: vi.fn(), data: null, loading: false, error: null, isRefetching: false, reset: vi.fn() }); + logger.info('Arrange: Setting up specific mock sequence for rerender.'); // Override specific sequence for re-render mockedUseApi .mockReturnValueOnce(mockGetQuickInsights) @@ -161,43 +180,54 @@ describe('useAiAnalysis Hook', () => { .mockReturnValueOnce(mockComparePrices) .mockReturnValueOnce(mockGenerateImage); + logger.info('Act: Re-rendering hook to simulate data update.'); rerender(); + logger.info('Assert: Checking if results and sources state were updated for WEB_SEARCH.'); expect(result.current.results[AnalysisType.WEB_SEARCH]).toBe('Web search text'); expect(result.current.sources[AnalysisType.WEB_SEARCH]).toEqual([{ uri: 'http://a.com', title: 'Source A' }]); }); it('should call the correct execute function for COMPARE_PRICES', async () => { + logger.info('TEST: should call execute for COMPARE_PRICES'); mockComparePrices.execute.mockResolvedValue({ text: 'Price comparison text', sources: [] }); // This was a duplicate, fixed. const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Act: Running analysis for COMPARE_PRICES...'); await act(async () => { await result.current.runAnalysis(AnalysisType.COMPARE_PRICES); }); + logger.info('Assert: Checking if comparePrices.execute was called correctly.'); expect(mockComparePrices.execute).toHaveBeenCalledWith(mockWatchedItems); }); it('should call the correct execute function for PLAN_TRIP with geolocation', async () => { + logger.info('TEST: should call execute for PLAN_TRIP with geolocation'); mockPlanTrip.execute.mockResolvedValue({ text: 'Trip plan text', sources: [{ uri: 'http://maps.com', title: 'Map' }] }); const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Act: Running analysis for PLAN_TRIP...'); await act(async () => { await result.current.runAnalysis(AnalysisType.PLAN_TRIP); }); + logger.info('Assert: Checking if geolocation and planTrip.execute were called correctly.'); expect(navigator.geolocation.getCurrentPosition).toHaveBeenCalled(); expect(mockPlanTrip.execute).toHaveBeenCalledWith( mockFlyerItems, mockSelectedFlyer.store, { latitude: 50, longitude: 50 } ); + logger.info('PLAN_TRIP assertions passed.'); }); it('should derive a generic error message if an API call fails', () => { + logger.info('TEST: should derive a generic error message on API failure'); const apiError = new Error('API is down'); // Simulate useApi returning an error + logger.info('Arrange: Simulating useApi returning an error for QUICK_INSIGHTS.'); // Reset and provide full sequence mockedUseApi.mockReset(); mockedUseApi.mockReturnValue({ execute: vi.fn(), data: null, loading: false, error: null, isRefetching: false, reset: vi.fn() }); @@ -212,26 +242,33 @@ describe('useAiAnalysis Hook', () => { const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Assert: Checking if the error state is correctly populated.'); expect(result.current.error).toBe('API is down'); + logger.info('Error state assertion passed.'); }); it('should log an error for geolocation permission denial', async () => { + logger.info('TEST: should handle geolocation permission denial'); const geoError = new GeolocationPositionError(); Object.defineProperty(geoError, 'code', { value: GeolocationPositionError.PERMISSION_DENIED }); + logger.info('Arrange: Mocking navigator.geolocation.getCurrentPosition to call the error callback.'); vi.mocked(navigator.geolocation.getCurrentPosition).mockImplementation((success, error) => { if (error) error(geoError); }); + logger.info('Arrange: Mocking planTrip.execute to reject, simulating a failure caught by useApi.'); // The execute function will reject, and useApi will set the error state const rejectionError = new Error("Geolocation permission denied."); mockPlanTrip.execute.mockRejectedValue(rejectionError); const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Act: Running analysis for PLAN_TRIP, which is expected to fail.'); await act(async () => { await result.current.runAnalysis(AnalysisType.PLAN_TRIP); }); + logger.info('Assert: Checking if the internal error state reflects the geolocation failure.'); // The test now verifies that the error from the failed execute call is propagated. // The specific user-friendly message is now part of the component that consumes the hook. expect(result.current.error).toBe(rejectionError.message); @@ -240,47 +277,24 @@ describe('useAiAnalysis Hook', () => { describe('generateImage', () => { it('should not run if there are no DEEP_DIVE results', async () => { + logger.info('TEST: should not run generateImage if DEEP_DIVE results are missing'); const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + logger.info('Act: Calling generateImage while results are empty.'); await act(async () => { await result.current.generateImage(); }); + logger.info('Assert: Checking that the logger was warned and the API was not called.'); expect(mockGenerateImage.execute).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith('generateImage called but no meal plan text available in results'); }); it('should call the API and set the image URL on success', async () => { - const { result } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); - - // First, simulate having a deep dive result - act(() => { - // This state is internal, so we test the effect by calling the function - result.current.results[AnalysisType.DEEP_DIVE] = 'A great meal plan'; - }); - - mockGenerateImage.execute.mockResolvedValue('base64string'); - - await act(async () => { - await result.current.generateImage(); - }); - - expect(mockGenerateImage.execute).toHaveBeenCalledWith('A great meal plan'); - }); - - it('should set an error if image generation fails', async () => { + logger.info('TEST: should call generateImage API and update URL on success'); const { result, rerender } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); - // Initial state setup (simulated via internal mutation for test speed, - // or by re-mocking UseApi to return result, but that requires a rerender) - // Since we want to test generateImage which reads from state, we must ensure state is set. - // The best way is to simulate a successful deep dive first. - - // Let's just bypass the state check by ensuring results is populated or mocking results differently? - // No, we can just mutate the result.current.results for testing purposes - // IF the hook doesn't memoize generateImage based on it deeply. - // generateImage depends on [results]. - - // Better: Re-render with deep dive data present. + logger.info('Step 1 (Arrange): Simulating DEEP_DIVE results being present via rerender.'); mockedUseApi.mockReset(); mockedUseApi.mockReturnValue({ execute: vi.fn(), data: null, loading: false, error: null, isRefetching: false, reset: vi.fn() }); mockedUseApi @@ -290,31 +304,55 @@ describe('useAiAnalysis Hook', () => { .mockReturnValueOnce(mockPlanTrip) .mockReturnValueOnce(mockComparePrices) .mockReturnValueOnce(mockGenerateImage); - rerender(); - // Now set up the failure for the next render/call - const apiError = new Error('Image model failed'); - // We mock the execute function to reject. - // AND we need the hook to re-render with the error state *after* the rejection. - // But useApi internal state updates trigger the re-render. - - // Ideally, we test that the hook *reacts* to useApi returning an error. - // But here we are testing the *action* calling the API. - - // Let's just mock the execute to reject, and assume the hook handles the rejection (caught). - // Then re-mock useApi to show the error state, simulate a re-render, and check error. - - // For this test, we just want to ensure it tries to call it and handles error. - // Actually, checking result.current.error relies on useApi returning the error. - - // Step 1: Execute fails - mockGenerateImage.execute.mockRejectedValue(apiError); + logger.info('Step 2 (Act): Calling `generateImage`.'); await act(async () => { await result.current.generateImage(); }); - - // Step 2: Simulate useApi update (re-render with error state) + + logger.info('Step 3 (Assert): Verifying the image generation API was called.'); + expect(mockGenerateImage.execute).toHaveBeenCalledWith('A great meal plan'); + + logger.info('Step 4 (Arrange): Simulating `useApi` for image generation returning a successful result via rerender.'); + mockedUseApi.mockReset(); + mockedUseApi + .mockReturnValueOnce(mockGetQuickInsights) + .mockReturnValueOnce({ ...mockGetDeepDive, data: 'A great meal plan' }) + .mockReturnValueOnce(mockSearchWeb) + .mockReturnValueOnce(mockPlanTrip) + .mockReturnValueOnce(mockComparePrices) + .mockReturnValueOnce({ ...mockGenerateImage, data: 'base64string' }); // Now has data + rerender(); + + logger.info('Step 5 (Assert): Checking for correctly formatted image URL.'); + expect(result.current.generatedImageUrl).toBe('data:image/png;base64,base64string'); + logger.info('Image URL assertion passed.'); + }); + + it('should set an error if image generation fails', async () => { + logger.info('TEST: should set an error if image generation fails'); + const { result, rerender } = renderHook(() => useAiAnalysis(defaultParams), { wrapper }); + + logger.info('Step 1 (Arrange): Re-render with deep dive data present so we can call generateImage.'); + mockedUseApi.mockReset(); + mockedUseApi.mockReturnValue({ execute: vi.fn(), data: null, loading: false, error: null, isRefetching: false, reset: vi.fn() }); + mockedUseApi + .mockReturnValueOnce(mockGetQuickInsights) + .mockReturnValueOnce({ ...mockGetDeepDive, data: 'A great meal plan' }) // Deep dive data + .mockReturnValueOnce(mockSearchWeb) + .mockReturnValueOnce(mockPlanTrip) + .mockReturnValueOnce(mockComparePrices) + .mockReturnValueOnce(mockGenerateImage); + rerender(); + + logger.info('Step 2 (Act): Call generateImage. The execute function is called.'); + await act(async () => { + await result.current.generateImage(); + }); + + logger.info('Step 3 (Arrange): Simulate the useApi hook re-rendering our component with an error state.'); + const apiError = new Error('Image model failed'); mockedUseApi.mockReset(); mockedUseApi.mockReturnValue({ execute: vi.fn(), data: null, loading: false, error: null, isRefetching: false, reset: vi.fn() }); mockedUseApi @@ -323,11 +361,12 @@ describe('useAiAnalysis Hook', () => { .mockReturnValueOnce(mockSearchWeb) .mockReturnValueOnce(mockPlanTrip) .mockReturnValueOnce(mockComparePrices) - .mockReturnValueOnce({ ...mockGenerateImage, error: apiError }); // Image gen has error - + .mockReturnValueOnce({ ...mockGenerateImage, error: apiError, reset: vi.fn() }); // Image gen now has an error rerender(); + logger.info("Step 4 (Assert): The error from the useApi hook is now exposed as the hook's primary error state."); expect(result.current.error).toBe('Image model failed'); + logger.info('Error state assertion passed.'); }); }); }); \ No newline at end of file diff --git a/src/hooks/useAiAnalysis.ts b/src/hooks/useAiAnalysis.ts index 1be0aeb4..01bd15fe 100644 --- a/src/hooks/useAiAnalysis.ts +++ b/src/hooks/useAiAnalysis.ts @@ -132,21 +132,12 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi logger.warn('generateImage called but no meal plan text available in results'); return; } + // Clear any previous internal errors. The useApi hook will manage the loading/error state + // for this specific API call, and its state is already wired up to this hook's `error` value. setInternalError(null); - try { - await generateImageApi(mealPlanText); - } catch (e) { - logger.error('generateImage failed', { error: e }); - let message = 'An unexpected error occurred'; - if (e instanceof Error) { - message = e.message; - } else if (typeof e === 'object' && e !== null && 'message' in e) { - message = String((e as { message: unknown }).message); - } else if (typeof e === 'string') { - message = e; - } - setInternalError(message); - } + // No try/catch is needed here because the useApi hook handles promise rejections + // and exposes the error through its `error` return value. + await generateImageApi(mealPlanText); }, [results, generateImageApi]); return { diff --git a/src/pages/admin/components/AdminBrandManager.tsx b/src/pages/admin/components/AdminBrandManager.tsx index 9b74387f..a0891aaa 100644 --- a/src/pages/admin/components/AdminBrandManager.tsx +++ b/src/pages/admin/components/AdminBrandManager.tsx @@ -7,12 +7,20 @@ import { ErrorDisplay } from '../../../components/ErrorDisplay'; import { useApiOnMount } from '../../../hooks/useApiOnMount'; export const AdminBrandManager: React.FC = () => { - // Use the new hook to fetch brands when the component mounts. - // It handles loading and error states for us. We create a wrapper function - // to ensure the AbortSignal from the hook is correctly passed to the API client. - // The underlying `fetchAllBrands` function takes no arguments, so we create a wrapper - // that accepts the signal from the hook but doesn't pass it down. - const fetchBrandsWrapper = () => fetchAllBrands(); + // The wrapper function now correctly handles the async nature of the API call. + // It awaits the response and parses the JSON body, ensuring that the + // `useApiOnMount` hook receives the actual data (Brand[]) rather than the raw Response object. + const fetchBrandsWrapper = async () => { + const response = await fetchAllBrands(); + if (!response.ok) { + // Improve error handling by attempting to read text from the response body. + const errorText = await response.text(); + throw new Error(errorText || `Request failed with status ${response.status}`); + } + // Return the parsed JSON so the hook gets the correct data type. + return response.json(); + }; + const { data: initialBrands, loading, error } = useApiOnMount(fetchBrandsWrapper, []); // We still need local state for brands so we can update it after a logo upload @@ -43,6 +51,13 @@ export const AdminBrandManager: React.FC = () => { try { const response = await uploadBrandLogo(brandId, file); + + // Check for a successful response before attempting to parse JSON. + if (!response.ok) { + const errorBody = await response.text(); + throw new Error(errorBody || `Upload failed with status ${response.status}`); + } + const { logoUrl } = await response.json(); toast.success('Logo updated successfully!', { id: toastId }); diff --git a/src/pages/admin/components/ProfileManager.Authenticated.test.tsx b/src/pages/admin/components/ProfileManager.Authenticated.test.tsx index 52b55513..95bfa02c 100644 --- a/src/pages/admin/components/ProfileManager.Authenticated.test.tsx +++ b/src/pages/admin/components/ProfileManager.Authenticated.test.tsx @@ -1,6 +1,6 @@ // src/pages/admin/components/ProfileManager.Authenticated.test.tsx import React from 'react'; -import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor, cleanup, act } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest'; import { ProfileManager } from './ProfileManager'; import * as apiClient from '../../../services/apiClient'; @@ -195,9 +195,16 @@ describe('ProfileManager Authenticated User Features', () => { fireEvent.change(screen.getByLabelText(/city/i), { target: { value: 'NewCity' } }); console.log('[TEST LOG] City input value after change:', (cityInput as HTMLInputElement).value); - console.log('[TEST LOG] Firing SUBMIT event on the form.'); - // Use fireEvent.submit on the form for more robust testing of submission logic. - fireEvent.submit(screen.getByTestId('profile-form')); + console.log('[TEST LOG] About to enter act() block for form submission.'); + // Wrap the submission in `act` to ensure all state updates from the async hooks + // are processed before moving on. This is crucial for complex components. + await act(async () => { + console.log('[TEST LOG] INSIDE act(): Firing submit event on the form.'); + fireEvent.submit(screen.getByTestId('profile-form')); + // This log confirms the event has been fired within the act block. + console.log('[TEST LOG] INSIDE act(): Submit event fired. Awaiting internal state updates to flush...'); + }); + console.log('[TEST LOG] Exited act() block. All updates should now be flushed.'); console.log('[TEST LOG] Waiting for notifyError to be called...'); // Since only the address changed and it failed, we expect an error notification (handled by useApi) diff --git a/src/pages/admin/components/ProfileManager.tsx b/src/pages/admin/components/ProfileManager.tsx index 6ed370fb..fc00f8e4 100644 --- a/src/pages/admin/components/ProfileManager.tsx +++ b/src/pages/admin/components/ProfileManager.tsx @@ -180,13 +180,16 @@ export const ProfileManager: React.FC = ({ isOpen, onClose, } onClose(); } else { - logger.warn('[handleProfileSave] One or more operations failed. The useApi hook should have shown an error notification.'); + logger.warn('[handleProfileSave] One or more operations failed. The useApi hook should have shown an error notification. The modal will remain open.'); } } catch (error) { // This catch block is a safeguard. In normal operation, the useApi hook // should prevent any promises from rejecting. - logger.error({ err: error }, 'An unexpected error occurred in handleProfileSave:'); + logger.error({ err: error }, '[CRITICAL] An unexpected error was caught directly in handleProfileSave\'s catch block.'); } + + // This log confirms the function has completed its execution. + logger.debug('[handleProfileSave] Save process finished.'); }; const handleAddressChange = (field: keyof Address, value: string) => {