From 70ff11d501f67b9a70ac03a6aaf3b40f650f63c7 Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Tue, 18 Mar 2025 23:44:08 +0100 Subject: [PATCH 1/7] Refactor chat fetching to use ChatService Replaced direct API calls with ChatService for consistency and centralized API handling. Updated `getChatHistories` to return a typed response and adjusted `Sidebar` to use the new service implementation. Removed unused `fetchLLMProviders` function from the API module. --- src/api/index.ts | 8 -------- src/components/Sidebar.tsx | 7 ++++--- src/services/ChatService.ts | 7 +++++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/api/index.ts b/src/api/index.ts index ee71972..0694cb9 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -11,14 +11,6 @@ export const fetchChatHistories = async (token: string) => { return await response.json(); }; -export const fetchLLMProviders = async () => { - const response = await fetch(`${import.meta.env.VITE_MCP_BACKEND_API_ENDPOINT}/llm-providers`); - if (!response.ok) { - throw new Error('Failed to fetch LLM providers'); - } - return await response.json(); -}; - export const fetchTools = async () => { const response = await fetch('http://localhost:8081/api/tools'); if (!response.ok) { diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index 2424328..ad4570d 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -1,12 +1,12 @@ // components/Sidebar.tsx import React, {useCallback, useEffect, useState} from 'react'; import {ChatHistory} from "../types"; -import {fetchChatHistories} from "../api"; import {SidebarHeader} from "./sidebar/SidebarHeader"; import {NewChatSection} from "./sidebar/NewChatSection"; import {ChatHistoryList} from "./sidebar/ChatHistoryList"; import {SidebarFooter} from "./sidebar/SidebarFooter"; import {useAuth0} from '@auth0/auth0-react'; +import {ChatService} from "../services/ChatService.ts"; interface SidebarProps { @@ -33,8 +33,9 @@ export const Sidebar: React.FC = ({ setIsLoading(true); try { const token = await getAccessTokenSilently(); - const response = await fetchChatHistories(token); - setChatHistories(response.chats || []); + const chatService = new ChatService(token); + const response = await chatService.getChatHistories(); + setChatHistories(response.data.chats || []); setError(null); } catch (err) { setError('Failed to load chat histories'); diff --git a/src/services/ChatService.ts b/src/services/ChatService.ts index eabbb05..19212cc 100644 --- a/src/services/ChatService.ts +++ b/src/services/ChatService.ts @@ -25,6 +25,9 @@ interface StreamChunk { done?: boolean; } +interface ChatHistoriesResponse { + chats: ChatHistory[]; +} // Updated ChatService export class ChatService extends APIClient { @@ -32,8 +35,8 @@ export class ChatService extends APIClient { super(import.meta.env.VITE_MCP_BACKEND_API_ENDPOINT, token); } - async getChatHistories(): Promise> { - return this.fetchWithError('/chats'); + async getChatHistories(): Promise> { + return this.fetchWithError('/chats'); } async sendMessage(payload: ChatPayload): Promise> { From 41fb192d2f303c1ebdbdae14fe446ea7690ee577 Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Wed, 19 Mar 2025 00:00:32 +0100 Subject: [PATCH 2/7] Refactor ToolsModal to use ToolService for fetching tools --- src/api/index.ts | 13 ---- .../ChatInputButton/ToolsModal.test.tsx | 73 ++++++++++++------- src/components/ChatInputButton/ToolsModal.tsx | 8 +- src/services/ToolService.ts | 2 +- 4 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/api/index.ts b/src/api/index.ts index 0694cb9..80c3a50 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -1,16 +1,3 @@ -export const fetchChatHistories = async (token: string) => { - const response = await fetch(`${import.meta.env.VITE_MCP_BACKEND_API_ENDPOINT}/chats`, { - headers: { - 'Authorization': `Bearer ${token}`, - 'Content-Type': 'application/json' - } - }); - if (!response.ok) { - throw new Error('Failed to fetch chat histories'); - } - return await response.json(); -}; - export const fetchTools = async () => { const response = await fetch('http://localhost:8081/api/tools'); if (!response.ok) { diff --git a/src/components/ChatInputButton/ToolsModal.test.tsx b/src/components/ChatInputButton/ToolsModal.test.tsx index c99b129..b9ae5ff 100644 --- a/src/components/ChatInputButton/ToolsModal.test.tsx +++ b/src/components/ChatInputButton/ToolsModal.test.tsx @@ -1,53 +1,72 @@ -import { render, screen, fireEvent } from '@testing-library/react'; -import '@testing-library/jest-dom'; -import {fetchTools} from "../../api"; -import {act} from "react"; -import {ToolsModal} from "./ToolsModal.tsx"; -import {Tool} from "../../types/tools.ts"; - -// Mock the entire api module -jest.mock('../../api', () => ({ - fetchTools: jest.fn() -})); +import { render, screen, act, fireEvent } from '@testing-library/react'; +import { ToolsModal } from './ToolsModal'; +import { ToolService } from '../../services/ToolService'; +import { Tool } from '../../types/tools'; + +// Create mock data +const mockTools = [ + { name: 'Tool1', description: 'Description 1' }, + { name: 'Tool2', description: 'Description 2' }, + { name: 'Tool3', description: 'Description 3' }, +]; -// Mock the XMarkIcon component -jest.mock('@heroicons/react/24/outline', () => ({ - XMarkIcon: () =>
X
+// Mock the ToolItem component +jest.mock('../ToolItem/ToolItem', () => ({ + ToolItem: ({ tool, onToggle }: { + tool: Tool; + isSelected: boolean; + onToggle: (name: string) => void; + }) => ( +
onToggle(tool.name)} + > + {tool.name} +
+ ) })); // Mock SearchBar component -jest.mock('../SearchBar.tsx', () => ({ - SearchBar: ({ value, onChange }: { value: string; onChange: (value: string) => void }) => ( +jest.mock('../SearchBar', () => ({ + SearchBar: ({ value, onChange }: { + value: string; + onChange: (value: string) => void; + }) => ( onChange(e.target.value)} - placeholder="Search tools" /> ) })); -// Mock ToolItem component +// Mock the ToolService +jest.mock('../../services/ToolService', () => ({ + ToolService: jest.fn().mockImplementation(() => ({ + getTools: jest.fn().mockResolvedValue({ + data: mockTools + }) + })) +})); + +// Mock the ToolItem component jest.mock('../ToolItem/ToolItem', () => ({ ToolItem: ({ tool, isSelected, onToggle }: { tool: Tool; isSelected: boolean; - onToggle: (name: string) => void + onToggle: (name: string) => void; }) => (
onToggle(tool.name)} > - {tool.name} {isSelected ? '(Selected)' : ''} + {tool.name} + {isSelected && ' (Selected)'}
) })); -const mockTools = [ - { name: 'Tool1', description: 'Description 1' }, - { name: 'Tool2', description: 'Description 2' }, - { name: 'Tool3', description: 'Description 3' }, -]; describe('ToolsModal', () => { const mockOnClose = jest.fn(); @@ -55,9 +74,9 @@ describe('ToolsModal', () => { beforeEach(() => { jest.clearAllMocks(); - (fetchTools as jest.Mock).mockResolvedValue(mockTools); }); + it('should render and fetch tools when opened', async () => { await act(async () => { render( @@ -71,7 +90,7 @@ describe('ToolsModal', () => { }); expect(screen.getByText('Available Tools')).toBeInTheDocument(); - expect(fetchTools).toHaveBeenCalledTimes(1); + expect(ToolService).toHaveBeenCalledTimes(1); expect(screen.getByTestId('tool-item-Tool1')).toBeInTheDocument(); expect(screen.getByTestId('tool-item-Tool2')).toBeInTheDocument(); diff --git a/src/components/ChatInputButton/ToolsModal.tsx b/src/components/ChatInputButton/ToolsModal.tsx index d745ae5..c8e89e3 100644 --- a/src/components/ChatInputButton/ToolsModal.tsx +++ b/src/components/ChatInputButton/ToolsModal.tsx @@ -2,8 +2,8 @@ import React, {useEffect, useMemo, useState} from 'react'; import {XMarkIcon} from "@heroicons/react/24/outline"; import {Tool, ToolsModalProps} from "../../types/tools.ts"; import {SearchBar} from "../SearchBar.tsx"; -import {fetchTools} from "../../api"; import {ToolItem} from "../ToolItem/ToolItem.tsx"; +import {ToolService} from "../../services/ToolService.ts"; export const ToolsModal: React.FC = ({ isOpen, @@ -18,8 +18,9 @@ export const ToolsModal: React.FC = ({ useEffect(() => { const loadTools = async () => { try { - const data = await fetchTools(); - setTools(data); + const toolService = new ToolService(); + const response = await toolService.getTools(); + setTools(response.data || []); } catch (error) { console.error('Error fetching tools:', error); } @@ -30,7 +31,6 @@ export const ToolsModal: React.FC = ({ } }, [isOpen]); - const handleToolToggle = (toolName: string) => { setSelectedTools(prev => prev.includes(toolName) diff --git a/src/services/ToolService.ts b/src/services/ToolService.ts index 3678857..aa2bbfb 100644 --- a/src/services/ToolService.ts +++ b/src/services/ToolService.ts @@ -9,4 +9,4 @@ export class ToolService extends APIClient { async getTools(): Promise> { return this.fetchWithError('/tools'); } -} \ No newline at end of file +} From 5b78749f60817b1ee143365eb53e4945ac1b7ed1 Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Wed, 19 Mar 2025 00:00:49 +0100 Subject: [PATCH 3/7] Remove unused fetchTools API function The fetchTools function was no longer in use and has been deleted to simplify the codebase. Removing unused code improves maintainability and reduces potential confusion. --- src/api/index.ts | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 src/api/index.ts diff --git a/src/api/index.ts b/src/api/index.ts deleted file mode 100644 index 80c3a50..0000000 --- a/src/api/index.ts +++ /dev/null @@ -1,7 +0,0 @@ -export const fetchTools = async () => { - const response = await fetch('http://localhost:8081/api/tools'); - if (!response.ok) { - throw new Error('Failed to fetch tools'); - } - return await response.json(); -}; \ No newline at end of file From a73e22c4abb85c25d6dc82e06a57e718e10157d8 Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Wed, 19 Mar 2025 00:05:12 +0100 Subject: [PATCH 4/7] Update ChatService API endpoints to use /api/v1 namespace Replaced old chat API endpoints with new ones prefixed by /api/v1 for consistency and to align with the updated API structure. This ensures all requests are routed correctly to the new versioned endpoints. --- src/services/ChatService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/ChatService.ts b/src/services/ChatService.ts index 19212cc..88039b1 100644 --- a/src/services/ChatService.ts +++ b/src/services/ChatService.ts @@ -36,7 +36,7 @@ export class ChatService extends APIClient { } async getChatHistories(): Promise> { - return this.fetchWithError('/chats'); + return this.fetchWithError('/api/v1/chats'); } async sendMessage(payload: ChatPayload): Promise> { @@ -47,7 +47,7 @@ export class ChatService extends APIClient { } async loadChatHistory(chatId: string): Promise> { - return this.fetchWithError<{ messages: ApiChatMessage[] }>(`/chats/${chatId}`); + return this.fetchWithError<{ messages: ApiChatMessage[] }>(`/api/v1/chats/${chatId}`); } async sendStreamMessage( @@ -55,7 +55,7 @@ export class ChatService extends APIClient { onChunk: (chunk: StreamChunk) => void ): Promise { try { - const response = await this.fetchStream('/ask-stream', { + const response = await this.fetchStream('/api/v1/chats/stream', { method: 'POST', body: JSON.stringify(payload), }); From e35704ebf8bff0ef82558c02c5a11f8e5d66c8b4 Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Wed, 19 Mar 2025 00:07:00 +0100 Subject: [PATCH 5/7] Update LLMService and ToolService to use /api/v1 namespace for endpoints --- src/services/LLMService.ts | 2 +- src/services/ToolService.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/LLMService.ts b/src/services/LLMService.ts index 29c25ef..d940830 100644 --- a/src/services/LLMService.ts +++ b/src/services/LLMService.ts @@ -7,6 +7,6 @@ export class LLMService extends APIClient { } async getLLMProviders(): Promise> { - return this.fetchWithError('/llm-providers'); + return this.fetchWithError('/api/v1/llm-providers'); } } \ No newline at end of file diff --git a/src/services/ToolService.ts b/src/services/ToolService.ts index aa2bbfb..d5c719d 100644 --- a/src/services/ToolService.ts +++ b/src/services/ToolService.ts @@ -3,10 +3,10 @@ import {APIClient, APIResponse} from "./APIClient.ts"; export class ToolService extends APIClient { constructor(token: string = '') { - super('http://localhost:8081/api', token); + super('http://localhost:8081', token); } async getTools(): Promise> { - return this.fetchWithError('/tools'); + return this.fetchWithError('/api/v1/tools'); } } From 50610bf41c3d412f988a1e4d0d44c9a1d134997f Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Wed, 19 Mar 2025 00:09:28 +0100 Subject: [PATCH 6/7] Update API endpoint and clean up redundant mocks Replaced hardcoded API URL with an environment variable to improve configurability. Removed unused API mocks from ChatContainer tests to simplify and declutter the test setup. --- src/components/ChatContainer/ChatContainer.test.tsx | 7 ------- src/services/ToolService.ts | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/components/ChatContainer/ChatContainer.test.tsx b/src/components/ChatContainer/ChatContainer.test.tsx index 1d15fe7..9e8a5c4 100644 --- a/src/components/ChatContainer/ChatContainer.test.tsx +++ b/src/components/ChatContainer/ChatContainer.test.tsx @@ -26,13 +26,6 @@ jest.mock('../../services/ToolService', () => ({ })) })); -// Mock the api module -jest.mock('../../api', () => ({ - fetchChatHistories: jest.fn(), - fetchLLMProviders: jest.fn(), - fetchTools: jest.fn() -})); - // Mock the auth0 hook jest.mock('@auth0/auth0-react'); diff --git a/src/services/ToolService.ts b/src/services/ToolService.ts index d5c719d..0f2adee 100644 --- a/src/services/ToolService.ts +++ b/src/services/ToolService.ts @@ -3,7 +3,7 @@ import {APIClient, APIResponse} from "./APIClient.ts"; export class ToolService extends APIClient { constructor(token: string = '') { - super('http://localhost:8081', token); + super(import.meta.env.VITE_MCP_BACKEND_API_ENDPOINT, token); } async getTools(): Promise> { From 4289cc1aaf0c8fe6e700a5e4c1651e58186bbacd Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Wed, 19 Mar 2025 00:18:25 +0100 Subject: [PATCH 7/7] "Refactor error handling with reusable handleError function Replaced duplicated error handling logic with a reusable `handleError` function using `useCallback`. This reduces redundancy and improves maintainability by centralizing error notification logic." --- .../ChatContainer/ChatContainer.test.tsx | 6 ---- .../ChatContainer/ChatContainer.tsx | 33 ++++++++----------- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/components/ChatContainer/ChatContainer.test.tsx b/src/components/ChatContainer/ChatContainer.test.tsx index 9e8a5c4..d2067d5 100644 --- a/src/components/ChatContainer/ChatContainer.test.tsx +++ b/src/components/ChatContainer/ChatContainer.test.tsx @@ -88,12 +88,6 @@ beforeAll(() => { Element.prototype.scrollIntoView = jest.fn(); }); -afterAll(() => { - // Clean up - // @ts-ignore - delete window.import; -}); - describe('ChatContainer', () => { const mockModelSettings = { temperature: 0.7, diff --git a/src/components/ChatContainer/ChatContainer.tsx b/src/components/ChatContainer/ChatContainer.tsx index 0991695..816c0e3 100644 --- a/src/components/ChatContainer/ChatContainer.tsx +++ b/src/components/ChatContainer/ChatContainer.tsx @@ -1,4 +1,4 @@ -import React, {useEffect, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useRef, useState} from 'react'; import {useAuth0} from '@auth0/auth0-react'; import {ChatContainerProps, ChatPayload, MessageHandlerConfig} from "../../types/chat.ts"; import {ApiChatMessage, ChatService, ClientChatMessage} from "../../services/ChatService.ts"; @@ -24,6 +24,16 @@ export const ChatContainer: React.FC = ({ const [useStreaming, setUseStreaming] = useState(true); const [availableTools, setAvailableTools] = useState([]); + const handleError = useCallback((error: unknown) => { + addNotification( + 'error', + error instanceof Error || (typeof error === 'object' && error && 'message' in error) + ? (error as { message: string }).message + : 'Failed to send message' + ); + }, [addNotification]); + + const handleStreamingChange = (value: boolean) => { setUseStreaming(value); }; @@ -50,16 +60,7 @@ export const ChatContainer: React.FC = ({ if (response.error) { console.error('Error loading tools:', response.error); - // Creating a local function that uses addNotification - const notifyError = () => { - addNotification( - 'error', - typeof response.error === 'object' && true && 'message' in response.error - ? response.error - : 'Failed to send message' - ); - }; - notifyError(); + handleError(response.error); return; } @@ -68,14 +69,8 @@ export const ChatContainer: React.FC = ({ } } catch (error) { console.error('Error fetching tools:', error); - // Creating another local function - const notifyError = () => { - addNotification( - 'error', - error instanceof Error ? error.message : 'Failed to send message' - ); - }; - notifyError(); + handleError(error); + return; } };