Skip to content

Commit 352317f

Browse files
committed
refactor(ui): make PANEL_SEARCH_CONFIG registration mandatory and update tests to use correct entity types
Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>
1 parent 80bdbed commit 352317f

File tree

2 files changed

+32
-40
lines changed

2 files changed

+32
-40
lines changed

mcpgateway/admin_ui/formHandlers.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ import { getCookie, isInactiveChecked } from "./utils.js";
1010
export const handleFormSubmitAndRefresh = async function (event, type) {
1111
event.preventDefault();
1212

13+
// Validate PANEL_SEARCH_CONFIG registration before proceeding
14+
const panelConfig = PANEL_SEARCH_CONFIG[type];
15+
if (!panelConfig) {
16+
throw new Error(`No PANEL_SEARCH_CONFIG found for type: ${type}. All entity types must be registered in PANEL_SEARCH_CONFIG (constants.js) with partialPath and targetSelector.`);
17+
}
18+
1319
const isInactiveCheckedBool = isInactiveChecked(type);
1420
const form = event.target;
1521
const teamId = new URL(window.location.href).searchParams.get("team_id");
@@ -50,12 +56,8 @@ export const handleFormSubmitAndRefresh = async function (event, type) {
5056
}
5157

5258
// Trigger HTMX request to refresh the table using PANEL_SEARCH_CONFIG
53-
const panelConfig = PANEL_SEARCH_CONFIG[type];
54-
if (!panelConfig) {
55-
console.warn(`No PANEL_SEARCH_CONFIG found for type: ${type}, using fallback pattern. Consider adding this type to PANEL_SEARCH_CONFIG in constants.js`);
56-
}
57-
const partialPath = panelConfig?.partialPath || `${type}/partial`;
58-
const targetSelector = panelConfig?.targetSelector || `#${type}-table`;
59+
const partialPath = panelConfig.partialPath;
60+
const targetSelector = panelConfig.targetSelector;
5961
const partialUrl = `${window.ROOT_PATH}/admin/${partialPath}?${params.toString()}`;
6062

6163
if (window.htmx) {
@@ -67,9 +69,9 @@ export const handleFormSubmitAndRefresh = async function (event, type) {
6769
// Fallback to full reload if HTMX not available
6870
navigateAdmin(fragment, params);
6971
}
70-
} catch (e) {
72+
} catch (error) {
7173
// Network error — still navigate so the user sees refreshed state.
72-
console.error("Form submit error:", e);
74+
console.error("Form submit error:", error);
7375
const fragment = TOGGLE_FRAGMENT_MAP[type] || type;
7476
const params = new URLSearchParams();
7577
if (teamId) {

tests/unit/js/formHandlers.test.js

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ describe("handleSubmitWithConfirmation", () => {
8282

8383
vi.spyOn(window, "confirm").mockReturnValue(true);
8484

85-
await handleSubmitWithConfirmation(event, "tool");
85+
await handleSubmitWithConfirmation(event, "tools");
8686

8787
expect(window.confirm).toHaveBeenCalledWith(
88-
expect.stringContaining("permanently delete this tool")
88+
expect.stringContaining("permanently delete this tools")
8989
);
9090
expect(fetchMock).toHaveBeenCalled();
9191
});
@@ -101,7 +101,7 @@ describe("handleSubmitWithConfirmation", () => {
101101

102102
vi.spyOn(window, "confirm").mockReturnValue(false);
103103

104-
const result = handleSubmitWithConfirmation(event, "tool");
104+
const result = handleSubmitWithConfirmation(event, "tools");
105105

106106
expect(result).toBe(false);
107107
expect(fetchMock).not.toHaveBeenCalled();
@@ -125,7 +125,7 @@ describe("handleDeleteSubmit", () => {
125125
.mockReturnValueOnce(true) // first confirm (delete)
126126
.mockReturnValueOnce(true); // second confirm (purge metrics)
127127

128-
await handleDeleteSubmit(event, "gateway", "test-gw");
128+
await handleDeleteSubmit(event, "gateways", "test-gw");
129129

130130
expect(window.confirm).toHaveBeenCalledTimes(2);
131131
const purgeField = form.querySelector('input[name="purge_metrics"]');
@@ -145,10 +145,10 @@ describe("handleDeleteSubmit", () => {
145145
.mockReturnValueOnce(true)
146146
.mockReturnValueOnce(false);
147147

148-
handleDeleteSubmit(event, "tool", "my-tool");
148+
handleDeleteSubmit(event, "tools", "my-tool");
149149

150150
expect(window.confirm).toHaveBeenCalledWith(
151-
expect.stringContaining('tool "my-tool"')
151+
expect.stringContaining('tools "my-tool"')
152152
);
153153
});
154154

@@ -165,7 +165,7 @@ describe("handleDeleteSubmit", () => {
165165
.mockReturnValueOnce(true)
166166
.mockReturnValueOnce(false);
167167

168-
await handleDeleteSubmit(event, "server");
168+
await handleDeleteSubmit(event, "catalog");
169169

170170
const purgeField = form.querySelector('input[name="purge_metrics"]');
171171
expect(purgeField).toBeNull();
@@ -181,7 +181,7 @@ describe("handleDeleteSubmit", () => {
181181

182182
vi.spyOn(window, "confirm").mockReturnValue(false);
183183

184-
const result = handleDeleteSubmit(event, "resource");
184+
const result = handleDeleteSubmit(event, "resources");
185185

186186
expect(result).toBe(false);
187187
expect(form.submit).not.toHaveBeenCalled();
@@ -204,7 +204,7 @@ describe("handleDeleteSubmit", () => {
204204
.mockReturnValueOnce(true)
205205
.mockReturnValueOnce(false);
206206

207-
await handleDeleteSubmit(event, "tool", "t1");
207+
await handleDeleteSubmit(event, "tools", "t1");
208208

209209
expect(fetchMock).toHaveBeenCalled();
210210
const callArgs = fetchMock.mock.calls[0];
@@ -215,10 +215,10 @@ describe("handleDeleteSubmit", () => {
215215
});
216216

217217
test("passes inactiveType to isInactiveChecked via hidden field value", async () => {
218-
// Add checked checkbox for "custom-type" so isInactiveChecked("custom-type") returns true
218+
// Add checked checkbox for "prompts" so isInactiveChecked("prompts") returns true
219219
const cb = document.createElement("input");
220220
cb.type = "checkbox";
221-
cb.id = "show-inactive-custom-type";
221+
cb.id = "show-inactive-prompts";
222222
cb.checked = true;
223223
document.body.appendChild(cb);
224224

@@ -237,7 +237,7 @@ describe("handleDeleteSubmit", () => {
237237
return Promise.resolve({ ok: true });
238238
});
239239

240-
await handleDeleteSubmit(event, "tool", "t1", "custom-type");
240+
await handleDeleteSubmit(event, "tools", "t1", "prompts");
241241

242242
expect(capturedFormData.get("is_inactive_checked")).toBe("true");
243243
});
@@ -266,6 +266,8 @@ describe("handleDeleteSubmit", () => {
266266
.mockReturnValueOnce(true)
267267
.mockReturnValueOnce(false);
268268

269+
// type="agent" for the confirmation message, but inactiveType="a2a-agents" is used
270+
// for refresh lookup in PANEL_SEARCH_CONFIG (handleDeleteSubmit uses inactiveType || type)
269271
await handleDeleteSubmit(event, "agent", "test-agent", "a2a-agents");
270272

271273
// Verify HTMX was called with correct partial path (a2a/partial) and target selector (#agents-table)
@@ -303,7 +305,7 @@ describe("handleDeleteSubmit", () => {
303305
.mockReturnValueOnce(true)
304306
.mockReturnValueOnce(false);
305307

306-
await handleDeleteSubmit(event, "server", "test-server", "catalog");
308+
await handleDeleteSubmit(event, "catalog", "test-server", "catalog");
307309

308310
// Verify HTMX was called with correct partial path (servers/partial) and target selector (#servers-table)
309311
expect(htmxAjaxMock).toHaveBeenCalledWith(
@@ -316,7 +318,7 @@ describe("handleDeleteSubmit", () => {
316318
);
317319
});
318320

319-
test("warns when PANEL_SEARCH_CONFIG is missing for a type", async () => {
321+
test("throws error when PANEL_SEARCH_CONFIG is missing for a type", async () => {
320322
const form = document.createElement("form");
321323
form.id = "test-form";
322324
form.action = "/test";
@@ -330,30 +332,18 @@ describe("handleDeleteSubmit", () => {
330332
global.window.htmx = { ajax: htmxAjaxMock };
331333
global.window.ROOT_PATH = "";
332334

333-
// Mock console.warn
334-
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
335-
336335
const event = { preventDefault: vi.fn(), target: form };
337336

338337
vi.spyOn(window, "confirm")
339338
.mockReturnValueOnce(true)
340339
.mockReturnValueOnce(false);
341340

342-
await handleDeleteSubmit(event, "unknown", "test-unknown", "unknown-type");
343-
344-
// Verify console.warn was called with appropriate message
345-
expect(consoleWarnSpy).toHaveBeenCalledWith(
346-
expect.stringContaining('No PANEL_SEARCH_CONFIG found for type: unknown-type')
347-
);
341+
// Should throw error for unregistered entity type
342+
await expect(async () => {
343+
await handleDeleteSubmit(event, "unknown", "test-unknown", "unknown-type");
344+
}).rejects.toThrow('No PANEL_SEARCH_CONFIG found for type: unknown-type');
348345

349-
// Should still use fallback pattern
350-
expect(htmxAjaxMock).toHaveBeenCalledWith(
351-
'GET',
352-
expect.stringContaining('/admin/unknown-type/partial'),
353-
expect.objectContaining({
354-
target: '#unknown-type-table',
355-
swap: 'outerHTML'
356-
})
357-
);
346+
// HTMX should NOT be called since error is thrown before refresh
347+
expect(htmxAjaxMock).not.toHaveBeenCalled();
358348
});
359349
});

0 commit comments

Comments
 (0)