Skip to content

Commit 4ab7794

Browse files
olaservoolaservo
authored
Merge pull request #512 from richardkmichael/notification-expansion-state
Fix Server Notifications expansion state
2 parents 516fe95 + aa73daa commit 4ab7794

File tree

3 files changed

+242
-4
lines changed

3 files changed

+242
-4
lines changed

‎client/src/App.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ import { z } from "zod";
5252
import "./App.css";
5353
import AuthDebugger from "./components/AuthDebugger";
5454
import ConsoleTab from "./components/ConsoleTab";
55-
import HistoryAndNotifications from "./components/History";
55+
import HistoryAndNotifications from "./components/HistoryAndNotifications";
5656
import PingTab from "./components/PingTab";
5757
import PromptsTab, { Prompt } from "./components/PromptsTab";
5858
import ResourcesTab from "./components/ResourcesTab";

‎client/src/components/History.tsx renamed to ‎client/src/components/HistoryAndNotifications.tsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,27 @@ const HistoryAndNotifications = ({
110110
>
111111
<div
112112
className="flex justify-between items-center cursor-pointer"
113-
onClick={() => toggleNotificationExpansion(index)}
113+
onClick={() =>
114+
toggleNotificationExpansion(
115+
serverNotifications.length - 1 - index,
116+
)
117+
}
114118
>
115119
<span className="font-mono">
116120
{serverNotifications.length - index}.{" "}
117121
{notification.method}
118122
</span>
119-
<span>{expandedNotifications[index] ? "▼" : "▶"}</span>
123+
<span>
124+
{expandedNotifications[
125+
serverNotifications.length - 1 - index
126+
]
127+
? "▼"
128+
: "▶"}
129+
</span>
120130
</div>
121-
{expandedNotifications[index] && (
131+
{expandedNotifications[
132+
serverNotifications.length - 1 - index
133+
] && (
122134
<div className="mt-2">
123135
<div className="flex justify-between items-center mb-1">
124136
<span className="font-semibold text-purple-600">

‎client/src/components/__tests__/HistoryAndNotifications.test.tsx

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
import { render, screen, fireEvent } from "@testing-library/react";
2+
import { describe, it, expect, jest } from "@jest/globals";
3+
import HistoryAndNotifications from "../HistoryAndNotifications";
4+
import { ServerNotification } from "@modelcontextprotocol/sdk/types.js";
5+
6+
// Mock JsonView component
7+
jest.mock("../JsonView", () => {
8+
return function JsonView({ data }: { data: string }) {
9+
return <div data-testid="json-view">{data}</div>;
10+
};
11+
});
12+
13+
describe("HistoryAndNotifications", () => {
14+
const mockRequestHistory = [
15+
{
16+
request: JSON.stringify({ method: "test/method1", params: {} }),
17+
response: JSON.stringify({ result: "success" }),
18+
},
19+
{
20+
request: JSON.stringify({ method: "test/method2", params: {} }),
21+
response: JSON.stringify({ result: "success" }),
22+
},
23+
];
24+
25+
const mockNotifications: ServerNotification[] = [
26+
{
27+
method: "notifications/message",
28+
params: {
29+
level: "info" as const,
30+
message: "First notification",
31+
},
32+
},
33+
{
34+
method: "notifications/progress",
35+
params: {
36+
progressToken: "test-token",
37+
progress: 50,
38+
message: "Second notification",
39+
},
40+
},
41+
];
42+
43+
it("renders history and notifications sections", () => {
44+
render(
45+
<HistoryAndNotifications
46+
requestHistory={mockRequestHistory}
47+
serverNotifications={mockNotifications}
48+
/>,
49+
);
50+
51+
expect(screen.getByText("History")).toBeTruthy();
52+
expect(screen.getByText("Server Notifications")).toBeTruthy();
53+
});
54+
55+
it("displays request history items with correct numbering", () => {
56+
render(
57+
<HistoryAndNotifications
58+
requestHistory={mockRequestHistory}
59+
serverNotifications={[]}
60+
/>,
61+
);
62+
63+
// Items should be numbered in reverse order (newest first)
64+
expect(screen.getByText("2. test/method2")).toBeTruthy();
65+
expect(screen.getByText("1. test/method1")).toBeTruthy();
66+
});
67+
68+
it("displays server notifications with correct numbering", () => {
69+
render(
70+
<HistoryAndNotifications
71+
requestHistory={[]}
72+
serverNotifications={mockNotifications}
73+
/>,
74+
);
75+
76+
// Items should be numbered in reverse order (newest first)
77+
expect(screen.getByText("2. notifications/progress")).toBeTruthy();
78+
expect(screen.getByText("1. notifications/message")).toBeTruthy();
79+
});
80+
81+
it("expands and collapses request items when clicked", () => {
82+
render(
83+
<HistoryAndNotifications
84+
requestHistory={mockRequestHistory}
85+
serverNotifications={[]}
86+
/>,
87+
);
88+
89+
const firstRequestHeader = screen.getByText("2. test/method2");
90+
91+
// Initially collapsed - should show ▶ arrows (there are multiple)
92+
expect(screen.getAllByText("▶")).toHaveLength(2);
93+
expect(screen.queryByText("Request:")).toBeNull();
94+
95+
// Click to expand
96+
fireEvent.click(firstRequestHeader);
97+
98+
// Should now be expanded - one ▼ and one ▶
99+
expect(screen.getByText("▼")).toBeTruthy();
100+
expect(screen.getAllByText("▶")).toHaveLength(1);
101+
expect(screen.getByText("Request:")).toBeTruthy();
102+
expect(screen.getByText("Response:")).toBeTruthy();
103+
});
104+
105+
it("expands and collapses notification items when clicked", () => {
106+
render(
107+
<HistoryAndNotifications
108+
requestHistory={[]}
109+
serverNotifications={mockNotifications}
110+
/>,
111+
);
112+
113+
const firstNotificationHeader = screen.getByText(
114+
"2. notifications/progress",
115+
);
116+
117+
// Initially collapsed
118+
expect(screen.getAllByText("▶")).toHaveLength(2);
119+
expect(screen.queryByText("Details:")).toBeNull();
120+
121+
// Click to expand
122+
fireEvent.click(firstNotificationHeader);
123+
124+
// Should now be expanded
125+
expect(screen.getByText("▼")).toBeTruthy();
126+
expect(screen.getAllByText("▶")).toHaveLength(1);
127+
expect(screen.getByText("Details:")).toBeTruthy();
128+
});
129+
130+
it("maintains expanded state when new notifications are added", () => {
131+
const { rerender } = render(
132+
<HistoryAndNotifications
133+
requestHistory={[]}
134+
serverNotifications={mockNotifications}
135+
/>,
136+
);
137+
138+
// Find and expand the older notification (should be "1. notifications/message")
139+
const olderNotificationHeader = screen.getByText(
140+
"1. notifications/message",
141+
);
142+
fireEvent.click(olderNotificationHeader);
143+
144+
// Verify it's expanded
145+
expect(screen.getByText("Details:")).toBeTruthy();
146+
147+
// Add a new notification at the beginning (simulating real behavior)
148+
const newNotifications: ServerNotification[] = [
149+
{
150+
method: "notifications/resources/updated",
151+
params: { uri: "file://test.txt" },
152+
},
153+
...mockNotifications,
154+
];
155+
156+
// Re-render with new notifications
157+
rerender(
158+
<HistoryAndNotifications
159+
requestHistory={[]}
160+
serverNotifications={newNotifications}
161+
/>,
162+
);
163+
164+
// The original notification should still be expanded
165+
// It's now numbered as "2. notifications/message" due to the new item
166+
expect(screen.getByText("3. notifications/progress")).toBeTruthy();
167+
expect(screen.getByText("2. notifications/message")).toBeTruthy();
168+
expect(screen.getByText("1. notifications/resources/updated")).toBeTruthy();
169+
170+
// The originally expanded notification should still show its details
171+
expect(screen.getByText("Details:")).toBeTruthy();
172+
});
173+
174+
it("maintains expanded state when new requests are added", () => {
175+
const { rerender } = render(
176+
<HistoryAndNotifications
177+
requestHistory={mockRequestHistory}
178+
serverNotifications={[]}
179+
/>,
180+
);
181+
182+
// Find and expand the older request (should be "1. test/method1")
183+
const olderRequestHeader = screen.getByText("1. test/method1");
184+
fireEvent.click(olderRequestHeader);
185+
186+
// Verify it's expanded
187+
expect(screen.getByText("Request:")).toBeTruthy();
188+
expect(screen.getByText("Response:")).toBeTruthy();
189+
190+
// Add a new request at the beginning
191+
const newRequestHistory = [
192+
{
193+
request: JSON.stringify({ method: "test/new_method", params: {} }),
194+
response: JSON.stringify({ result: "new success" }),
195+
},
196+
...mockRequestHistory,
197+
];
198+
199+
// Re-render with new request history
200+
rerender(
201+
<HistoryAndNotifications
202+
requestHistory={newRequestHistory}
203+
serverNotifications={[]}
204+
/>,
205+
);
206+
207+
// The original request should still be expanded
208+
// It's now numbered as "2. test/method1" due to the new item
209+
expect(screen.getByText("3. test/method2")).toBeTruthy();
210+
expect(screen.getByText("2. test/method1")).toBeTruthy();
211+
expect(screen.getByText("1. test/new_method")).toBeTruthy();
212+
213+
// The originally expanded request should still show its details
214+
expect(screen.getByText("Request:")).toBeTruthy();
215+
expect(screen.getByText("Response:")).toBeTruthy();
216+
});
217+
218+
it("displays empty state messages when no data is available", () => {
219+
render(
220+
<HistoryAndNotifications requestHistory={[]} serverNotifications={[]} />,
221+
);
222+
223+
expect(screen.getByText("No history yet")).toBeTruthy();
224+
expect(screen.getByText("No notifications yet")).toBeTruthy();
225+
});
226+
});

0 commit comments

Comments
 (0)