[CCOR-13383]fix:task definition form missing fields

ui-next/src/pages/definition/task/TaskDefinitionButtons.description.test.tsx

@@ -0,0 +1,148 @@
+/**
+ * Save used to be disabled whenever the description was empty, with nothing on
+ * screen explaining why. helpers.test.ts pins the rule that replaced it, but
+ * only by omission — isSaveDisabled takes no description at all. This asserts
+ * it where a description actually exists: the rendered Save button.
+ *
+ * The real task definition and form machines run, with the fetch service
+ * stubbed; only leaf UI is replaced.
+ */
+import "@testing-library/jest-dom";
+import { render, waitFor } from "@testing-library/react";
+import { interpret } from "xstate";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { Provider as ThemeProvider } from "theme/material/provider";
+import { taskDefinitionMachine } from "pages/definition/task/state/machine";
+import { TaskDefinitionFormEventType } from "pages/definition/task/form/state/types";
+import { TASK_FORM_MACHINE_ID } from "pages/definition/task/state/helpers";
+import { TaskDefinitionDto } from "types/TaskDefinition";
+
+vi.mock("components/features/auth", () => ({
+  useAuth: () => ({ isTrialExpired: false }),
+}));
+
+vi.mock(
+  "../EditorPanel/TaskFormTab/forms/TestTaskButton/OpenTestTaskButton",
+  () => ({ OpenTestTaskButton: () => null }),
+);
+
+vi.mock("components/ui/buttons/ConductorSplitButton", () => ({
+  default: ({
+    children,
+    disabled,
+    id,
+  }: {
+    children?: React.ReactNode;
+    disabled?: boolean;
+    id?: string;
+  }) => (
+    <button type="button" id={id} disabled={disabled}>
+      {children}
+    </button>
+  ),
+}));
+
+const TASK_WITHOUT_DESCRIPTION = {
+  name: "my_task",
+  description: "",
+  retryCount: 3,
+  retryDelaySeconds: 60,
+  retryLogic: "FIXED",
+  timeoutSeconds: 3600,
+  timeoutPolicy: "TIME_OUT_WF",
+  responseTimeoutSeconds: 600,
+  ownerEmail: "owner@orkes.io",
+} as unknown as TaskDefinitionDto;
+
+/**
+ * Boots the page's machine on an existing definition, so Save renders through
+ * the form-state wrapper rather than the new-definition split button.
+ */
+const renderButtons = async (taskDefinition: TaskDefinitionDto) => {
+  const { default: TaskDefinitionButtons } =
+    await import("./TaskDefinitionButtons");
+  const service = interpret(
+    taskDefinitionMachine
+      .withConfig({
+        services: {
+          fetchTaskDefinitionByNameService: async () => taskDefinition,
+        },
+      })
+      .withContext({
+        ...taskDefinitionMachine.context,
+        isNewTaskDef: false,
+        modifiedTaskDefinition: taskDefinition,
+        originTaskDefinition: taskDefinition,
+      }),
+  ).start();
+
+  render(
+    <ThemeProvider>
+      <TaskDefinitionButtons taskDefActor={service as never} />
+    </ThemeProvider>,
+  );
+
+  await waitFor(() =>
+    expect(service.children.get(TASK_FORM_MACHINE_ID)).toBeTruthy(),
+  );
+  return { service };
+};
+
+/** Edits a field other than the description, so there is something to save. */
+const editSomethingElse = (service: {
+  children: Map<string, { send: (event: unknown) => void }>;
+}) => {
+  service.children.get(TASK_FORM_MACHINE_ID)!.send({
+    type: TaskDefinitionFormEventType.HANDLE_CHANGE_TASK_FORM,
+    name: "timeoutSeconds",
+    value: 1200,
+  });
+};
+
+const saveButton = () => document.querySelector("#task-save-btn");
+
+describe("TaskDefinitionButtons — description is not required to save", () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it("enables Save on an edited definition whose description is empty", async () => {
+    const { service } = await renderButtons(TASK_WITHOUT_DESCRIPTION);
+
+    editSomethingElse(service as never);
+
+    await waitFor(() => expect(saveButton()).not.toBeDisabled());
+  });
+
+  it("enables Save just the same when a description is present", async () => {
+    const { service } = await renderButtons({
+      ...TASK_WITHOUT_DESCRIPTION,
+      description: "does something useful",
+    });
+
+    editSomethingElse(service as never);
+
+    await waitFor(() => expect(saveButton()).not.toBeDisabled());
+  });
+
+  it("still disables Save when nothing has been edited", async () => {
+    // The remaining reason Save is blocked, so the test above is not passing
+    // for want of any gate at all.
+    await renderButtons(TASK_WITHOUT_DESCRIPTION);
+
+    await waitFor(() => expect(saveButton()).toBeDisabled());
+  });
+
+  it("clearing the description does not disable Save", async () => {
+    const { service } = await renderButtons({
+      ...TASK_WITHOUT_DESCRIPTION,
+      description: "about to be removed",
+    });
+
+    service.children.get(TASK_FORM_MACHINE_ID)!.send({
+      type: TaskDefinitionFormEventType.HANDLE_CHANGE_TASK_FORM,
+      name: "description",
+      value: "",
+    });
+
+    await waitFor(() => expect(saveButton()).not.toBeDisabled());
+  });
+});

