Skip to content

Commit d25164d

Browse files
authored
Re-factoring API services (#14)
This pull request includes significant refactoring and improvements to the codebase, focusing on updating API endpoints, improving error handling, and enhancing test coverage. The most important changes include the removal of deprecated API calls, the introduction of a new error handling mechanism, and the refactoring of service classes to use updated endpoints. ### API Refactoring: * Removed deprecated API calls in `src/api/index.ts` and updated the corresponding service classes to use new endpoints. (`[[1]](diffhunk://#diff-764761d79ed791ea154187526b088322dba3bebe94f9d38ab22120554aba77c7L1-L28)`, `[[2]](diffhunk://#diff-9dd13b51c673219aa9d6fb02c8bc1100134172df11c9c69008b0b299c0b3d53fR28-R39)`, `[[3]](diffhunk://#diff-9dd13b51c673219aa9d6fb02c8bc1100134172df11c9c69008b0b299c0b3d53fL47-R58)`, `[[4]](diffhunk://#diff-cd77df0f49406951d1fac5878b9dbaedda26efdcfdf299051c8efa5b820c22fcL10-R10)`, `[[5]](diffhunk://#diff-ebf1d939d609a4816004caf5632789818db419201d7af8b12e6123dcfd0b995dL6-R10)`) ### Error Handling: * Introduced a new error handling mechanism using `useCallback` in `ChatContainer.tsx` to centralize error notifications. (`[[1]](diffhunk://#diff-c0bfccaf03a29a5059f6a47c6dd2cdd194fcb1e06fe8e0e188d700a86204f073R27-R36)`, `[[2]](diffhunk://#diff-c0bfccaf03a29a5059f6a47c6dd2cdd194fcb1e06fe8e0e188d700a86204f073L53-R63)`, `[[3]](diffhunk://#diff-c0bfccaf03a29a5059f6a47c6dd2cdd194fcb1e06fe8e0e188d700a86204f073L71-R73)`) ### Test Coverage: * Refactored tests in `ChatContainer.test.tsx` and `ToolsModal.test.tsx` to mock the updated service classes and ensure proper test isolation. (`[[1]](diffhunk://#diff-5bc04b73d213740634c14cc7f3b8c028d08edba97c1c08197b8ed348a3112d47L29-L35)`, `[[2]](diffhunk://#diff-5bc04b73d213740634c14cc7f3b8c028d08edba97c1c08197b8ed348a3112d47L98-L103)`, `[[3]](diffhunk://#diff-5403659b7ee64a58dcfa7026e4f58e57a731ed7d0cd222e7c4391912bd6d10afL1-R79)`, `[[4]](diffhunk://#diff-5403659b7ee64a58dcfa7026e4f58e57a731ed7d0cd222e7c4391912bd6d10afL74-R93)`) ### Component Updates: * Updated `ToolsModal.tsx` and `Sidebar.tsx` components to use the refactored service classes for fetching data. (`[[1]](diffhunk://#diff-63552adfc136b5745280f1413214afb454e46b6558a54e0ac1a62d3076779256L5-R6)`, `[[2]](diffhunk://#diff-63552adfc136b5745280f1413214afb454e46b6558a54e0ac1a62d3076779256L21-R23)`, `[[3]](diffhunk://#diff-63552adfc136b5745280f1413214afb454e46b6558a54e0ac1a62d3076779256L33)`, `[[4]](diffhunk://#diff-5f3a5cc0c274c553bf7e5a260f594f138c16fd5197e1555cb608f862f0120f1cL4-R9)`, `[[5]](diffhunk://#diff-5f3a5cc0c274c553bf7e5a260f594f138c16fd5197e1555cb608f862f0120f1cL36-R38)`) These changes collectively improve the maintainability, reliability, and testability of the codebase.
1 parent de24331 commit d25164d

File tree

9 files changed

+79
-102
lines changed

9 files changed

+79
-102
lines changed

src/api/index.ts

Lines changed: 0 additions & 28 deletions
This file was deleted.

src/components/ChatContainer/ChatContainer.test.tsx

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,6 @@ jest.mock('../../services/ToolService', () => ({
2626
}))
2727
}));
2828

