fix: persist template step edits on workflow save and add step delete confirmation (#394)

This commit is contained in:
Carlos Florêncio
2026-03-08 20:41:11 +00:00
committed by GitHub
parent fe9d91ddb3
commit ddcd101762
8 changed files with 472 additions and 26 deletions
@@ -261,7 +261,12 @@ function WorkflowSectionActions({ onExport, onImport, onAdd }: WorkflowSectionAc
<IconUpload className="h-4 w-4 mr-2" />
Import
</Button>
<Button size="sm" onClick={onAdd} className="cursor-pointer">
<Button
size="sm"
onClick={onAdd}
className="cursor-pointer"
data-testid="add-workflow-button"
>
<IconPlus className="h-4 w-4 mr-2" />
Add Workflow
</Button>
@@ -306,6 +311,7 @@ function WorkflowList({
workflow={workflow}
isWorkflowDirty={isWorkflowDirty(workflow)}
initialWorkflowSteps={initialWorkflowSteps}
templateStepCount={isTempWorkflow ? templateSteps.length : 0}
otherWorkflows={workflowItems.filter(
(w) => w.id !== workflow.id && !w.id.startsWith("temp-"),
)}
@@ -136,7 +136,7 @@ export function CreateWorkflowDialog({
}: CreateWorkflowDialogProps) {
return (
<Dialog open={open} onOpenChange={onOpenChange}>
<DialogContent className="sm:w-[900px] sm:max-w-none">
<DialogContent className="sm:w-[900px] sm:max-w-none" data-testid="create-workflow-dialog">
<DialogHeader>
<DialogTitle>Add Workflow</DialogTitle>
</DialogHeader>
@@ -148,6 +148,7 @@ export function CreateWorkflowDialog({
placeholder="My Project Workflow"
value={workflowName}
onChange={(e) => onWorkflowNameChange(e.target.value)}
data-testid="workflow-name-input"
/>
</div>
{workflowTemplates.length > 0 && (
@@ -191,7 +192,11 @@ export function CreateWorkflowDialog({
<Button variant="outline" onClick={() => onOpenChange(false)} className="cursor-pointer">
Cancel
</Button>
<Button onClick={onCreate} className="cursor-pointer">
<Button
onClick={onCreate}
className="cursor-pointer"
data-testid="confirm-create-workflow"
>
Add Workflow
</Button>
</DialogFooter>
@@ -377,10 +377,58 @@ export function useStepDeleteHandlers({
return { handleMigrateAndDeleteStep, handleDeleteStepAndTasks };
}
function diffStepUpdates(userStep: WorkflowStep, backendStep: WorkflowStep): Partial<WorkflowStep> {
const updates: Partial<WorkflowStep> = {};
if (userStep.name !== backendStep.name) updates.name = userStep.name;
if (userStep.color !== backendStep.color) updates.color = userStep.color;
if (userStep.prompt !== backendStep.prompt) updates.prompt = userStep.prompt;
if (userStep.is_start_step !== backendStep.is_start_step)
updates.is_start_step = userStep.is_start_step;
if (userStep.allow_manual_move !== backendStep.allow_manual_move)
updates.allow_manual_move = userStep.allow_manual_move;
if (JSON.stringify(userStep.events) !== JSON.stringify(backendStep.events))
updates.events = userStep.events;
return updates;
}
function stepPayload(workflowId: string, step: WorkflowStep) {
return {
workflow_id: workflowId,
name: step.name,
position: step.position,
color: step.color,
prompt: step.prompt,
events: step.events,
is_start_step: step.is_start_step,
allow_manual_move: step.allow_manual_move,
};
}
async function reconcileTemplateSteps(
createdId: string,
userSteps: WorkflowStep[],
templateStepCount: number,
) {
const { steps: backendSteps = [] } = await listWorkflowStepsAction(createdId);
for (const backendStep of backendSteps) {
const userStep = userSteps.find((s) => s.position === backendStep.position);
if (!userStep) continue;
const updates = diffStepUpdates(userStep, backendStep);
if (Object.keys(updates).length > 0) await updateWorkflowStepAction(backendStep.id, updates);
}
for (const step of userSteps) {
if (step.position >= templateStepCount)
await createWorkflowStepAction(stepPayload(createdId, step));
}
}
type WorkflowSaveActionsParams = {
workflow: Workflow;
isNewWorkflow: boolean;
workflowSteps: WorkflowStep[];
templateStepCount: number;
onSaveWorkflow: () => Promise<unknown>;
onWorkflowCreated?: (created: Workflow) => void;
toast: ReturnType<typeof useToast>["toast"];
@@ -390,6 +438,7 @@ export function useWorkflowSaveActions({
workflow,
isNewWorkflow,
workflowSteps,
templateStepCount,
onSaveWorkflow,
onWorkflowCreated,
toast,
@@ -403,23 +452,17 @@ export function useWorkflowSaveActions({
name: workflow.name.trim() || "New Workflow",
workflow_template_id: templateId || undefined,
});
// When a template is used, the backend creates steps with properly
// remapped step_id references (template aliases → real UUIDs).
// Only create steps manually when there's no template.
if (!templateId) {
if (templateId) {
// Backend creates template steps with remapped step_id references.
// Reconcile user edits and additions on top.
await reconcileTemplateSteps(created.id, workflowSteps, templateStepCount);
} else {
for (const step of workflowSteps) {
await createWorkflowStepAction({
workflow_id: created.id,
name: step.name,
position: step.position,
color: step.color,
prompt: step.prompt,
events: step.events,
is_start_step: step.is_start_step,
allow_manual_move: step.allow_manual_move,
});
await createWorkflowStepAction(stepPayload(created.id, step));
}
}
onWorkflowCreated?.(created);
});
@@ -26,6 +26,7 @@ type WorkflowCardProps = {
workflow: Workflow;
isWorkflowDirty: boolean;
initialWorkflowSteps?: WorkflowStep[];
templateStepCount?: number;
otherWorkflows?: Workflow[];
onUpdateWorkflow: (updates: { name?: string; description?: string }) => void;
onDeleteWorkflow: () => Promise<unknown>;
@@ -192,6 +193,7 @@ function WorkflowCardActions({
onClick={onDeleteClick}
disabled={deleteDisabled}
className="cursor-pointer"
data-testid="delete-workflow-button"
>
<IconTrash className="h-4 w-4 mr-2" />
Delete Workflow
@@ -339,6 +341,7 @@ export function WorkflowCard({
workflow,
isWorkflowDirty,
initialWorkflowSteps,
templateStepCount = 0,
otherWorkflows = [],
onUpdateWorkflow,
onDeleteWorkflow,
@@ -370,6 +373,7 @@ export function WorkflowCard({
workflow,
isNewWorkflow,
workflowSteps,
templateStepCount,
onSaveWorkflow,
onWorkflowCreated,
toast,
@@ -393,7 +397,7 @@ export function WorkflowCard({
: [];
return (
<Card>
<Card data-testid={`workflow-card-${workflow.id}`}>
<CardContent className="pt-6">
<div className="space-y-4">
<WorkflowCardBody
@@ -24,6 +24,16 @@ import {
IconRosetteNumber1,
} from "@tabler/icons-react";
import { StepCapabilityIcons } from "@/components/step-capability-icons";
import {
AlertDialog,
AlertDialogAction,
AlertDialogCancel,
AlertDialogContent,
AlertDialogDescription,
AlertDialogFooter,
AlertDialogHeader,
AlertDialogTitle,
} from "@kandev/ui/alert-dialog";
import { ScrollArea, ScrollBar } from "@kandev/ui/scroll-area";
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@kandev/ui/tooltip";
import type { WorkflowStep } from "@/lib/types/http";
@@ -199,6 +209,7 @@ function PipelineArea({
type="button"
onClick={readOnly ? undefined : onAddStep}
disabled={readOnly}
data-testid="add-step-button"
className="shrink-0 h-10 w-10 rounded-lg border border-dashed border-border text-muted-foreground hover:border-primary/50 hover:text-foreground flex items-center justify-center cursor-pointer disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
>
<IconPlus className="h-4 w-4" />
@@ -207,6 +218,39 @@ function PipelineArea({
);
}
// --- Step Delete Confirmation ---
function StepDeleteConfirmation({
stepName,
open,
onOpenChange,
onConfirm,
}: {
stepName: string;
open: boolean;
onOpenChange: (open: boolean) => void;
onConfirm: () => void;
}) {
return (
<AlertDialog open={open} onOpenChange={onOpenChange}>
<AlertDialogContent data-testid="step-delete-confirm-dialog">
<AlertDialogHeader>
<AlertDialogTitle>Delete step &ldquo;{stepName}&rdquo;?</AlertDialogTitle>
<AlertDialogDescription>
This will permanently remove the step from this workflow. This action cannot be undone.
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel className="cursor-pointer">Cancel</AlertDialogCancel>
<AlertDialogAction variant="destructive" onClick={onConfirm} className="cursor-pointer">
Delete
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
);
}
// --- Main Pipeline Editor ---
export function WorkflowPipelineEditor({
@@ -219,6 +263,7 @@ export function WorkflowPipelineEditor({
}: WorkflowPipelineEditorProps) {
const [selectedStepId, setSelectedStepId] = useState<string | null>(null);
const [prevStepCount, setPrevStepCount] = useState(steps.length);
const [stepToConfirmDelete, setStepToConfirmDelete] = useState<string | null>(null);
if (steps.length !== prevStepCount) {
if (steps.length > prevStepCount && steps.length > 0)
@@ -250,18 +295,26 @@ export function WorkflowPipelineEditor({
const handleSelectStep = (stepId: string) =>
setSelectedStepId((prev) => (prev === stepId ? null : stepId));
const handleRemoveStep = (stepId: string) => {
onRemoveStep(stepId);
if (selectedStepId === stepId) setSelectedStepId(null);
const requestRemoveStep = (stepId: string) => setStepToConfirmDelete(stepId);
const confirmRemoveStep = () => {
if (!stepToConfirmDelete) return;
onRemoveStep(stepToConfirmDelete);
if (selectedStepId === stepToConfirmDelete) setSelectedStepId(null);
setStepToConfirmDelete(null);
};
const stepToDeleteName = stepToConfirmDelete
? (steps.find((s) => s.id === stepToConfirmDelete)?.name ?? "this step")
: "";
const selectedStep = steps.find((s) => s.id === selectedStepId);
const pipelineArea = (
<PipelineArea
steps={steps}
selectedStepId={selectedStepId}
onSelectStep={handleSelectStep}
onRemoveStep={handleRemoveStep}
onRemoveStep={requestRemoveStep}
onAddStep={onAddStep}
readOnly={readOnly}
/>
@@ -291,13 +344,18 @@ export function WorkflowPipelineEditor({
step={selectedStep}
steps={steps}
onUpdate={(updates) => onUpdateStep(selectedStep.id, updates)}
onRemove={() => {
onRemoveStep(selectedStep.id);
setSelectedStepId(null);
}}
onRemove={() => requestRemoveStep(selectedStep.id)}
readOnly={readOnly}
/>
)}
<StepDeleteConfirmation
stepName={stepToDeleteName}
open={!!stepToConfirmDelete}
onOpenChange={(open) => {
if (!open) setStepToConfirmDelete(null);
}}
onConfirm={confirmRemoveStep}
/>
</div>
);
}
+14
View File
@@ -284,6 +284,20 @@ export class ApiClient {
await this.request("PUT", `/api/v1/workflow/steps/${stepId}`, { id: stepId, ...updates });
}
async deleteWorkflow(workflowId: string): Promise<void> {
await this.request("DELETE", `/api/v1/workflows/${workflowId}`);
}
async deleteWorkflowStep(stepId: string): Promise<void> {
await this.request("DELETE", `/api/v1/workflow/steps/${stepId}`);
}
async listWorkflowTemplates(): Promise<{
templates: Array<{ id: string; name: string; default_steps?: Array<{ name: string }> }>;
}> {
return this.request("GET", "/api/v1/workflow/templates");
}
// --- E2E Test Reset ---
async e2eReset(workspaceId: string, keepWorkflowIds?: string[]): Promise<void> {
@@ -0,0 +1,109 @@
import { type Locator, type Page, expect } from "@playwright/test";
export class WorkflowSettingsPage {
readonly page: Page;
readonly addWorkflowButton: Locator;
readonly createDialog: Locator;
readonly workflowNameInput: Locator;
readonly confirmCreateButton: Locator;
constructor(page: Page) {
this.page = page;
this.addWorkflowButton = page.getByTestId("add-workflow-button");
this.createDialog = page.getByTestId("create-workflow-dialog");
this.workflowNameInput = page.getByTestId("workflow-name-input");
this.confirmCreateButton = page.getByTestId("confirm-create-workflow");
}
async goto(workspaceId: string) {
await this.page.goto(`/settings/workspace/${workspaceId}/workflows`);
// Wait for a client-rendered element to confirm hydration is complete
// (networkidle is unreliable with persistent WebSocket connections)
await expect(this.addWorkflowButton).toBeVisible();
}
/** Returns the card container for a workflow by matching text in the card's name input. */
workflowCard(workflowId: string): Locator {
return this.page.getByTestId(`workflow-card-${workflowId}`);
}
/** Find a workflow card by the name shown in its input field using its current value. */
async findWorkflowCard(name: string): Promise<Locator> {
const cards = this.page.locator('[data-testid^="workflow-card-"]');
const count = await cards.count();
for (let i = 0; i < count; i++) {
const card = cards.nth(i);
const input = card.locator("input").first();
const value = await input.inputValue();
if (value === name) {
// Return a stable locator using the specific data-testid, not positional nth()
const testId = await card.getAttribute("data-testid");
if (testId) return this.page.getByTestId(testId);
return card;
}
}
// Return a locator that won't match so assertions can fail with a good message
return this.page.getByTestId(`workflow-card-not-found-${name}`);
}
/** The pipeline step nodes within a specific workflow card. */
stepNodes(card: Locator): Locator {
return card.locator('[data-slot="alert-dialog-trigger"], .group.relative').filter({
has: this.page.locator(".rounded-full"),
});
}
/** Find a step node by its name text within a card. */
stepNodeByName(card: Locator, stepName: string): Locator {
return card.locator(".group.relative").filter({ hasText: stepName });
}
/** The add-step (+) button within a workflow card. */
addStepButton(card: Locator): Locator {
return card.getByTestId("add-step-button");
}
/** The save button within a workflow card (matches button text "Save"). */
saveButton(card: Locator): Locator {
return card.getByRole("button", { name: "Save" });
}
/** The delete workflow button within a card. */
deleteWorkflowButton(card: Locator): Locator {
return card.getByTestId("delete-workflow-button");
}
/** The step delete confirmation dialog. */
get stepDeleteDialog(): Locator {
return this.page.getByTestId("step-delete-confirm-dialog");
}
/** Open the "Add Workflow" dialog and create a workflow. */
async createWorkflow(name: string, templateName?: string) {
await this.addWorkflowButton.click();
await expect(this.createDialog).toBeVisible();
if (name) {
await this.workflowNameInput.fill(name);
}
if (templateName === "Custom") {
await this.createDialog.locator('label[for="custom"]').click();
} else if (templateName) {
await this.createDialog.getByText(templateName, { exact: false }).first().click();
}
await this.confirmCreateButton.click();
await expect(this.createDialog).not.toBeVisible();
}
/** Hover over a step node to reveal the trash button, then click it. */
async clickDeleteStepButton(card: Locator, stepName: string) {
const node = this.stepNodeByName(card, stepName);
await node.hover();
await node
.locator("button")
.filter({ has: this.page.locator(".tabler-icon-trash") })
.click();
}
}
@@ -0,0 +1,207 @@
import { test, expect } from "../fixtures/test-base";
import { WorkflowSettingsPage } from "../pages/workflow-settings-page";
test.describe("Workflow settings", () => {
test("displays existing workflows on the settings page", async ({ testPage, seedData }) => {
const page = new WorkflowSettingsPage(testPage);
await page.goto(seedData.workspaceId);
// The seeded "E2E Workflow" should be visible
const card = await page.findWorkflowCard("E2E Workflow");
await expect(card).toBeVisible();
// Should display workflow steps from the "simple" template
for (const step of seedData.steps) {
await expect(card.getByText(step.name)).toBeVisible();
}
});
test("creates a workflow from template and persists after save", async ({
testPage,
seedData,
}) => {
const page = new WorkflowSettingsPage(testPage);
await page.goto(seedData.workspaceId);
// Create a new workflow from the first available template
await page.createWorkflow("Template Test Workflow");
// Verify the new card appears
const card = await page.findWorkflowCard("Template Test Workflow");
await expect(card).toBeVisible();
// Save the workflow
await page.saveButton(card).click();
// Wait for save to complete
await testPage.waitForTimeout(1000);
// Reload and verify persistence
await page.goto(seedData.workspaceId);
const reloadedCard = await page.findWorkflowCard("Template Test Workflow");
await expect(reloadedCard).toBeVisible();
});
test("creates a custom workflow without template", async ({ testPage, seedData }) => {
const page = new WorkflowSettingsPage(testPage);
await page.goto(seedData.workspaceId);
await page.createWorkflow("Custom Test Workflow", "Custom");
const card = await page.findWorkflowCard("Custom Test Workflow");
await expect(card).toBeVisible();
// Custom workflows get default steps (Todo, In Progress, Review, Done)
await expect(card.getByText("Todo")).toBeVisible();
await expect(card.getByText("In Progress")).toBeVisible();
await expect(card.getByText("Review")).toBeVisible();
await expect(card.getByText("Done")).toBeVisible();
// Save
await page.saveButton(card).click();
await testPage.waitForTimeout(1000);
// Reload and verify
await page.goto(seedData.workspaceId);
const reloadedCard = await page.findWorkflowCard("Custom Test Workflow");
await expect(reloadedCard).toBeVisible();
});
test("adds a step to template workflow and persists after save", async ({
testPage,
seedData,
}) => {
const page = new WorkflowSettingsPage(testPage);
await page.goto(seedData.workspaceId);
// Create workflow from template
await page.createWorkflow("Step Add Test");
const card = await page.findWorkflowCard("Step Add Test");
await expect(card).toBeVisible();
// Click the add step button
await page.addStepButton(card).click();
// A "New Step" should appear
await expect(card.getByText("New Step")).toBeVisible();
// Save the workflow
await page.saveButton(card).click();
await testPage.waitForTimeout(1000);
// Reload and verify the extra step persists
await page.goto(seedData.workspaceId);
const reloadedCard = await page.findWorkflowCard("Step Add Test");
await expect(reloadedCard).toBeVisible();
await expect(reloadedCard.getByText("New Step")).toBeVisible();
});
test("modifies a template step name and persists after save", async ({ testPage, seedData }) => {
const page = new WorkflowSettingsPage(testPage);
await page.goto(seedData.workspaceId);
// Use the first step name from seed data (same template)
const firstStepName = seedData.steps[0]?.name;
if (!firstStepName) {
test.skip(true, "No template steps available");
return;
}
// Create workflow from template
await page.createWorkflow("Step Edit Test");
const card = await page.findWorkflowCard("Step Edit Test");
await expect(card).toBeVisible();
// Click on the first step to open config panel
const stepNode = page.stepNodeByName(card, firstStepName);
await stepNode.click();
// Find the step name input in the config panel and rename it
const nameInput = card.getByPlaceholder("Step name");
await nameInput.clear();
await nameInput.fill("Renamed Step");
// Wait for debounced name update to propagate to state (500ms debounce)
await testPage.waitForTimeout(600);
// Save the workflow
await page.saveButton(card).click();
await testPage.waitForTimeout(1000);
// Reload and verify the renamed step persists
await page.goto(seedData.workspaceId);
const reloadedCard = await page.findWorkflowCard("Step Edit Test");
await expect(reloadedCard).toBeVisible();
await expect(reloadedCard.getByText("Renamed Step")).toBeVisible();
});
test("shows delete confirmation dialog when removing a step", async ({ testPage, seedData }) => {
const page = new WorkflowSettingsPage(testPage);
await page.goto(seedData.workspaceId);
// Create a custom workflow so we don't affect the seeded one
await page.createWorkflow("Delete Step Test", "Custom");
const card = await page.findWorkflowCard("Delete Step Test");
await expect(card).toBeVisible();
// Try to delete the "Review" step via hovering and clicking trash
await page.clickDeleteStepButton(card, "Review");
// Confirmation dialog should appear
await expect(page.stepDeleteDialog).toBeVisible();
await expect(page.stepDeleteDialog.getByText("Review")).toBeVisible();
// Cancel — step should still exist
await page.stepDeleteDialog.getByRole("button", { name: "Cancel" }).click();
await expect(page.stepDeleteDialog).not.toBeVisible();
await expect(card.getByText("Review")).toBeVisible();
// Delete again and confirm
await page.clickDeleteStepButton(card, "Review");
await expect(page.stepDeleteDialog).toBeVisible();
await page.stepDeleteDialog.getByRole("button", { name: "Delete" }).click();
await expect(page.stepDeleteDialog).not.toBeVisible();
// Step should be removed
await expect(page.stepNodeByName(card, "Review")).not.toBeVisible();
});
test("deletes a workflow", async ({ testPage, seedData }) => {
const page = new WorkflowSettingsPage(testPage);
await page.goto(seedData.workspaceId);
// Create and save a workflow first
await page.createWorkflow("To Delete Workflow", "Custom");
const card = await page.findWorkflowCard("To Delete Workflow");
await expect(card).toBeVisible();
// Save it first so it's persisted
await page.saveButton(card).click();
await testPage.waitForTimeout(1000);
// Reload to get the real workflow card
await page.goto(seedData.workspaceId);
const savedCard = await page.findWorkflowCard("To Delete Workflow");
await expect(savedCard).toBeVisible();
// Click delete workflow
await page.deleteWorkflowButton(savedCard).click();
// The delete dialog should appear — confirm deletion
const deleteDialog = testPage.getByRole("dialog").filter({ hasText: "Delete" });
await expect(deleteDialog).toBeVisible();
// Click the delete button (it will say "Delete" or "Delete Workflow")
await deleteDialog
.getByRole("button", { name: /delete/i })
.last()
.click();
// Workflow card should be removed
const deletedCard = await page.findWorkflowCard("To Delete Workflow");
await expect(deletedCard).not.toBeVisible();
});
});