ui-next/src/pages/definition/task/TaskDefinitionButtons.tsx

@@ -9,7 +9,10 @@ import TrashIcon from "components/icons/TrashIcon";
 import XCloseIcon from "components/icons/XCloseIcon";
 import fastDeepEqual from "fast-deep-equal";
 import { TaskDefinitionFormMachineEvent } from "pages/definition/task/form/state/types";
-import { TASK_FORM_MACHINE_ID } from "pages/definition/task/state/helpers";
+import {
+  TASK_FORM_MACHINE_ID,
+  isSaveDisabled,
+} from "pages/definition/task/state/helpers";
 import { useTaskDefinition } from "pages/definition/task/state/hook";
 import {
   TaskDefinitionButtonsProps,
@@ -41,18 +44,14 @@ const withFormState =
       [modifiedTaskDefinition, originTaskDefinition],
     );
     const isReset = buttonProps?.role === "reset";
-    const resetDisabledConditions = noChanges;
-    const saveDisabledConditions =
-      (!isNewTaskDef && noChanges) || isTrialExpired;
-    const noDescription = !(modifiedTaskDefinition.description ?? "").trim();
 
     return (
       <ButtonComponent
         {...buttonProps}
         disabled={
           isReset
-            ? resetDisabledConditions
-            : saveDisabledConditions || noDescription
+            ? noChanges
+            : isSaveDisabled({ noChanges, isNewTaskDef, isTrialExpired })
         }
       />
     );
@@ -82,18 +81,19 @@ const withEditorState =
     );
 
     const isReset = buttonProps?.role === "reset";
-    const resetDisabledConditions = noChanges;
-    const saveDisabledConditions =
-      jsonInvalid || (!isNewTaskDef && noChanges) || isTrialExpired;
-    const noDescription = !(modifiedTaskDefinition.description ?? "").trim();
 
     return (
       <ButtonComponent
         {...buttonProps}
         disabled={
           isReset
-            ? resetDisabledConditions
-            : saveDisabledConditions || noDescription
+            ? noChanges
+            : isSaveDisabled({
+                noChanges,
+                isNewTaskDef,
+                isTrialExpired,
+                jsonInvalid,
+              })
         }
       />
     );

ui-next/src/pages/definition/task/form/TaskDefinitionForm.test.tsx

