From 855df78912621908e13e6409a542106c0147ee2e Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Fri, 3 Jul 2026 15:52:27 +0800 Subject: [PATCH 1/6] UI: Fix partition key display and input handling Non-partitioned Dag runs showed a dangling "Partition key:" label, an empty partition key typed then cleared was sent to the trigger/materialize APIs and rejected with 400, the trigger form exposed the field even for non-partitioned Dags, the manual asset-event partition key used a JsonEditor for a plain string, and the runs table hid the partition_key column even when its filter was shown for a partitioned Dag. Also unify the partition key label so cron/runtime-partitioned runs no longer say "Mapped Partition key". --- .../ui/public/i18n/locales/en/components.json | 1 + .../public/i18n/locales/zh-TW/components.json | 1 + .../ui/src/components/Assets/AssetEvent.tsx | 2 +- .../TriggerDag/TriggerDAGAdvancedOptions.tsx | 38 +++++++++------- .../TriggerDag/TriggerDAGForm.test.tsx | 44 +++++++++++++++++++ .../components/TriggerDag/TriggerDAGForm.tsx | 2 +- .../src/airflow/ui/src/mocks/handlers/dag.ts | 18 ++++++++ .../src/pages/Asset/CreateAssetEventModal.tsx | 15 +++++-- .../ui/src/pages/DagRuns/DagRuns.test.tsx | 34 ++++++++++++-- .../airflow/ui/src/pages/DagRuns/DagRuns.tsx | 30 ++++++++++++- .../src/airflow/ui/src/pages/Run/Header.tsx | 2 +- .../src/airflow/ui/src/queries/useTrigger.ts | 5 ++- 12 files changed, 161 insertions(+), 31 deletions(-) diff --git a/airflow-core/src/airflow/ui/public/i18n/locales/en/components.json b/airflow-core/src/airflow/ui/public/i18n/locales/en/components.json index 350e3898a39ee..10351910eee16 100644 --- a/airflow-core/src/airflow/ui/public/i18n/locales/en/components.json +++ b/airflow-core/src/airflow/ui/public/i18n/locales/en/components.json @@ -136,6 +136,7 @@ "loading": "Loading Dag information...", "loadingFailed": "Failed to load Dag information. Please try again.", "manualRunDenied": "Manual runs are not allowed for this Dag", + "partitionKeyHelp": "Optional - only applies to partitioned Dags", "runIdHelp": "Optional - will be generated if not provided", "selectDescription": "Trigger a single run of this Dag", "selectLabel": "Single Run", diff --git a/airflow-core/src/airflow/ui/public/i18n/locales/zh-TW/components.json b/airflow-core/src/airflow/ui/public/i18n/locales/zh-TW/components.json index 44889c7d2ce7c..b78fe102f45e1 100644 --- a/airflow-core/src/airflow/ui/public/i18n/locales/zh-TW/components.json +++ b/airflow-core/src/airflow/ui/public/i18n/locales/zh-TW/components.json @@ -136,6 +136,7 @@ "loading": "正在載入 Dag 資訊...", "loadingFailed": "載入 Dag 資訊失敗,請重試。", "manualRunDenied": "此 Dag 不允許手動執行", + "partitionKeyHelp": "選填 - 僅適用於分區的 Dag", "runIdHelp": "選填 - 若未提供將會自動產生", "selectDescription": "觸發此 Dag 單次執行", "selectLabel": "單次執行", diff --git a/airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.tsx b/airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.tsx index b7d4b62d265e2..ba6eb565f64b8 100644 --- a/airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.tsx +++ b/airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.tsx @@ -95,7 +95,7 @@ export const AssetEvent = ({ - {event.partition_key === undefined ? undefined : ( + {event.partition_key === undefined || event.partition_key === null ? undefined : ( {rootTranslate("dagRun.partitionKey")}: {event.partition_key} diff --git a/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGAdvancedOptions.tsx b/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGAdvancedOptions.tsx index 77d16664e5297..d5512579093f0 100644 --- a/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGAdvancedOptions.tsx +++ b/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGAdvancedOptions.tsx @@ -25,9 +25,10 @@ import type { DagRunTriggerParams } from "./types"; type TriggerDAGAdvancedOptionsProps = { readonly control: Control; + readonly isPartitioned: boolean; }; -const TriggerDAGAdvancedOptions = ({ control }: TriggerDAGAdvancedOptionsProps) => { +const TriggerDAGAdvancedOptions = ({ control, isPartitioned }: TriggerDAGAdvancedOptionsProps) => { const { t: translate } = useTranslation(["common", "components"]); const { t: rootTranslate } = useTranslation(); @@ -51,22 +52,25 @@ const TriggerDAGAdvancedOptions = ({ control }: TriggerDAGAdvancedOptionsProps) )} /> - ( - - - - {rootTranslate("dagRun.partitionKey")} - - - - - - - )} - /> + {isPartitioned ? ( + ( + + + + {rootTranslate("dagRun.partitionKey")} + + + + + {translate("components:triggerDag.partitionKeyHelp")} + + + )} + /> + ) : undefined} { expect(configJson.value).toContain('"Updated message"'); }); }); + + it("hides the partition key field for non-partitioned Dags", async () => { + render( + , + { wrapper: Wrapper }, + ); + + fireEvent.click(screen.getByText("Advanced Options")); + + await waitFor(() => expect(screen.getByText("runId")).toBeInTheDocument()); + expect(screen.queryByText("dagRun.partitionKey")).not.toBeInTheDocument(); + }); + + it("shows the partition key field for partitioned Dags", async () => { + render( + , + { wrapper: Wrapper }, + ); + + fireEvent.click(screen.getByText("Advanced Options")); + + await waitFor(() => expect(screen.getByText("dagRun.partitionKey")).toBeInTheDocument()); + expect(screen.getByText("components:triggerDag.partitionKeyHelp")).toBeInTheDocument(); + }); }); diff --git a/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx b/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx index 3cac3a7edc113..ccd79c4fc25f9 100644 --- a/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx +++ b/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx @@ -244,7 +244,7 @@ const TriggerDAGForm = ({ setErrors={setErrors} setFormError={setFormError} > - + diff --git a/airflow-core/src/airflow/ui/src/mocks/handlers/dag.ts b/airflow-core/src/airflow/ui/src/mocks/handlers/dag.ts index 7fbe561e4546b..ca0773b3cac7e 100644 --- a/airflow-core/src/airflow/ui/src/mocks/handlers/dag.ts +++ b/airflow-core/src/airflow/ui/src/mocks/handlers/dag.ts @@ -54,10 +54,28 @@ export const MOCK_DAG = { tags: [{ dag_id: "tutorial_taskflow_api", name: "example" }], template_search_path: null, timetable_description: "Never, external triggers only", + timetable_partitioned: false, timetable_summary: null, timezone: "UTC", }; +// Matches the dagId used by the dag_runs mock handler (see src/mocks/handlers/dag_runs.ts), +// which returns the same static runs regardless of dagId. +export const MOCK_NON_PARTITIONED_DAG = { + ...MOCK_DAG, + dag_display_name: "test_dag", + dag_id: "test_dag", +}; + +export const MOCK_PARTITIONED_DAG = { + ...MOCK_DAG, + dag_display_name: "test_partitioned_dag", + dag_id: "test_partitioned_dag", + timetable_partitioned: true, +}; + export const handlers: Array = [ http.get("/api/v2/dags/tutorial_taskflow_api/details", () => HttpResponse.json(MOCK_DAG)), + http.get("/api/v2/dags/test_dag/details", () => HttpResponse.json(MOCK_NON_PARTITIONED_DAG)), + http.get("/api/v2/dags/test_partitioned_dag/details", () => HttpResponse.json(MOCK_PARTITIONED_DAG)), ]; diff --git a/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx index 24d7d98755a48..d7729c8bb8d7b 100644 --- a/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx +++ b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { Button, Field, Heading, HStack, Text, VStack } from "@chakra-ui/react"; +import { Button, Field, Heading, HStack, Input, Text, VStack } from "@chakra-ui/react"; import { useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; import { useTranslation } from "react-i18next"; @@ -158,7 +158,10 @@ export const CreateAssetEventModal = ({ asset, onClose, open }: Props) => { data_interval_start: dataIntervalStart?.toISOString() ?? null, logical_date: logicalDate?.toISOString() ?? null, note: dagRunRequestBody.note === "" ? undefined : dagRunRequestBody.note, - partition_key: dagRunRequestBody.partitionKey ?? null, + partition_key: + dagRunRequestBody.partitionKey === undefined || dagRunRequestBody.partitionKey === "" + ? null + : dagRunRequestBody.partitionKey, }; materializeAsset({ @@ -172,7 +175,7 @@ export const CreateAssetEventModal = ({ asset, onClose, open }: Props) => { requestBody: { asset_id: asset.id, extra: JSON.parse(extra) as Record, - partition_key: partitionKey ?? null, + partition_key: partitionKey === undefined || partitionKey === "" ? null : partitionKey, }, }); @@ -226,7 +229,11 @@ export const CreateAssetEventModal = ({ asset, onClose, open }: Props) => { <> {translate("common:dagRun.partitionKey")} - + setPartitionKey(event.target.value)} + size="sm" + value={partitionKey ?? ""} + /> diff --git a/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.test.tsx b/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.test.tsx index 3b9b19e0eabef..6838c27eea4a2 100644 --- a/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.test.tsx +++ b/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.test.tsx @@ -30,6 +30,10 @@ vi.mock("src/components/RenderedJsonField", () => ({ ), })); +afterEach(() => { + localStorage.clear(); +}); + // The dag_runs mock handler (see src/mocks/handlers/dag_runs.ts) returns: // - run_before_filter (logical_date: 2024-12-31) — excluded when filtering Jan 2025 // - run_in_range (logical_date: 2025-01-15) — included when filtering Jan 2025 @@ -64,10 +68,6 @@ describe("DagRuns conf expand/collapse", () => { ); }); - afterEach(() => { - globalThis.localStorage.clear(); - }); - it("toggles conf JSON collapse state via the expand/collapse all buttons", async () => { render(); @@ -86,3 +86,29 @@ describe("DagRuns conf expand/collapse", () => { ); }); }); + +// See src/mocks/handlers/dag.ts for the partitioned/non-partitioned dag details mocks. +describe("DagRuns partition_key column visibility", () => { + it("hides the partition_key column by default in the global (non-Dag-scoped) runs list", async () => { + render(); + + await waitFor(() => expect(screen.getByText("run_in_range")).toBeInTheDocument()); + expect(screen.queryByText("dagRun.partitionKey")).not.toBeInTheDocument(); + }); + + it("hides the partition_key column by default for a non-partitioned Dag's runs tab", async () => { + render(); + + await waitFor(() => expect(screen.getByText("run_in_range")).toBeInTheDocument()); + expect(screen.queryByText("dagRun.partitionKey")).not.toBeInTheDocument(); + }); + + it("shows the partition_key column by default for a partitioned Dag's runs tab", async () => { + render(); + + await waitFor(() => expect(screen.getByText("run_in_range")).toBeInTheDocument()); + // The column's default visibility depends on the Dag details fetch settling, which can + // resolve slightly after the runs themselves, so wait for it separately. + await waitFor(() => expect(screen.getByText("dagRun.partitionKey")).toBeInTheDocument()); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx b/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx index d9efe994719c7..4450c374d4908 100644 --- a/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx +++ b/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx @@ -19,10 +19,11 @@ import { Flex, HStack, Text, useDisclosure } from "@chakra-ui/react"; import type { ColumnDef } from "@tanstack/react-table"; import type { TFunction } from "i18next"; +import { useRef } from "react"; import { useTranslation } from "react-i18next"; import { useParams, useSearchParams } from "react-router-dom"; -import { useDagRunServiceGetDagRuns } from "openapi/queries"; +import { useDagRunServiceGetDagRuns, useDagServiceGetDagDetails } from "openapi/queries"; import type { DAGRunResponse } from "openapi/requests/types.gen"; import { ClearRunButton } from "src/components/Clear"; import { DagVersion } from "src/components/DagVersion"; @@ -230,11 +231,32 @@ export const DagRuns = () => { const [searchParams] = useSearchParams(); const { onClose, onOpen, open } = useDisclosure(); + const { data: dag } = useDagServiceGetDagDetails({ dagId: dagId ?? "" }, undefined, { + enabled: Boolean(dagId), + }); + + // The Dag details fetch resolves after this component's first render, but the underlying + // column-visibility default is only read once, on mount — so latch the first known answer + // (reset when dagId actually changes) instead of a plain derived value that could revert to + // "unknown" on an unrelated re-render and flip the column back to hidden. + const dagIdRef = useRef(dagId); + const isPartitionedRef = useRef(undefined); + + if (Boolean(dagId) && dagIdRef.current !== dagId) { + dagIdRef.current = dagId; + isPartitionedRef.current = undefined; + } + if (dag !== undefined && isPartitionedRef.current === undefined) { + isPartitionedRef.current = dag.timetable_partitioned; + } + + const isPartitioned = isPartitionedRef.current ?? false; + const { setTableURLState, tableURLState } = useTableURLState({ columnVisibility: { dag_version: false, end_date: false, - partition_key: false, + partition_key: isPartitioned, }, }); const { cursor, pagination, sorting } = tableURLState; @@ -367,6 +389,10 @@ export const DagRuns = () => { errorMessage={} initialState={tableURLState} isLoading={isLoading} + // Remounts once the Dag's partitioned status is known, so the partition_key column's + // default visibility (set once, on mount, by the underlying column-visibility storage) + // reflects it instead of the pre-load "not partitioned" assumption. + key={`${dagId ?? "all"}-${isPartitionedRef.current === undefined ? "pending" : String(isPartitionedRef.current)}`} modelName="common:dagRun" nextCursor={nextCursor} onStateChange={setTableURLState} diff --git a/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx b/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx index acba855305f47..c31db0e9ac0c9 100644 --- a/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx +++ b/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx @@ -75,7 +75,7 @@ export const Header = ({ dagRun }: { readonly dagRun: DAGRunResponse }) => { ? [] : [ { - label: translate("dagRun.mappedPartitionKey"), + label: translate("dagRun.partitionKey"), value: dagRun.partition_key, }, ]), diff --git a/airflow-core/src/airflow/ui/src/queries/useTrigger.ts b/airflow-core/src/airflow/ui/src/queries/useTrigger.ts index d96fbf45cd78f..84afb11d60ec4 100644 --- a/airflow-core/src/airflow/ui/src/queries/useTrigger.ts +++ b/airflow-core/src/airflow/ui/src/queries/useTrigger.ts @@ -108,7 +108,10 @@ export const useTrigger = ({ dagId, onSuccessConfirm }: { dagId: string; onSucce data_interval_start: formattedDataIntervalStart, logical_date: formattedLogicalDate, note: checkNote, - partition_key: dagRunRequestBody.partitionKey ?? null, + partition_key: + dagRunRequestBody.partitionKey === undefined || dagRunRequestBody.partitionKey === "" + ? null + : dagRunRequestBody.partitionKey, }, }); }; From 99cea9bc419dc488ecd1b937c2c745566525532e Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Fri, 10 Jul 2026 19:00:59 +0800 Subject: [PATCH 2/6] fixup! UI: Fix partition key display and input handling --- .../src/components/Assets/AssetEvent.test.tsx | 48 ++++ .../Asset/CreateAssetEventModal.test.tsx | 222 ++++++++++++++++++ .../airflow/ui/src/pages/Run/Header.test.tsx | 94 ++++++++ 3 files changed, 364 insertions(+) create mode 100644 airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.test.tsx create mode 100644 airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx create mode 100644 airflow-core/src/airflow/ui/src/pages/Run/Header.test.tsx diff --git a/airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.test.tsx b/airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.test.tsx new file mode 100644 index 0000000000000..c8856d0eab90e --- /dev/null +++ b/airflow-core/src/airflow/ui/src/components/Assets/AssetEvent.test.tsx @@ -0,0 +1,48 @@ +/*! + * 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 "@testing-library/jest-dom"; +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import type { AssetEventResponse } from "openapi/requests/types.gen"; +import { Wrapper } from "src/utils/Wrapper"; + +import { AssetEvent } from "./AssetEvent"; + +const baseEvent = { + asset_id: 1, + created_dagruns: [], + id: 1, + source_map_index: -1, + timestamp: "2025-01-01T00:00:00Z", +} satisfies Partial as AssetEventResponse; + +describe("AssetEvent", () => { + it("does not render a partition key line when partition_key is null", () => { + render(, { wrapper: Wrapper }); + + expect(screen.queryByText(/dagRun\.partitionKey/u)).not.toBeInTheDocument(); + }); + + it("renders the partition key line when partition_key is set", () => { + render(, { wrapper: Wrapper }); + + expect(screen.getByText("dagRun.partitionKey: foo")).toBeInTheDocument(); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx new file mode 100644 index 0000000000000..a7e91e64c05f1 --- /dev/null +++ b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx @@ -0,0 +1,222 @@ +/*! + * 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 "@testing-library/jest-dom"; +import { fireEvent, render, screen } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type * as OpenapiQueries from "openapi/queries"; +import type { AssetResponse, DAGDetailsResponse } from "openapi/requests/types.gen"; +import type { DagRunTriggerParams } from "src/components/TriggerDag/types"; +import type * as Ui from "src/components/ui"; +import { Wrapper } from "src/utils/Wrapper"; + +import { CreateAssetEventModal } from "./CreateAssetEventModal"; + +const materializeSubmitParams = vi.hoisted(() => ({ + conf: "{}", + dagRunId: "", + dataIntervalEnd: "", + dataIntervalMode: "auto", + dataIntervalStart: "", + logicalDate: "", + note: "", + partitionKey: undefined, +})); + +vi.mock("src/components/ui", async (importOriginal) => { + const actual = await importOriginal(); + // Must stay inside the factory: vitest hoists vi.mock above module scope, so an outer-scope + // component cannot be referenced here. + // eslint-disable-next-line unicorn/consistent-function-scoping + const DialogPart = ({ children }: { readonly children?: ReactNode }) =>
{children}
; + + return { + ...actual, + Dialog: { + ...actual.Dialog, + Body: DialogPart, + CloseTrigger: () => undefined, + Content: DialogPart, + Footer: DialogPart, + Header: DialogPart, + Root: ({ children, open }: { readonly children?: ReactNode; readonly open?: boolean }) => + open ?
{children}
: undefined, + }, + }; +}); + +vi.mock("src/components/JsonEditor", () => ({ + JsonEditor: ({ value = "{}" }: { readonly value?: string }) => ( +