From bdab6b6e331394b5e86bfce133777d5b6ca671bb Mon Sep 17 00:00:00 2001 From: aaldebs99 Date: Thu, 19 Jun 2025 20:06:15 +0000 Subject: [PATCH] fix(frontend): labels handling --- frontend/src/pages/LabelsPage.tsx | 40 +++++-- .../src/pages/__tests__/LabelsPage.test.tsx | 106 +++++++++++++++++- 2 files changed, 133 insertions(+), 13 deletions(-) diff --git a/frontend/src/pages/LabelsPage.tsx b/frontend/src/pages/LabelsPage.tsx index 2105a89..b583cf2 100644 --- a/frontend/src/pages/LabelsPage.tsx +++ b/frontend/src/pages/LabelsPage.tsx @@ -51,12 +51,32 @@ const LabelsPage: React.FC = () => { const fetchLabels = async () => { try { setLoading(true); - const response = await api.get('/api/labels?include_counts=true'); - setLabels(response.data); - setError(null); - } catch (error) { + const response = await api.get('/labels?include_counts=true'); + + // Validate response status and data format + if (response.status === 200 && Array.isArray(response.data)) { + setLabels(response.data); + setError(null); + } else { + console.error('Invalid response - Status:', response.status, 'Data:', response.data); + setError(`Server returned unexpected response (${response.status})`); + setLabels([]); // Reset to empty array to prevent filter errors + } + } catch (error: any) { console.error('Failed to fetch labels:', error); - setError('Failed to load labels'); + + // Handle different types of errors more specifically + if (error?.response?.status === 401) { + setError('Authentication required. Please log in again.'); + } else if (error?.response?.status === 403) { + setError('Access denied. You do not have permission to view labels.'); + } else if (error?.response?.status >= 500) { + setError('Server error. Please try again later.'); + } else { + setError('Failed to load labels. Please check your connection.'); + } + + setLabels([]); // Reset to empty array to prevent filter errors } finally { setLoading(false); } @@ -67,12 +87,12 @@ const LabelsPage: React.FC = () => { }, []); // Filter labels based on search and system label preference - const filteredLabels = labels.filter(label => { + const filteredLabels = Array.isArray(labels) ? labels.filter(label => { const matchesSearch = label.name.toLowerCase().includes(searchTerm.toLowerCase()) || (label.description || '').toLowerCase().includes(searchTerm.toLowerCase()); const matchesFilter = showSystemLabels || !label.is_system; return matchesSearch && matchesFilter; - }); + }) : []; // Group labels const systemLabels = filteredLabels.filter(label => label.is_system); @@ -80,7 +100,7 @@ const LabelsPage: React.FC = () => { const handleCreateLabel = async (labelData: Omit) => { try { - const response = await api.post('/api/labels', labelData); + const response = await api.post('/labels', labelData); await fetchLabels(); // Refresh the list } catch (error) { console.error('Failed to create label:', error); @@ -92,7 +112,7 @@ const LabelsPage: React.FC = () => { if (!editingLabel) return; try { - await api.put(`/api/labels/${editingLabel.id}`, labelData); + await api.put(`/labels/${editingLabel.id}`, labelData); await fetchLabels(); // Refresh the list setEditingLabel(null); } catch (error) { @@ -103,7 +123,7 @@ const LabelsPage: React.FC = () => { const handleDeleteLabel = async (labelId: string) => { try { - await api.delete(`/api/labels/${labelId}`); + await api.delete(`/labels/${labelId}`); await fetchLabels(); // Refresh the list setDeleteDialogOpen(false); setLabelToDelete(null); diff --git a/frontend/src/pages/__tests__/LabelsPage.test.tsx b/frontend/src/pages/__tests__/LabelsPage.test.tsx index c3d7459..cc49993 100644 --- a/frontend/src/pages/__tests__/LabelsPage.test.tsx +++ b/frontend/src/pages/__tests__/LabelsPage.test.tsx @@ -144,7 +144,7 @@ describe('LabelsPage Component', () => { renderLabelsPage(); await waitFor(() => { - expect(screen.getByText('Failed to load labels')).toBeInTheDocument(); + expect(screen.getByText(/Failed to load labels/)).toBeInTheDocument(); }); }); @@ -154,13 +154,113 @@ describe('LabelsPage Component', () => { renderLabelsPage(); await waitFor(() => { - expect(screen.getByText('Failed to load labels')).toBeInTheDocument(); + expect(screen.getByText(/Failed to load labels/)).toBeInTheDocument(); }); const closeButton = screen.getByLabelText('Close'); await user.click(closeButton); - expect(screen.queryByText('Failed to load labels')).not.toBeInTheDocument(); + expect(screen.queryByText(/Failed to load labels/)).not.toBeInTheDocument(); + }); + + test('should handle 401 authentication errors', async () => { + mockApi.get.mockRejectedValue({ + response: { status: 401 }, + message: 'Unauthorized' + }); + + renderLabelsPage(); + + await waitFor(() => { + expect(screen.getByText('Authentication required. Please log in again.')).toBeInTheDocument(); + }); + }); + + test('should handle 403 access denied errors', async () => { + mockApi.get.mockRejectedValue({ + response: { status: 403 }, + message: 'Forbidden' + }); + + renderLabelsPage(); + + await waitFor(() => { + expect(screen.getByText('Access denied. You do not have permission to view labels.')).toBeInTheDocument(); + }); + }); + + test('should handle 500 server errors', async () => { + mockApi.get.mockRejectedValue({ + response: { status: 500 }, + message: 'Internal Server Error' + }); + + renderLabelsPage(); + + await waitFor(() => { + expect(screen.getByText('Server error. Please try again later.')).toBeInTheDocument(); + }); + }); + + test('should handle non-array response data', async () => { + // This is the main fix - when API returns non-array data + mockApi.get.mockResolvedValue({ + status: 200, + data: { error: 'Something went wrong' } // Not an array! + }); + + renderLabelsPage(); + + await waitFor(() => { + expect(screen.getByText('Received invalid data format from server')).toBeInTheDocument(); + }); + + // Should not crash - labels should be empty array + expect(screen.getByText('No labels found')).toBeInTheDocument(); + }); + + test('should handle unexpected response status with valid array', async () => { + mockApi.get.mockResolvedValue({ + status: 202, // Unexpected status + data: mockLabels + }); + + renderLabelsPage(); + + await waitFor(() => { + expect(screen.getByText('Server returned unexpected response (202)')).toBeInTheDocument(); + }); + }); + + test('should ensure labels state is always an array', async () => { + // Mock a scenario where data is not an array + mockApi.get.mockResolvedValue({ + status: 200, + data: null + }); + + renderLabelsPage(); + + await waitFor(() => { + expect(screen.getByText('Received invalid data format from server')).toBeInTheDocument(); + }); + + // Should not crash when trying to filter + expect(screen.getByText('No labels found')).toBeInTheDocument(); + }); + + test('should handle string response data', async () => { + // Another scenario where response.data is not an array + mockApi.get.mockResolvedValue({ + status: 200, + data: 'Server maintenance in progress' + }); + + renderLabelsPage(); + + await waitFor(() => { + expect(screen.getByText('Received invalid data format from server')).toBeInTheDocument(); + }); }); });