@@ -0,0 +1,150 @@
+/**
+ * The Task tab must expose every field the Code tab's JSON carries, otherwise
+ * a setting can only be reached by hand-editing raw JSON. These tests cover
+ * the four that had no control: maxRetryDelaySeconds, backoffJitterMs,
+ * totalTimeoutSeconds and taskStatusListenerEnabled.
+ *
+ * They assert against the form machine's context, which is what the Code tab
+ * renders — so a control wired to a misspelled field name fails here. The
+ * compiler cannot catch that: modifiedTaskDefinition reaches the form through
+ * a loosely typed xstate ActorRef and is effectively `any`.
+ */
+import "@testing-library/jest-dom";
+import { fireEvent, render, screen } from "@testing-library/react";
+import { interpret } from "xstate";
+import { Provider as ThemeProvider } from "theme/material/provider";
+import TaskDefinitionForm from "pages/definition/task/form/TaskDefinitionForm";
+import { taskDefinitionFormMachine } from "pages/definition/task/form/state/machine";
+import { TaskRetryLogic, TaskTimeoutPolicy } from "pages/definition/task/state";
+import { TaskDefinitionDto } from "types/TaskDefinition";
+
+vi.mock("utils/query", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("utils/query")>()),
+  useFetch: () => ({ data: [], refetch: vi.fn() }),
+}));
+
+const baseTaskDefinition = {
+  name: "my_task",
+  description: "",
+  retryCount: 3,
+  retryDelaySeconds: 60,
+  retryLogic: TaskRetryLogic.FIXED,
+  backoffScaleFactor: 1,
+  timeoutSeconds: 3600,
+  timeoutPolicy: TaskTimeoutPolicy.TIME_OUT_WF,
+  responseTimeoutSeconds: 600,
+  pollTimeoutSeconds: 3600,
+  rateLimitPerFrequency: 0,
+  rateLimitFrequencyInSeconds: 1,
+  concurrentExecLimit: 0,
+  inputKeys: [],
+  outputKeys: [],
+  inputTemplate: {},
+} as unknown as TaskDefinitionDto;
+
+const renderForm = (overrides: Partial<TaskDefinitionDto> = {}) => {
+  const taskDefinition = {
+    ...baseTaskDefinition,
+    ...overrides,
+  } as TaskDefinitionDto;
+  const service = interpret(
+    taskDefinitionFormMachine.withContext({
+      modifiedTaskDefinition: taskDefinition,
+      originTaskDefinition: taskDefinition,
+    }),
+  ).start();
+
+  render(
+    <ThemeProvider>
+      <TaskDefinitionForm formActor={service as never} />
+    </ThemeProvider>,
+  );
+
+  return {
+    /** What the Code tab would show. */
+    definition: () =>
+      service.getSnapshot().context.modifiedTaskDefinition as Record<
+        string,
+        unknown
+      >,
+    json: () =>
+      (
+        service.getSnapshot().context as unknown as {
+          modifiedTaskDefinitionString?: string;
+        }
+      ).modifiedTaskDefinitionString ?? "",
+  };
+};
+
+const field = (label: string) =>
+  screen.getByLabelText(label) as HTMLInputElement;
+
+const setNumber = (label: string, value: string) =>
+  fireEvent.change(field(label), { target: { value } });
+
+describe("TaskDefinitionForm — fields that were JSON-only", () => {
+  it("shows the stored values for all four fields", () => {
+    renderForm({
+      maxRetryDelaySeconds: 120,
+      backoffJitterMs: 250,
+      totalTimeoutSeconds: 7200,
+      taskStatusListenerEnabled: false,
+    });
+
+    expect(field("Max retry delay seconds")).toHaveValue("120");
+    expect(field("Backoff jitter ms")).toHaveValue("250");
+    expect(field("Total Timeout Seconds")).toHaveValue("7200");
+    expect(
+      screen.getByLabelText("Enable task status listener"),
+    ).not.toBeChecked();
+  });
+
+  it("writes maxRetryDelaySeconds back into the definition", () => {
+    const { definition, json } = renderForm({ maxRetryDelaySeconds: 0 });
+
+    setNumber("Max retry delay seconds", "45");
+
+    expect(definition().maxRetryDelaySeconds).toBe(45);
+    expect(json()).toContain('"maxRetryDelaySeconds": 45');
+  });
+
+  it("writes backoffJitterMs back into the definition", () => {
+    const { definition } = renderForm({ backoffJitterMs: 0 });
+
+    setNumber("Backoff jitter ms", "500");
+
+    expect(definition().backoffJitterMs).toBe(500);
+  });
+
+  it("writes totalTimeoutSeconds back into the definition", () => {
+    const { definition } = renderForm({ totalTimeoutSeconds: 0 });
+
+    setNumber("Total Timeout Seconds", "900");
+
+    expect(definition().totalTimeoutSeconds).toBe(900);
+  });
+
+  it("toggles taskStatusListenerEnabled", () => {
+    const { definition } = renderForm({ taskStatusListenerEnabled: true });
+
+    fireEvent.click(screen.getByLabelText("Enable task status listener"));
+
+    expect(definition().taskStatusListenerEnabled).toBe(false);
+  });
+
+  it("reads an absent taskStatusListenerEnabled as on, matching the server default", () => {
+    renderForm();
+
+    expect(screen.getByLabelText("Enable task status listener")).toBeChecked();
+  });
+
+  it("keeps the retry delay cap and jitter editable under a FIXED retry policy", () => {
+    // Both are applied by the server for every retry policy, unlike the
+    // backoff scale factor, which only applies to the backoff policies.
+    renderForm({ retryLogic: TaskRetryLogic.FIXED });
+
+    expect(field("Max retry delay seconds")).toBeEnabled();
+    expect(field("Backoff jitter ms")).toBeEnabled();
+    expect(field("Backoff scale factor")).toBeDisabled();
+  });
+});