Skip to content

Commit dc9e546

Browse files
marekdanoMarek Danoa-effort
authored andcommitted
feat(ui-rewrite): delete a prompt from the details panel (#5670)
* feat(ui-rewrite): delete a prompt from the details panel Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * modified dialog height Signed-off-by: a-effort <anna.effort@ibm.com> * fix: unit test for dialog position Signed-off-by: Marek Dano <Marek.Dano@ibm.com> * fix(ui-rewrite): bound dialog height and reconcile failed prompt deletes Signed-off-by: Marek Dano <Marek.Dano@ibm.com> --------- Signed-off-by: Marek Dano <Marek.Dano@ibm.com> Signed-off-by: a-effort <anna.effort@ibm.com> Co-authored-by: Marek Dano <Marek.Dano@ibm.com> Co-authored-by: a-effort <anna.effort@ibm.com>
1 parent fcda9bc commit dc9e546

9 files changed

Lines changed: 413 additions & 13 deletions

File tree

client/src/api/prompts.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,42 @@ describe("promptsApi", () => {
123123
);
124124
});
125125
});
126+
127+
describe("delete", () => {
128+
it("calls DELETE /prompts/:id with CSRF token and same-origin credentials", async () => {
129+
mockFetch.mockResolvedValueOnce(new Response(null, { status: 204 }));
130+
131+
await promptsApi.delete("prompt-abc-123");
132+
133+
expect(mockFetch).toHaveBeenCalledWith(
134+
expect.stringContaining("/prompts/prompt-abc-123"),
135+
expect.objectContaining({
136+
method: "DELETE",
137+
headers: expect.objectContaining({
138+
"X-CSRF-Token": "test-csrf-token",
139+
}),
140+
credentials: "same-origin", // pragma: allowlist secret
141+
}),
142+
);
143+
});
144+
145+
it("throws synchronously for an empty ID", () => {
146+
expect(() => promptsApi.delete("")).toThrow("Invalid prompt ID");
147+
});
148+
149+
it("throws synchronously for an ID with path traversal characters", () => {
150+
expect(() => promptsApi.delete("../etc/passwd")).toThrow("Invalid prompt ID format");
151+
});
152+
153+
it("throws ApiError on 404 response", async () => {
154+
mockFetch.mockResolvedValueOnce(
155+
new Response(JSON.stringify({ detail: "Prompt not found" }), {
156+
status: 404,
157+
headers: { "Content-Type": "application/json" },
158+
}),
159+
);
160+
161+
await expect(promptsApi.delete("prompt-abc-123")).rejects.toThrow("HTTP 404");
162+
});
163+
});
126164
});

client/src/api/prompts.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,16 @@ export const promptsApi = {
116116
const validId = validatePromptId(id);
117117
return api.put<PromptRead>(`/prompts/${encodeURIComponent(validId)}`, { tags });
118118
},
119+
120+
/**
121+
* Delete a prompt by its primary-key ID.
122+
*
123+
* Mirrors `DELETE /prompts/{prompt_id}` (`mcpgateway/main.py`), which requires
124+
* the `prompts.delete` permission. The ID is validated (not the name used by
125+
* {@link promptsApi.render}) to guard against path traversal and injection.
126+
*/
127+
delete: (id: string): Promise<void> => {
128+
const validId = validatePromptId(id);
129+
return api.delete(`/prompts/${encodeURIComponent(validId)}`);
130+
},
119131
};

client/src/components/ui/dialog.test.tsx

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,33 @@ describe("Dialog Components", () => {
204204

205205
const content = screen.getByTestId("content");
206206
expect(content).toHaveClass("fixed");
207-
expect(content).toHaveClass("inset-0");
208-
expect(content).toHaveClass("m-auto");
207+
expect(content).toHaveClass("inset-x-0");
208+
expect(content).toHaveClass("top-1/2");
209+
expect(content).toHaveClass("-translate-y-1/2");
210+
expect(content).toHaveClass("mx-auto");
209211
expect(content).toHaveClass("z-50");
210212
});
211213