29-
// Mock the api module
30-
jest.mock('../../api', () => ({
31-
fetchChatHistories: jest.fn(),
32-
fetchLLMProviders: jest.fn(),
33-
fetchTools: jest.fn()
34-
}));
35-
3629
// Mock the auth0 hook
3730
jest.mock('@auth0/auth0-react');
3831

@@ -95,12 +88,6 @@ beforeAll(() => {
9588
Element.prototype.scrollIntoView = jest.fn();
9689
});
9790

98-
afterAll(() => {
99-
// Clean up
100-
// @ts-ignore
101-
delete window.import;
102-
});
103-
10491
describe('ChatContainer', () => {
10592
const mockModelSettings = {
10693
temperature: 0.7,

src/components/ChatContainer/ChatContainer.tsx

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, {useEffect, useRef, useState} from 'react';
1+
import React, {useCallback, useEffect, useRef, useState} from 'react';
22
import {useAuth0} from '@auth0/auth0-react';
33
import {ChatContainerProps, ChatPayload, MessageHandlerConfig} from "../../types/chat.ts";
44
import {ApiChatMessage, ChatService, ClientChatMessage} from "../../services/ChatService.ts";
@@ -24,6 +24,16 @@ export const ChatContainer: React.FC<ChatContainerProps> = ({
2424
const [useStreaming, setUseStreaming] = useState(true);
2525
const [availableTools, setAvailableTools] = useState<string[]>([]);
2626

27+
const handleError = useCallback((error: unknown) => {
28+
addNotification(
29+
'error',
30+
error instanceof Error || (typeof error === 'object' && error && 'message' in error)
31+
? (error as { message: string }).message
32+
: 'Failed to send message'
33+
);
34+
}, [addNotification]);
35+
36+
2737
const handleStreamingChange = (value: boolean) => {
2838
setUseStreaming(value);
2939
};
@@ -50,16 +60,7 @@ export const ChatContainer: React.FC<ChatContainerProps> = ({
5060

5161
if (response.error) {
5262
console.error('Error loading tools:', response.error);
53-
// Creating a local function that uses addNotification
54-
const notifyError = () => {
55-
addNotification(
56-
'error',
57-
typeof response.error === 'object' && true && 'message' in response.error
58-
? response.error
59-
: 'Failed to send message'
60-
);
61-
};
62-
notifyError();
63+
handleError(response.error);
6364
return;
6465
}
6566

@@ -68,14 +69,8 @@ export const ChatContainer: React.FC<ChatContainerProps> = ({
6869
}
6970
} catch (error) {
7071
console.error('Error fetching tools:', error);
71-
// Creating another local function
72-
const notifyError = () => {
73-
addNotification(
74-
'error',
75-
error instanceof Error ? error.message : 'Failed to send message'
76-
);
77-
};
78-
notifyError();
72+
handleError(error);
73+
return;
7974
}
8075
};
8176

src/components/ChatInputButton/ToolsModal.test.tsx

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,82 @@
1-
import { render, screen, fireEvent } from '@testing-library/react';
2-
import '@testing-library/jest-dom';
3-
import {fetchTools} from "../../api";
4-
import {act} from "react";
5-
import {ToolsModal} from "./ToolsModal.tsx";
6-
import {Tool} from "../../types/tools.ts";
7-
8-
// Mock the entire api module
9-
jest.mock('../../api', () => ({
10-
fetchTools: jest.fn()
11-
}));
1+
import { render, screen, act, fireEvent } from '@testing-library/react';
2+
import { ToolsModal } from './ToolsModal';
3+
import { ToolService } from '../../services/ToolService';
4+
import { Tool } from '../../types/tools';
5+
6+
// Create mock data
7+
const mockTools = [
8+
{ name: 'Tool1', description: 'Description 1' },
9+
{ name: 'Tool2', description: 'Description 2' },
10+
{ name: 'Tool3', description: 'Description 3' },
11+
];
1212

13-
// Mock the XMarkIcon component
14-
jest.mock('@heroicons/react/24/outline', () => ({
15-
XMarkIcon: () => <div data-testid="close-icon">X</div>
13+
// Mock the ToolItem component
14+
jest.mock('../ToolItem/ToolItem', () => ({
15+
ToolItem: ({ tool, onToggle }: {
16+
tool: Tool;
17+
isSelected: boolean;
18+
onToggle: (name: string) => void;
19+
}) => (
20+
<div
21+
data-testid={`tool-item-${tool.name}`}
22+
onClick={() => onToggle(tool.name)}
23+
>
24+
{tool.name}
25+
</div>
26+
)
1627
}));
1728

1829
// Mock SearchBar component
19-
jest.mock('../SearchBar.tsx', () => ({
20-
SearchBar: ({ value, onChange }: { value: string; onChange: (value: string) => void }) => (
30+
jest.mock('../SearchBar', () => ({
31+
SearchBar: ({ value, onChange }: {
32+
value: string;
33+
onChange: (value: string) => void;
34+
}) => (
2135
<input
2236
data-testid="search-bar"
37+
placeholder="Search tools"
2338
value={value}
2439
onChange={(e) => onChange(e.target.value)}
25-
placeholder="Search tools"
2640
/>
2741
)
2842
}));
2943

30-
// Mock ToolItem component
44+
// Mock the ToolService
45+
jest.mock('../../services/ToolService', () => ({
46+
ToolService: jest.fn().mockImplementation(() => ({
47+
getTools: jest.fn().mockResolvedValue({
48+
data: mockTools
49+
})
50+
}))
51+
}));
52+
53+
// Mock the ToolItem component
3154
jest.mock('../ToolItem/ToolItem', () => ({
3255
ToolItem: ({ tool, isSelected, onToggle }: {
3356
tool: Tool;
3457
isSelected: boolean;
35-
onToggle: (name: string) => void
58+
onToggle: (name: string) => void;
3659
}) => (
3760
<div
3861
data-testid={`tool-item-${tool.name}`}
3962
onClick={() => onToggle(tool.name)}
4063
>
41-
{tool.name} {isSelected ? '(Selected)' : ''}
64+
{tool.name}
65+
{isSelected && ' (Selected)'}
4266
</div>
4367
)
4468
}));
4569

46-
const mockTools = [
47-
{ name: 'Tool1', description: 'Description 1' },
48-
{ name: 'Tool2', description: 'Description 2' },
49-
{ name: 'Tool3', description: 'Description 3' },
50-
];
5170

5271
describe('ToolsModal', () => {
5372
const mockOnClose = jest.fn();
5473
const mockOnSave = jest.fn();
5574

5675
beforeEach(() => {
5776
jest.clearAllMocks();
58-
(fetchTools as jest.Mock).mockResolvedValue(mockTools);
5977
});
6078

79+
6180
it('should render and fetch tools when opened', async () => {
6281
await act(async () => {
6382
render(
@@ -71,7 +90,7 @@ describe('ToolsModal', () => {
7190
});
7291

7392
expect(screen.getByText('Available Tools')).toBeInTheDocument();
74-
expect(fetchTools).toHaveBeenCalledTimes(1);
93+
expect(ToolService).toHaveBeenCalledTimes(1);
7594

7695
expect(screen.getByTestId('tool-item-Tool1')).toBeInTheDocument();
7796
expect(screen.getByTestId('tool-item-Tool2')).toBeInTheDocument();

src/components/ChatInputButton/ToolsModal.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import React, {useEffect, useMemo, useState} from 'react';
22
import {XMarkIcon} from "@heroicons/react/24/outline";
33
import {Tool, ToolsModalProps} from "../../types/tools.ts";
44
import {SearchBar} from "../SearchBar.tsx";
5-
import {fetchTools} from "../../api";
65
import {ToolItem} from "../ToolItem/ToolItem.tsx";
6+
import {ToolService} from "../../services/ToolService.ts";
77

88
export const ToolsModal: React.FC<ToolsModalProps> = ({
99
isOpen,
@@ -18,8 +18,9 @@ export const ToolsModal: React.FC<ToolsModalProps> = ({
1818
useEffect(() => {
1919
const loadTools = async () => {
2020
try {
21-
const data = await fetchTools();
22-
setTools(data);
21+
const toolService = new ToolService();
22+
const response = await toolService.getTools();
23+
setTools(response.data || []);
2324
} catch (error) {
2425
console.error('Error fetching tools:', error);
2526
}
@@ -30,7 +31,6 @@ export const ToolsModal: React.FC<ToolsModalProps> = ({
3031
}
3132
}, [isOpen]);
3233

33-
3434
const handleToolToggle = (toolName: string) => {
3535
setSelectedTools(prev =>
3636
prev.includes(toolName)

src/components/Sidebar.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// components/Sidebar.tsx
22
import React, {useCallback, useEffect, useState} from 'react';
33
import {ChatHistory} from "../types";
4-
import {fetchChatHistories} from "../api";
54
import {SidebarHeader} from "./sidebar/SidebarHeader";
65
import {NewChatSection} from "./sidebar/NewChatSection";
76
import {ChatHistoryList} from "./sidebar/ChatHistoryList";
87
import {SidebarFooter} from "./sidebar/SidebarFooter";
98
import {useAuth0} from '@auth0/auth0-react';
9+
import {ChatService} from "../services/ChatService.ts";
1010

1111

1212
interface SidebarProps {
@@ -33,8 +33,9 @@ export const Sidebar: React.FC<SidebarProps> = ({
3333
setIsLoading(true);
3434
try {
3535
const token = await getAccessTokenSilently();
36-
const response = await fetchChatHistories(token);
37-
setChatHistories(response.chats || []);
36+
const chatService = new ChatService(token);
37+
const response = await chatService.getChatHistories();
38+
setChatHistories(response.data.chats || []);
3839
setError(null);
3940
} catch (err) {
4041
setError('Failed to load chat histories');

src/services/ChatService.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,18 @@ interface StreamChunk {
2525
done?: boolean;
2626
}
2727

28+
interface ChatHistoriesResponse {
29+
chats: ChatHistory[];
30+
}
2831

2932
// Updated ChatService
3033
export class ChatService extends APIClient {
3134
constructor(token: string = '') {
3235
super(import.meta.env.VITE_MCP_BACKEND_API_ENDPOINT, token);
3336
}
3437

35-
async getChatHistories(): Promise<APIResponse<ChatHistory[]>> {
36-
return this.fetchWithError<ChatHistory[]>('/chats');
38+
async getChatHistories(): Promise<APIResponse<ChatHistoriesResponse>> {
39+
return this.fetchWithError<ChatHistoriesResponse>('/api/v1/chats');
3740
}
3841

3942
async sendMessage(payload: ChatPayload): Promise<APIResponse<ChatResponse>> {
@@ -44,15 +47,15 @@ export class ChatService extends APIClient {
4447
}
4548

4649
async loadChatHistory(chatId: string): Promise<APIResponse<{ messages: ApiChatMessage[] }>> {
47-
return this.fetchWithError<{ messages: ApiChatMessage[] }>(`/chats/${chatId}`);
50+
return this.fetchWithError<{ messages: ApiChatMessage[] }>(`/api/v1/chats/${chatId}`);
4851
}
4952

5053
async sendStreamMessage(
5154
payload: ChatPayload,
5255
onChunk: (chunk: StreamChunk) => void
5356
): Promise<void> {
5457
try {
55-
const response = await this.fetchStream('/ask-stream', {
58+
const response = await this.fetchStream('/api/v1/chats/stream', {
5659
method: 'POST',
5760
body: JSON.stringify(payload),
5861
});

src/services/LLMService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ export class LLMService extends APIClient {
77
}
88

99
async getLLMProviders(): Promise<APIResponse<LLMProvidersResponse>> {
10-
return this.fetchWithError<LLMProvidersResponse>('/llm-providers');
10+
return this.fetchWithError<LLMProvidersResponse>('/api/v1/llm-providers');
1111
}
1212
}

src/services/ToolService.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ import {APIClient, APIResponse} from "./APIClient.ts";
33

44
export class ToolService extends APIClient {
55
constructor(token: string = '') {
6-
super('http://localhost:8081/api', token);
6+
super(import.meta.env.VITE_MCP_BACKEND_API_ENDPOINT, token);
77
}
88

99
async getTools(): Promise<APIResponse<Tool[]>> {
10-
return this.fetchWithError<Tool[]>('/tools');
10+
return this.fetchWithError<Tool[]>('/api/v1/tools');
1111
}
12-
}
12+
}

0 commit comments

Comments
 (0)