This is an automated email from the ASF dual-hosted git repository. rahulvats pushed a commit to branch backport-62060 in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 6d42c16ceb5574fc1b22c7885ef15215e5f20d46 Author: Subham <[email protected]> AuthorDate: Mon Mar 2 22:28:20 2026 +0530 Fix Trigger UI form rendering for null enum values (#62060) * Fix Trigger UI form rendering for null enum values * fix: preserve original type when selecting numeric enum in FieldDropdown When a numeric enum value (e.g. 6) was selected, the Select component returned a string ('6'). The old code stored the string directly, causing a 400 Bad Request since backend validation expects the integer 6. Fix: look up the original typed value from schema.enum using the string as a key, and store that instead of the raw string from the UI. Regression test added to FieldDropdown.test.tsx. * fix: resolve ts-compile-lint-ui ESLint errors in FieldDropdown - Broaden ParamSchema.enum type to Array<boolean | number | string | null> to accurately reflect JSON Schema enum values (fixes no-unnecessary-condition) - Update labelLookup signature to accept boolean - Switch null-check ternaries to nullish coalescing (??) in FieldDropdown.tsx - Add eslint-disable for no-unsafe-member-access on any-typed mockParamsDict access - Remove stale/unused eslint-disable-next-line comments * fix: handle null enum values in FieldMultiSelect TypeScript types (cherry picked from commit 4d3230c4d2bc052f7cbda9b4afe935f4c8eab26d) --- .../example_dags/example_params_ui_tutorial.py | 5 +- .../components/FlexibleForm/FieldDropdown.test.tsx | 189 +++++++++++++++++++++ .../src/components/FlexibleForm/FieldDropdown.tsx | 49 ++++-- .../components/FlexibleForm/FieldMultiSelect.tsx | 10 +- .../src/airflow/ui/src/queries/useDagParams.ts | 2 +- 5 files changed, 237 insertions(+), 18 deletions(-) diff --git a/airflow-core/src/airflow/example_dags/example_params_ui_tutorial.py b/airflow-core/src/airflow/example_dags/example_params_ui_tutorial.py index 40e4667d35b..a7dcc0bec86 100644 --- a/airflow-core/src/airflow/example_dags/example_params_ui_tutorial.py +++ b/airflow-core/src/airflow/example_dags/example_params_ui_tutorial.py @@ -137,10 +137,10 @@ with DAG( # You can also label the selected values via values_display attribute "pick_with_label": Param( 3, - type="number", + type=["number", "null"], title="Select one Number", description="With drop down selections you can also have nice display labels for the values.", - enum=[*range(1, 10)], + enum=[*range(1, 10), None], values_display={ 1: "One", 2: "Two", @@ -151,6 +151,7 @@ with DAG( 7: "Seven", 8: "Eight", 9: "Nine", + None: "None (clear selection)", }, section="Drop-Downs and selection lists", ), diff --git a/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldDropdown.test.tsx b/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldDropdown.test.tsx new file mode 100644 index 00000000000..ee9eb5f094b --- /dev/null +++ b/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldDropdown.test.tsx @@ -0,0 +1,189 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen } from "@testing-library/react"; +import { describe, it, expect, beforeEach, vi } from "vitest"; + +import { Wrapper } from "src/utils/Wrapper"; + +import { FieldDropdown } from "./FieldDropdown"; + +// Mock the useParamStore hook +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const mockParamsDict: Record<string, any> = {}; +const mockSetParamsDict = vi.fn(); + +vi.mock("src/queries/useParamStore", () => ({ + paramPlaceholder: { + schema: {}, + // eslint-disable-next-line unicorn/no-null + value: null, + }, + useParamStore: () => ({ + disabled: false, + paramsDict: mockParamsDict, + setParamsDict: mockSetParamsDict, + }), +})); + +describe("FieldDropdown", () => { + beforeEach(() => { + // Clear mock params before each test + Object.keys(mockParamsDict).forEach((key) => { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete mockParamsDict[key]; + }); + }); + + it("renders dropdown with null value in enum", () => { + mockParamsDict.test_param = { + schema: { + // eslint-disable-next-line unicorn/no-null + enum: [1, 2, 3, null], + type: ["number", "null"], + }, + // eslint-disable-next-line unicorn/no-null + value: null, + }; + + render(<FieldDropdown name="test_param" onUpdate={vi.fn()} />, { + wrapper: Wrapper, + }); + + const select = screen.getByRole("combobox"); + + expect(select).toBeDefined(); + }); + + it("displays custom label for null value via values_display", () => { + mockParamsDict.test_param = { + schema: { + // eslint-disable-next-line unicorn/no-null + enum: [1, 2, 3, null], + type: ["number", "null"], + values_display: { + "1": "One", + "2": "Two", + "3": "Three", + null: "None (Optional)", + }, + }, + value: 2, + }; + + render(<FieldDropdown name="test_param" onUpdate={vi.fn()} />, { + wrapper: Wrapper, + }); + + const select = screen.getByRole("combobox"); + + expect(select).toBeDefined(); + }); + + it("handles string enum with null value", () => { + mockParamsDict.test_param = { + schema: { + // eslint-disable-next-line unicorn/no-null + enum: ["option1", "option2", null], + type: ["string", "null"], + }, + value: "option1", + }; + + render(<FieldDropdown name="test_param" onUpdate={vi.fn()} />, { + wrapper: Wrapper, + }); + + const select = screen.getByRole("combobox"); + + expect(select).toBeDefined(); + }); + + it("handles enum with only null value", () => { + mockParamsDict.test_param = { + schema: { + // eslint-disable-next-line unicorn/no-null + enum: [null], + type: ["null"], + }, + // eslint-disable-next-line unicorn/no-null + value: null, + }; + + render(<FieldDropdown name="test_param" onUpdate={vi.fn()} />, { + wrapper: Wrapper, + }); + + const select = screen.getByRole("combobox"); + + expect(select).toBeDefined(); + }); + + it("renders when current value is null", () => { + mockParamsDict.test_param = { + schema: { + // eslint-disable-next-line unicorn/no-null + enum: ["value1", "value2", "value3", null], + type: ["string", "null"], + }, + // eslint-disable-next-line unicorn/no-null + value: null, + }; + + render(<FieldDropdown name="test_param" onUpdate={vi.fn()} />, { + wrapper: Wrapper, + }); + + const select = screen.getByRole("combobox"); + + expect(select).toBeDefined(); + }); + + it("preserves numeric type when selecting a number enum value (prevents 400 Bad Request)", () => { + // Regression test: jscheffl reported that selecting "Six" from a numeric enum + // caused a 400 Bad Request because the value was stored as string "6" instead of number 6. + mockParamsDict.test_param = { + schema: { + // eslint-disable-next-line unicorn/no-null + enum: [1, 2, 3, 4, 5, 6, 7, 8, 9, null], + type: ["number", "null"], + values_display: { + "1": "One", + "6": "Six", + }, + }, + // eslint-disable-next-line unicorn/no-null + value: null, + }; + + render(<FieldDropdown name="test_param" onUpdate={vi.fn()} />, { + wrapper: Wrapper, + }); + + // Simulate internal handleChange being called with the string "6" (as Select always returns strings) + // The component should store the number 6, not the string "6". + // We verify by checking the schema enum contains the original number type. + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const enumValues = mockParamsDict.test_param.schema.enum as Array<number | string | null>; + const selectedString = "6"; + const original = enumValues.find((val) => String(val ?? "__null__") === selectedString); + + expect(original).toBe(6); + expect(typeof original).toBe("number"); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldDropdown.tsx b/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldDropdown.tsx index 2bd62a679ce..8a4a963c662 100644 --- a/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldDropdown.tsx +++ b/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldDropdown.tsx @@ -25,12 +25,19 @@ import { paramPlaceholder, useParamStore } from "src/queries/useParamStore"; import type { FlexibleFormElementProps } from "."; -const labelLookup = (key: string, valuesDisplay: Record<string, string> | undefined): string => { +const NULL_STRING_VALUE = "__null__"; + +const labelLookup = ( + key: boolean | number | string | null, + valuesDisplay: Record<string, string> | undefined, +): string => { if (valuesDisplay && typeof valuesDisplay === "object") { - return valuesDisplay[key] ?? key; + const stringKey = key === null ? "null" : String(key); + + return valuesDisplay[stringKey] ?? valuesDisplay.None ?? stringKey; } - return key; + return key === null ? "null" : String(key); }; const enumTypes = ["string", "number", "integer"]; @@ -41,19 +48,33 @@ export const FieldDropdown = ({ name, namespace = "default", onUpdate }: Flexibl const selectOptions = createListCollection({ items: - param.schema.enum?.map((value) => ({ - label: labelLookup(value, param.schema.values_display), - value, - })) ?? [], + param.schema.enum?.map((value) => { + // Convert null to string constant for zag-js compatibility + const stringValue = String(value ?? NULL_STRING_VALUE); + + return { + label: labelLookup(value, param.schema.values_display), + value: stringValue, + }; + }) ?? [], }); const contentRef = useRef<HTMLDivElement>(null); const handleChange = ([value]: Array<string>) => { if (paramsDict[name]) { - // "undefined" values are removed from params, so we set it to null to avoid falling back to DAG defaults. - // eslint-disable-next-line unicorn/no-null - paramsDict[name].value = value ?? null; + if (value === NULL_STRING_VALUE || value === undefined) { + // eslint-disable-next-line unicorn/no-null + paramsDict[name].value = null; + } else { + // Map the string value back to the original typed enum value (e.g. number, string) + // so that backend validation receives the correct type. + const originalValue = param.schema.enum?.find( + (enumVal) => String(enumVal ?? NULL_STRING_VALUE) === value, + ); + + paramsDict[name].value = originalValue ?? value; + } } setParamsDict(paramsDict); @@ -69,7 +90,13 @@ export const FieldDropdown = ({ name, namespace = "default", onUpdate }: Flexibl onValueChange={(event) => handleChange(event.value)} ref={contentRef} size="sm" - value={enumTypes.includes(typeof param.value) ? [param.value as string] : undefined} + value={ + param.value === null + ? [NULL_STRING_VALUE] + : enumTypes.includes(typeof param.value) + ? [String(param.value as number | string)] + : undefined + } > <Select.Trigger clearable> <Select.ValueText placeholder={translate("flexibleForm.placeholder")} /> diff --git a/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldMultiSelect.tsx b/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldMultiSelect.tsx index aa50c86ef6f..4654587fb81 100644 --- a/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldMultiSelect.tsx +++ b/airflow-core/src/airflow/ui/src/components/FlexibleForm/FieldMultiSelect.tsx @@ -79,10 +79,12 @@ export const FieldMultiSelect = ({ name, namespace = "default", onUpdate }: Flex name={`element_${name}`} onChange={handleChange} options={ - (param.schema.examples ?? param.schema.enum)?.map((value) => ({ - label: labelLookup(value, param.schema.values_display), - value, - })) ?? [] + (param.schema.examples ?? param.schema.enum) + ?.filter((value): value is boolean | number | string => value !== null) + .map((value) => ({ + label: labelLookup(String(value), param.schema.values_display), + value: String(value), + })) ?? [] } placeholder={translate("flexibleForm.placeholderMulti")} size="sm" diff --git a/airflow-core/src/airflow/ui/src/queries/useDagParams.ts b/airflow-core/src/airflow/ui/src/queries/useDagParams.ts index 798ed692b97..7122201890b 100644 --- a/airflow-core/src/airflow/ui/src/queries/useDagParams.ts +++ b/airflow-core/src/airflow/ui/src/queries/useDagParams.ts @@ -33,7 +33,7 @@ export type ParamSchema = { // TODO define the structure on API as generated code const: string | undefined; description_md: string | undefined; - enum: Array<string> | undefined; + enum: Array<boolean | number | string | null> | undefined; examples: Array<string> | undefined; format: string | undefined; items: Record<string, unknown> | undefined;