214+
it("bounds its height to the viewport and scrolls overflowing content", () => {
215+
// Tall dialogs must stay within the viewport so the header, top content,
216+
// and close button remain reachable instead of extending off-screen.
217+
render(
218+
<Dialog open={true}>
219+
<DialogContent data-testid="content">
220+
<DialogTitle>Tall dialog</DialogTitle>
221+
<DialogDescription>Lots of content</DialogDescription>
222+
{Array.from({ length: 100 }, (_, i) => (
223+
<p key={i}>Row {i}</p>
224+
))}
225+
</DialogContent>
226+
</Dialog>,
227+
);
228+
229+
const content = screen.getByTestId("content");
230+
expect(content).toHaveClass("max-h-[calc(100vh-2rem)]");
231+
expect(content).toHaveClass("overflow-y-auto");
232+
});
233+
212234
it("should include close button", () => {
213235
render(
214236
<Dialog open={true}>

client/src/components/ui/dialog.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const DialogContent = React.forwardRef<
3636
<DialogPrimitive.Content
3737
ref={ref}
3838
className={cn(
39-
"fixed inset-0 z-50 m-auto grid h-fit w-full max-w-lg gap-4 border bg-popover p-6 shadow-lg duration-150 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 sm:rounded-lg",
39+
"fixed inset-x-0 top-1/2 z-50 mx-auto grid max-h-[calc(100vh-2rem)] w-full max-w-lg -translate-y-1/2 gap-5 overflow-y-auto border bg-popover p-7 shadow-lg duration-150 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 sm:rounded-lg",
4040
className,
4141
)}
4242
{...props}
@@ -52,7 +52,7 @@ const DialogContent = React.forwardRef<
5252
DialogContent.displayName = DialogPrimitive.Content.displayName;
5353

5454
const DialogHeader = ({ className, ...props }: React.HTMLAttributes<HTMLDivElement>) => (
55-
<div className={cn("flex flex-col space-y-1.5 text-center sm:text-left", className)} {...props} />
55+
<div className={cn("flex flex-col space-y-2 text-center sm:text-left", className)} {...props} />
5656
);
5757
DialogHeader.displayName = "DialogHeader";
5858

client/src/i18n/locales/en-US/prompts.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,10 @@
8181
"prompts.add.error.teamRequired": "Team selection is required when visibility is set to team",
8282
"prompts.add.error.argumentsInvalidJson": "Invalid JSON format",
8383
"prompts.add.error.argumentsArrayRequired": "Arguments must be a JSON array",
84-
"prompts.tags.addError": "Failed to add tag. Please try again."
84+
"prompts.tags.addError": "Failed to add tag. Please try again.",
85+
"prompts.delete.confirm.title": "Delete prompt",
86+
"prompts.delete.confirm.description": "Are you sure you want to delete \"{name}\"? This action cannot be undone.",
87+
"prompts.delete.confirm.button": "Delete",
88+
"prompts.delete.success": "Prompt \"{name}\" deleted successfully",
89+
"prompts.delete.error": "Failed to delete prompt"
8590
}

client/src/i18n/locales/es-ES/prompts.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,10 @@
8181
"prompts.add.error.teamRequired": "La selección de equipo es obligatoria cuando la visibilidad está configurada como equipo",
8282
"prompts.add.error.argumentsInvalidJson": "Formato JSON inválido",
8383
"prompts.add.error.argumentsArrayRequired": "Los argumentos deben ser un array JSON",
84-
"prompts.tags.addError": "No se pudo añadir la etiqueta. Inténtelo de nuevo."
84+
"prompts.tags.addError": "No se pudo añadir la etiqueta. Inténtelo de nuevo.",
85+
"prompts.delete.confirm.title": "Eliminar prompt",
86+
"prompts.delete.confirm.description": "¿Está seguro de que desea eliminar \"{name}\"? Esta acción no se puede deshacer.",
87+
"prompts.delete.confirm.button": "Eliminar",
88+
"prompts.delete.success": "Prompt \"{name}\" eliminado correctamente",
89+
"prompts.delete.error": "Error al eliminar el prompt"
8590
}

client/src/i18n/locales/pt-BR/prompts.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,10 @@
8181
"prompts.add.error.teamRequired": "A seleção de equipe é obrigatória quando a visibilidade está definida como equipe",
8282
"prompts.add.error.argumentsInvalidJson": "Formato JSON inválido",
8383
"prompts.add.error.argumentsArrayRequired": "Os argumentos devem ser um array JSON",
84-
"prompts.tags.addError": "Falha ao adicionar a tag. Tente novamente."
84+
"prompts.tags.addError": "Falha ao adicionar a tag. Tente novamente.",
85+
"prompts.delete.confirm.title": "Excluir prompt",
86+
"prompts.delete.confirm.description": "Tem certeza de que deseja excluir \"{name}\"? Esta ação não pode ser desfeita.",
87+
"prompts.delete.confirm.button": "Excluir",
88+
"prompts.delete.success": "Prompt \"{name}\" excluído com sucesso",
89+
"prompts.delete.error": "Falha ao excluir o prompt"
8590
}

