pierrejeambrun commented on code in PR #55811:
URL: https://github.com/apache/airflow/pull/55811#discussion_r2362843852
##########
airflow-core/src/airflow/ui/src/pages/Dag/Tasks/TaskFilters/TaskFilters.tsx:
##########
@@ -78,59 +53,116 @@ export const TaskFilters = ({ tasksData }: { readonly
tasksData: TaskCollectionR
{ key: "true", label: translate("mapped") },
{ key: "false", label: translate("notMapped") },
];
- const taskNamePattern = searchParams.get(NAME_PATTERN) ?? "";
- const handleSearchChange = (value: string) => {
- if (value) {
- searchParams.set(NAME_PATTERN, value);
- } else {
- searchParams.delete(NAME_PATTERN);
+
+ // Convert to FilterBar select option shape (label/value)
+ const operatorOptions = allOperatorNames
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const triggerRuleOptions = allTriggerRules
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const retryValueOptions = allRetryValues
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const mappedOptions = allMappedValues.map(({ key, label }) => ({ label,
value: key }));
+
+ // FilterBar configs (keys unchanged; labels stick to prior wording)
+ const configsUnfiltered: Array<FilterConfig | false> = [
+ {
+ hotkeyDisabled: true,
+ key: NAME_PATTERN,
+ label: translate("searchTasks"),
+ placeholder: translate("searchTasks"),
+ type: "text",
+ },
+ operatorOptions.length > 0 && {
+ key: OPERATOR,
+ label: translate("selectOperator", { defaultValue: "Operators" }),
+ multiple: true,
+ options: operatorOptions,
+ type: "select",
+ },
+ triggerRuleOptions.length > 0 && {
+ key: TRIGGER_RULE,
+ label: translate("selectTriggerRules", { defaultValue: "Trigger rules"
}),
+ multiple: true,
+ options: triggerRuleOptions,
+ type: "select",
+ },
+ retryValueOptions.length > 0 && {
+ key: RETRIES,
+ label: translate("retries", { defaultValue: "Retry values" }),
+ multiple: true,
+ options: retryValueOptions,
+ type: "select",
+ },
+ {
+ key: MAPPED,
+ label: translate("mapped", { defaultValue: "Mapped" }),
+ multiple: false,
+ options: mappedOptions,
+ type: "select",
+ },
+ ];
+
+ const configs: Array<FilterConfig> = configsUnfiltered.filter(Boolean) as
Array<FilterConfig>;
+
+ // Initial values mirror previous local names
+ const initialValues = {
+ [MAPPED]: selectedMapped,
+ [NAME_PATTERN]: taskNamePattern || undefined,
+ [OPERATOR]: selectedOperators.length ? selectedOperators : undefined,
+ [RETRIES]: selectedRetries.length ? selectedRetries : undefined,
+ [TRIGGER_RULE]: selectedTriggerRules.length ? selectedTriggerRules :
undefined,
+ };
+
+ // Single place to write the URL params (replaces the old individual
handlers)
+ const onFiltersChange = (record: Record<string, unknown>): void => {
+ const next = new URLSearchParams(searchParams);
+
+ // clear the filter-related keys before writing
+ [NAME_PATTERN, OPERATOR, TRIGGER_RULE, RETRIES, MAPPED,
"page"].forEach((key) => next.delete(key));
+
+ const name = typeof record[NAME_PATTERN] === "string" ?
record[NAME_PATTERN] : "";
+
+ if (name && String(name).trim() !== "") {
+ next.set(NAME_PATTERN, String(name));
+ }
+
+ const operators = Array.isArray(record[OPERATOR]) ? (record[OPERATOR] as
Array<string>) : [];
+
+ operators.forEach((val) => next.append(OPERATOR, val));
+
+ const triggers = Array.isArray(record[TRIGGER_RULE]) ?
(record[TRIGGER_RULE] as Array<string>) : [];
+
+ triggers.forEach((val) => next.append(TRIGGER_RULE, val));
+
+ const retries = Array.isArray(record[RETRIES]) ? (record[RETRIES] as
Array<string>) : [];
+
+ retries.forEach((val) => next.append(RETRIES, val));
+
+ const mapped = typeof record[MAPPED] === "string" ? record[MAPPED] : "";
+
+ if (mapped && String(mapped).trim() !== "") {
+ next.set(MAPPED, String(mapped));
}
- setSearchParams(searchParams);
+
+ // keep the existing behavior used by tests: reset to page 1 on any filter
change
+ next.set("page", "1");
+
Review Comment:
Also there is a lot of hard casting happening, we might want to take a look
and fix that. (`as`)
##########
airflow-core/src/airflow/ui/src/components/FilterBar/types.ts:
##########
@@ -18,17 +18,24 @@
*/
import type React from "react";
-export type FilterValue = Date | number | string | null | undefined;
+export type FilterValue = Date | number | ReadonlyArray<string> | string |
null | undefined;
Review Comment:
Nice, supporting multi values select is cool. We will need that for other
entities.
##########
airflow-core/src/airflow/ui/src/pages/Dag/Tasks/TaskFilters/TaskFilters.tsx:
##########
@@ -78,59 +53,116 @@ export const TaskFilters = ({ tasksData }: { readonly
tasksData: TaskCollectionR
{ key: "true", label: translate("mapped") },
{ key: "false", label: translate("notMapped") },
];
- const taskNamePattern = searchParams.get(NAME_PATTERN) ?? "";
- const handleSearchChange = (value: string) => {
- if (value) {
- searchParams.set(NAME_PATTERN, value);
- } else {
- searchParams.delete(NAME_PATTERN);
+
+ // Convert to FilterBar select option shape (label/value)
+ const operatorOptions = allOperatorNames
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const triggerRuleOptions = allTriggerRules
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const retryValueOptions = allRetryValues
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const mappedOptions = allMappedValues.map(({ key, label }) => ({ label,
value: key }));
+
+ // FilterBar configs (keys unchanged; labels stick to prior wording)
+ const configsUnfiltered: Array<FilterConfig | false> = [
+ {
+ hotkeyDisabled: true,
+ key: NAME_PATTERN,
+ label: translate("searchTasks"),
+ placeholder: translate("searchTasks"),
+ type: "text",
+ },
+ operatorOptions.length > 0 && {
+ key: OPERATOR,
+ label: translate("selectOperator", { defaultValue: "Operators" }),
+ multiple: true,
+ options: operatorOptions,
+ type: "select",
+ },
+ triggerRuleOptions.length > 0 && {
+ key: TRIGGER_RULE,
+ label: translate("selectTriggerRules", { defaultValue: "Trigger rules"
}),
+ multiple: true,
+ options: triggerRuleOptions,
+ type: "select",
+ },
+ retryValueOptions.length > 0 && {
+ key: RETRIES,
+ label: translate("retries", { defaultValue: "Retry values" }),
+ multiple: true,
+ options: retryValueOptions,
+ type: "select",
+ },
+ {
+ key: MAPPED,
+ label: translate("mapped", { defaultValue: "Mapped" }),
+ multiple: false,
+ options: mappedOptions,
+ type: "select",
+ },
+ ];
+
+ const configs: Array<FilterConfig> = configsUnfiltered.filter(Boolean) as
Array<FilterConfig>;
+
+ // Initial values mirror previous local names
+ const initialValues = {
+ [MAPPED]: selectedMapped,
+ [NAME_PATTERN]: taskNamePattern || undefined,
+ [OPERATOR]: selectedOperators.length ? selectedOperators : undefined,
+ [RETRIES]: selectedRetries.length ? selectedRetries : undefined,
+ [TRIGGER_RULE]: selectedTriggerRules.length ? selectedTriggerRules :
undefined,
+ };
+
+ // Single place to write the URL params (replaces the old individual
handlers)
+ const onFiltersChange = (record: Record<string, unknown>): void => {
+ const next = new URLSearchParams(searchParams);
+
+ // clear the filter-related keys before writing
+ [NAME_PATTERN, OPERATOR, TRIGGER_RULE, RETRIES, MAPPED,
"page"].forEach((key) => next.delete(key));
Review Comment:
Use react hook to get query parameters and path parameters. (useParams,
useSearchParams)
##########
airflow-core/src/airflow/ui/src/pages/Dag/Tasks/TaskFilters/TaskFilters.tsx:
##########
@@ -78,59 +53,116 @@ export const TaskFilters = ({ tasksData }: { readonly
tasksData: TaskCollectionR
{ key: "true", label: translate("mapped") },
{ key: "false", label: translate("notMapped") },
];
- const taskNamePattern = searchParams.get(NAME_PATTERN) ?? "";
- const handleSearchChange = (value: string) => {
- if (value) {
- searchParams.set(NAME_PATTERN, value);
- } else {
- searchParams.delete(NAME_PATTERN);
+
+ // Convert to FilterBar select option shape (label/value)
+ const operatorOptions = allOperatorNames
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const triggerRuleOptions = allTriggerRules
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const retryValueOptions = allRetryValues
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const mappedOptions = allMappedValues.map(({ key, label }) => ({ label,
value: key }));
+
+ // FilterBar configs (keys unchanged; labels stick to prior wording)
+ const configsUnfiltered: Array<FilterConfig | false> = [
+ {
+ hotkeyDisabled: true,
+ key: NAME_PATTERN,
+ label: translate("searchTasks"),
+ placeholder: translate("searchTasks"),
+ type: "text",
+ },
+ operatorOptions.length > 0 && {
+ key: OPERATOR,
+ label: translate("selectOperator", { defaultValue: "Operators" }),
+ multiple: true,
+ options: operatorOptions,
+ type: "select",
+ },
+ triggerRuleOptions.length > 0 && {
+ key: TRIGGER_RULE,
+ label: translate("selectTriggerRules", { defaultValue: "Trigger rules"
}),
+ multiple: true,
+ options: triggerRuleOptions,
+ type: "select",
+ },
+ retryValueOptions.length > 0 && {
+ key: RETRIES,
+ label: translate("retries", { defaultValue: "Retry values" }),
+ multiple: true,
+ options: retryValueOptions,
+ type: "select",
+ },
+ {
+ key: MAPPED,
+ label: translate("mapped", { defaultValue: "Mapped" }),
+ multiple: false,
+ options: mappedOptions,
+ type: "select",
+ },
+ ];
+
+ const configs: Array<FilterConfig> = configsUnfiltered.filter(Boolean) as
Array<FilterConfig>;
+
+ // Initial values mirror previous local names
+ const initialValues = {
+ [MAPPED]: selectedMapped,
+ [NAME_PATTERN]: taskNamePattern || undefined,
+ [OPERATOR]: selectedOperators.length ? selectedOperators : undefined,
+ [RETRIES]: selectedRetries.length ? selectedRetries : undefined,
+ [TRIGGER_RULE]: selectedTriggerRules.length ? selectedTriggerRules :
undefined,
+ };
+
+ // Single place to write the URL params (replaces the old individual
handlers)
+ const onFiltersChange = (record: Record<string, unknown>): void => {
+ const next = new URLSearchParams(searchParams);
+
+ // clear the filter-related keys before writing
+ [NAME_PATTERN, OPERATOR, TRIGGER_RULE, RETRIES, MAPPED,
"page"].forEach((key) => next.delete(key));
+
+ const name = typeof record[NAME_PATTERN] === "string" ?
record[NAME_PATTERN] : "";
+
+ if (name && String(name).trim() !== "") {
+ next.set(NAME_PATTERN, String(name));
+ }
+
+ const operators = Array.isArray(record[OPERATOR]) ? (record[OPERATOR] as
Array<string>) : [];
+
+ operators.forEach((val) => next.append(OPERATOR, val));
+
+ const triggers = Array.isArray(record[TRIGGER_RULE]) ?
(record[TRIGGER_RULE] as Array<string>) : [];
+
+ triggers.forEach((val) => next.append(TRIGGER_RULE, val));
+
+ const retries = Array.isArray(record[RETRIES]) ? (record[RETRIES] as
Array<string>) : [];
+
+ retries.forEach((val) => next.append(RETRIES, val));
+
+ const mapped = typeof record[MAPPED] === "string" ? record[MAPPED] : "";
+
+ if (mapped && String(mapped).trim() !== "") {
+ next.set(MAPPED, String(mapped));
}
- setSearchParams(searchParams);
+
+ // keep the existing behavior used by tests: reset to page 1 on any filter
change
+ next.set("page", "1");
+
Review Comment:
Code could be more consistent with other `FilterBar` usage.
##########
airflow-core/src/airflow/ui/src/pages/Dag/Tasks/TaskFilters/TaskFilters.tsx:
##########
@@ -78,59 +53,116 @@ export const TaskFilters = ({ tasksData }: { readonly
tasksData: TaskCollectionR
{ key: "true", label: translate("mapped") },
{ key: "false", label: translate("notMapped") },
];
- const taskNamePattern = searchParams.get(NAME_PATTERN) ?? "";
- const handleSearchChange = (value: string) => {
- if (value) {
- searchParams.set(NAME_PATTERN, value);
- } else {
- searchParams.delete(NAME_PATTERN);
+
+ // Convert to FilterBar select option shape (label/value)
+ const operatorOptions = allOperatorNames
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const triggerRuleOptions = allTriggerRules
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const retryValueOptions = allRetryValues
+ .map((value) => ({ label: value, value }))
+ .sort((left, right) => left.label.localeCompare(right.label));
+ const mappedOptions = allMappedValues.map(({ key, label }) => ({ label,
value: key }));
+
+ // FilterBar configs (keys unchanged; labels stick to prior wording)
+ const configsUnfiltered: Array<FilterConfig | false> = [
+ {
+ hotkeyDisabled: true,
+ key: NAME_PATTERN,
+ label: translate("searchTasks"),
+ placeholder: translate("searchTasks"),
+ type: "text",
+ },
+ operatorOptions.length > 0 && {
+ key: OPERATOR,
+ label: translate("selectOperator", { defaultValue: "Operators" }),
+ multiple: true,
+ options: operatorOptions,
+ type: "select",
+ },
+ triggerRuleOptions.length > 0 && {
+ key: TRIGGER_RULE,
+ label: translate("selectTriggerRules", { defaultValue: "Trigger rules"
}),
+ multiple: true,
+ options: triggerRuleOptions,
+ type: "select",
+ },
+ retryValueOptions.length > 0 && {
+ key: RETRIES,
+ label: translate("retries", { defaultValue: "Retry values" }),
+ multiple: true,
+ options: retryValueOptions,
+ type: "select",
+ },
+ {
+ key: MAPPED,
+ label: translate("mapped", { defaultValue: "Mapped" }),
+ multiple: false,
+ options: mappedOptions,
+ type: "select",
+ },
+ ];
Review Comment:
Task Instances list are also displayed on TI details view and DagRun details
view. There some filters some be removed accordingly e.g On a TI (group or
mapped index), dag run and dag id are fixed, those filters shouldn't be
possible.
You can take a look at the HITL `FilterBar` implementation to take an
example from.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