client/src/pages/Prompts.test.tsx

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,230 @@ describe("Prompts", () => {
295295
expect(screen.getByRole("menuitem", { name: "Delete" })).toBeInTheDocument();
296296
});
297297

298+
it("deletes the sole prompt in a group and closes the drawer after confirmation", async () => {
299+
const user = userEvent.setup();
300+
let remaining: Prompt[] = [
301+
createMockPrompt({
302+
id: "prompt-1",
303+
name: "summarize",
304+
displayName: "Summarize document",
305+
gatewayId: "gw-github",
306+
gatewaySlug: "gh-repo-tasks",
307+
}),
308+
];
309+
310+
const deleteSpy = vi.fn(() => {
311+
remaining = remaining.filter((p) => p.id !== "prompt-1");
312+
return HttpResponse.json({ status: "success" });
313+
});
314+
server.use(
315+
http.get("/prompts", () => HttpResponse.json(remaining)),
316+
http.delete("/prompts/prompt-1", deleteSpy),
317+
);
318+
319+
renderWithProviders(<Prompts />);
320+
321+
await waitFor(() => {
322+
expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument();
323+
});
324+
325+
// Open the group's details panel and its Definition tab.
326+
await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" }));
327+
await user.click(await screen.findByRole("menuitem", { name: "View details" }));
328+
await user.click(await screen.findByRole("tab", { name: /definition/i }));
329+
330+
// Trigger delete from the row overflow menu.
331+
await user.click(await screen.findByRole("button", { name: /more options for summarize/i }));
332+
await user.click(await screen.findByRole("menuitem", { name: "Delete" }));
333+
334+
// Confirm dialog names the prompt and requires an explicit confirmation.
335+
const dialog = await screen.findByRole("dialog");
336+
expect(within(dialog).getByText("Delete prompt")).toBeInTheDocument();
337+
expect(
338+
within(dialog).getByText(/Are you sure you want to delete "Summarize document"/),
339+
).toBeInTheDocument();
340+
341+
await user.click(within(dialog).getByRole("button", { name: /^delete$/i }));
342+
343+
await waitFor(() => {
344+
expect(deleteSpy).toHaveBeenCalledTimes(1);
345+
});
346+
347+
// The group's only prompt is gone, so its grid card (and the card-only
348+
// "More options" trigger) disappears.
349+
await waitFor(() => {
350+
expect(
351+
screen.queryByRole("button", { name: "More options for gh-repo-tasks" }),
352+
).not.toBeInTheDocument();
353+
});
354+
});
355+
356+
it("keeps the drawer and other rows when deleting one prompt from a multi-prompt group", async () => {
357+
const user = userEvent.setup();
358+
let remaining: Prompt[] = [
359+
createMockPrompt({
360+
id: "prompt-1",
361+
name: "summarize",
362+
displayName: "Summarize document",
363+
gatewayId: "gw-github",
364+
gatewaySlug: "gh-repo-tasks",
365+
}),
366+
createMockPrompt({
367+
id: "prompt-2",
368+
name: "translate",
369+
displayName: "Translate document",
370+
gatewayId: "gw-github",
371+
gatewaySlug: "gh-repo-tasks",
372+
}),
373+
];
374+
375+
server.use(
376+
http.get("/prompts", () => HttpResponse.json(remaining)),
377+
http.delete("/prompts/prompt-1", () => {
378+
remaining = remaining.filter((p) => p.id !== "prompt-1");
379+
return HttpResponse.json({ status: "success" });
380+
}),
381+
);
382+
383+
renderWithProviders(<Prompts />);
384+
385+
await waitFor(() => {
386+
expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument();
387+
});
388+
389+
await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" }));
390+
await user.click(await screen.findByRole("menuitem", { name: "View details" }));
391+
await user.click(await screen.findByRole("tab", { name: /definition/i }));
392+
393+
await user.click(await screen.findByRole("button", { name: /more options for summarize/i }));
394+
await user.click(await screen.findByRole("menuitem", { name: "Delete" }));
395+
396+
const dialog = await screen.findByRole("dialog");
397+
await user.click(within(dialog).getByRole("button", { name: /^delete$/i }));
398+
399+
// The deleted row is gone but the drawer stays open with the sibling prompt.
400+
await waitFor(() => {
401+
expect(
402+
screen.queryByRole("button", { name: /more options for summarize/i }),
403+
).not.toBeInTheDocument();
404+
});
405+
expect(screen.getByRole("button", { name: /more options for translate/i })).toBeInTheDocument();
406+
});
407+
408+
it("reconciles with the server and restores the row when the DELETE fails and the prompt survives", async () => {
409+
const user = userEvent.setup();
410+
const prompts: Prompt[] = [
411+
createMockPrompt({
412+
id: "prompt-1",
413+
name: "summarize",
414+
displayName: "Summarize document",
415+
gatewayId: "gw-github",
416+
gatewaySlug: "gh-repo-tasks",
417+
}),
418+
createMockPrompt({
419+
id: "prompt-2",
420+
name: "translate",
421+
displayName: "Translate document",
422+
gatewayId: "gw-github",
423+
gatewaySlug: "gh-repo-tasks",
424+
}),
425+
];
426+
427+
// The DELETE fails and the prompt is NOT removed server-side (a pre-commit
428+
// failure), so the post-failure refetch still returns it and the row
429+
// reappears.
430+
server.use(
431+
http.get("/prompts", () => HttpResponse.json(prompts)),
432+
http.delete("/prompts/prompt-1", () =>
433+
HttpResponse.json({ detail: "Prompt is in use" }, { status: 409 }),
434+
),
435+
);
436+
437+
renderWithProviders(<Prompts />);
438+
439+
await waitFor(() => {
440+
expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument();
441+
});
442+
443+
await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" }));
444+
await user.click(await screen.findByRole("menuitem", { name: "View details" }));
445+
await user.click(await screen.findByRole("tab", { name: /definition/i }));
446+
447+
await user.click(await screen.findByRole("button", { name: /more options for summarize/i }));
448+
await user.click(await screen.findByRole("menuitem", { name: "Delete" }));
449+
450+
const dialog = await screen.findByRole("dialog");
451+
await user.click(within(dialog).getByRole("button", { name: /^delete$/i }));
452+
453+
// The optimistic removal is reconciled against server truth, so the row
454+
// reappears because the prompt still exists.
455+
await waitFor(() => {
456+
expect(
457+
screen.getByRole("button", { name: /more options for summarize/i }),
458+
).toBeInTheDocument();
459+
});
460+
});
461+
462+
it("does not resurrect a prompt when the DELETE reports an error but the row is already gone", async () => {
463+
const user = userEvent.setup();
464+
// Simulates a post-commit failure: PromptService.delete_prompt() commits
465+
// the removal, then a later step (notification/cache invalidation) fails
466+
// and the endpoint returns an error. The server no longer has the prompt,
467+
// so reconciling must NOT restore it.
468+
let remaining: Prompt[] = [
469+
createMockPrompt({
470+
id: "prompt-1",
471+
name: "summarize",
472+
displayName: "Summarize document",
473+
gatewayId: "gw-github",
474+
gatewaySlug: "gh-repo-tasks",
475+
}),
476+
createMockPrompt({
477+
id: "prompt-2",
478+
name: "translate",
479+
displayName: "Translate document",
480+
gatewayId: "gw-github",
481+
gatewaySlug: "gh-repo-tasks",
482+
}),
483+
];
484+
485+
server.use(
486+
http.get("/prompts", () => HttpResponse.json(remaining)),
487+
http.delete("/prompts/prompt-1", () => {
488+
// Row is committed as deleted, but the request still errors out.
489+
remaining = remaining.filter((p) => p.id !== "prompt-1");
490+
return HttpResponse.json({ detail: "Post-commit hook failed" }, { status: 500 });
491+
}),
492+
);
493+
494+
renderWithProviders(<Prompts />);
495+
496+
await waitFor(() => {
497+
expect(screen.getByText("gh-repo-tasks")).toBeInTheDocument();
498+
});
499+
500+
await user.click(screen.getByRole("button", { name: "More options for gh-repo-tasks" }));
501+
await user.click(await screen.findByRole("menuitem", { name: "View details" }));
502+
await user.click(await screen.findByRole("tab", { name: /definition/i }));
503+
504+
await user.click(await screen.findByRole("button", { name: /more options for summarize/i }));
505+
await user.click(await screen.findByRole("menuitem", { name: "Delete" }));
506+
507+
const dialog = await screen.findByRole("dialog");
508+
await user.click(within(dialog).getByRole("button", { name: /^delete$/i }));
509+
510+
// The drawer stays open with the surviving sibling, and the deleted prompt
511+
// is not brought back by the failure path.
512+
await waitFor(() => {
513+
expect(
514+
screen.getByRole("button", { name: /more options for translate/i }),
515+
).toBeInTheDocument();
516+
});
517+
expect(
518+
screen.queryByRole("button", { name: /more options for summarize/i }),
519+
).not.toBeInTheDocument();
520+
});
521+
298522
it("collapses gateway-less prompts into a single REST prompts card", async () => {
299523
const prompts: Prompt[] = [
300524
createMockPrompt({

0 commit comments

Comments
 (0)