pierrejeambrun commented on code in PR #54210:
URL: https://github.com/apache/airflow/pull/54210#discussion_r2266184425
##########
airflow-core/src/airflow/ui/public/i18n/locales/en/browse.json:
##########
@@ -10,6 +10,14 @@
"user": "User",
"when": "When"
},
+ "filters": {
+ "after": "After",
+ "before": "Before",
+ "endDateTime": "End date and time",
+ "eventType": "Event Type",
+ "filters": "Filters",
Review Comment:
```suggestion
```
This key seems unused.
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -141,31 +144,78 @@ const eventsColumn = (
},
];
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+}: SearchParamsKeysType = SearchParamsKeys;
+
export const Events = () => {
const { t: translate } = useTranslation("browse");
const { dagId, runId, taskId } = useParams();
+ const [searchParams] = useSearchParams();
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const { onClose, onOpen, open } = useDisclosure();
+ // Get filter values from search params
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/ui/public/i18n/locales/en/browse.json:
##########
@@ -10,6 +10,14 @@
"user": "User",
"when": "When"
},
+ "filters": {
+ "after": "After",
+ "before": "Before",
Review Comment:
```suggestion
```
Unsued
##########
airflow-core/src/airflow/ui/src/constants/searchParams.ts:
##########
@@ -17,17 +17,26 @@
* under the License.
*/
export enum SearchParamsKeys {
+ AFTER = "after",
+ BEFORE = "before",
+ DAG_ID = "dag_id",
DEPENDENCIES = "dependencies",
END_DATE = "end_date",
+ // Event log specific filters
Review Comment:
This comment is misplaced, we should probably remove it.
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -141,31 +144,78 @@ const eventsColumn = (
},
];
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+}: SearchParamsKeysType = SearchParamsKeys;
+
export const Events = () => {
const { t: translate } = useTranslation("browse");
const { dagId, runId, taskId } = useParams();
+ const [searchParams] = useSearchParams();
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const { onClose, onOpen, open } = useDisclosure();
+ // Get filter values from search params
+ const afterFilter = searchParams.get(AFTER_PARAM);
+ const beforeFilter = searchParams.get(BEFORE_PARAM);
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM);
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM);
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM);
+ const runIdFilter = searchParams.get(RUN_ID_PARAM);
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM);
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM);
+ const userFilter = searchParams.get(USER_PARAM);
+
const orderBy = sort ? [`${sort.desc ? "-" : ""}${sort.id}`] : ["-when"];
+ // Convert string filters to appropriate types for API
+ const mapIndexNumber =
+ mapIndexFilter !== null && mapIndexFilter !== "" ?
parseInt(mapIndexFilter, 10) : undefined;
+ const tryNumberNumber =
+ tryNumberFilter !== null && tryNumberFilter !== "" ?
parseInt(tryNumberFilter, 10) : undefined;
+
+ // Handle date conversion - ensure valid ISO strings
+ const afterDate =
+ afterFilter !== null && afterFilter !== "" &&
!isNaN(Date.parse(afterFilter)) ? afterFilter : undefined;
+ const beforeDate =
+ beforeFilter !== null && beforeFilter !== "" &&
!isNaN(Date.parse(beforeFilter))
+ ? beforeFilter
+ : undefined;
+
const { data, error, isFetching, isLoading } =
useEventLogServiceGetEventLogs(
{
- dagId,
+ // Additional filters
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -141,31 +144,78 @@ const eventsColumn = (
},
];
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+}: SearchParamsKeysType = SearchParamsKeys;
+
export const Events = () => {
const { t: translate } = useTranslation("browse");
const { dagId, runId, taskId } = useParams();
+ const [searchParams] = useSearchParams();
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const { onClose, onOpen, open } = useDisclosure();
+ // Get filter values from search params
+ const afterFilter = searchParams.get(AFTER_PARAM);
+ const beforeFilter = searchParams.get(BEFORE_PARAM);
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM);
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM);
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM);
+ const runIdFilter = searchParams.get(RUN_ID_PARAM);
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM);
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM);
+ const userFilter = searchParams.get(USER_PARAM);
+
const orderBy = sort ? [`${sort.desc ? "-" : ""}${sort.id}`] : ["-when"];
+ // Convert string filters to appropriate types for API
+ const mapIndexNumber =
+ mapIndexFilter !== null && mapIndexFilter !== "" ?
parseInt(mapIndexFilter, 10) : undefined;
+ const tryNumberNumber =
+ tryNumberFilter !== null && tryNumberFilter !== "" ?
parseInt(tryNumberFilter, 10) : undefined;
+
+ // Handle date conversion - ensure valid ISO strings
+ const afterDate =
+ afterFilter !== null && afterFilter !== "" &&
!isNaN(Date.parse(afterFilter)) ? afterFilter : undefined;
+ const beforeDate =
+ beforeFilter !== null && beforeFilter !== "" &&
!isNaN(Date.parse(beforeFilter))
+ ? beforeFilter
+ : undefined;
+
const { data, error, isFetching, isLoading } =
useEventLogServiceGetEventLogs(
{
- dagId,
+ // Additional filters
+ after: afterDate,
+ before: beforeDate,
+ // URL params take precedence over filters for context-specific views
+ dagId: dagId ?? (dagIdFilter !== null && dagIdFilter !== "" ?
dagIdFilter : undefined),
+ event: eventTypeFilter !== null && eventTypeFilter !== "" ?
eventTypeFilter : undefined,
+ // Pagination and sorting
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -141,31 +144,78 @@ const eventsColumn = (
},
];
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+}: SearchParamsKeysType = SearchParamsKeys;
+
export const Events = () => {
const { t: translate } = useTranslation("browse");
const { dagId, runId, taskId } = useParams();
+ const [searchParams] = useSearchParams();
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const { onClose, onOpen, open } = useDisclosure();
+ // Get filter values from search params
+ const afterFilter = searchParams.get(AFTER_PARAM);
+ const beforeFilter = searchParams.get(BEFORE_PARAM);
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM);
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM);
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM);
+ const runIdFilter = searchParams.get(RUN_ID_PARAM);
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM);
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM);
+ const userFilter = searchParams.get(USER_PARAM);
+
const orderBy = sort ? [`${sort.desc ? "-" : ""}${sort.id}`] : ["-when"];
+ // Convert string filters to appropriate types for API
+ const mapIndexNumber =
+ mapIndexFilter !== null && mapIndexFilter !== "" ?
parseInt(mapIndexFilter, 10) : undefined;
+ const tryNumberNumber =
+ tryNumberFilter !== null && tryNumberFilter !== "" ?
parseInt(tryNumberFilter, 10) : undefined;
Review Comment:
Why are we explicitely checking for `""`. Either the query param is defined
and we get `!== null` and try to cast it to a Number, or it's `null` and we do
nothing or return undefined.
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -141,31 +144,78 @@ const eventsColumn = (
},
];
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+}: SearchParamsKeysType = SearchParamsKeys;
+
export const Events = () => {
const { t: translate } = useTranslation("browse");
const { dagId, runId, taskId } = useParams();
+ const [searchParams] = useSearchParams();
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const { onClose, onOpen, open } = useDisclosure();
+ // Get filter values from search params
+ const afterFilter = searchParams.get(AFTER_PARAM);
+ const beforeFilter = searchParams.get(BEFORE_PARAM);
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM);
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM);
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM);
+ const runIdFilter = searchParams.get(RUN_ID_PARAM);
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM);
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM);
+ const userFilter = searchParams.get(USER_PARAM);
+
const orderBy = sort ? [`${sort.desc ? "-" : ""}${sort.id}`] : ["-when"];
+ // Convert string filters to appropriate types for API
+ const mapIndexNumber =
+ mapIndexFilter !== null && mapIndexFilter !== "" ?
parseInt(mapIndexFilter, 10) : undefined;
+ const tryNumberNumber =
+ tryNumberFilter !== null && tryNumberFilter !== "" ?
parseInt(tryNumberFilter, 10) : undefined;
+
+ // Handle date conversion - ensure valid ISO strings
+ const afterDate =
+ afterFilter !== null && afterFilter !== "" &&
!isNaN(Date.parse(afterFilter)) ? afterFilter : undefined;
+ const beforeDate =
+ beforeFilter !== null && beforeFilter !== "" &&
!isNaN(Date.parse(beforeFilter))
+ ? beforeFilter
+ : undefined;
Review Comment:
Same everywhere, I don't think we should be checking `""` explicitely. It's
no different as any other string, valid or invalid.
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -141,31 +144,78 @@ const eventsColumn = (
},
];
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+}: SearchParamsKeysType = SearchParamsKeys;
+
export const Events = () => {
const { t: translate } = useTranslation("browse");
const { dagId, runId, taskId } = useParams();
+ const [searchParams] = useSearchParams();
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const { onClose, onOpen, open } = useDisclosure();
+ // Get filter values from search params
+ const afterFilter = searchParams.get(AFTER_PARAM);
+ const beforeFilter = searchParams.get(BEFORE_PARAM);
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM);
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM);
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM);
+ const runIdFilter = searchParams.get(RUN_ID_PARAM);
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM);
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM);
+ const userFilter = searchParams.get(USER_PARAM);
+
const orderBy = sort ? [`${sort.desc ? "-" : ""}${sort.id}`] : ["-when"];
+ // Convert string filters to appropriate types for API
+ const mapIndexNumber =
+ mapIndexFilter !== null && mapIndexFilter !== "" ?
parseInt(mapIndexFilter, 10) : undefined;
+ const tryNumberNumber =
+ tryNumberFilter !== null && tryNumberFilter !== "" ?
parseInt(tryNumberFilter, 10) : undefined;
+
+ // Handle date conversion - ensure valid ISO strings
+ const afterDate =
+ afterFilter !== null && afterFilter !== "" &&
!isNaN(Date.parse(afterFilter)) ? afterFilter : undefined;
+ const beforeDate =
+ beforeFilter !== null && beforeFilter !== "" &&
!isNaN(Date.parse(beforeFilter))
+ ? beforeFilter
+ : undefined;
+
const { data, error, isFetching, isLoading } =
useEventLogServiceGetEventLogs(
{
- dagId,
+ // Additional filters
+ after: afterDate,
+ before: beforeDate,
+ // URL params take precedence over filters for context-specific views
+ dagId: dagId ?? (dagIdFilter !== null && dagIdFilter !== "" ?
dagIdFilter : undefined),
+ event: eventTypeFilter !== null && eventTypeFilter !== "" ?
eventTypeFilter : undefined,
+ // Pagination and sorting
limit: pagination.pageSize,
+ mapIndex: mapIndexNumber,
offset: pagination.pageIndex * pagination.pageSize,
orderBy,
- runId,
- taskId,
+ owner: userFilter !== null && userFilter !== "" ? userFilter : undefined,
+ runId: runId ?? (runIdFilter !== null && runIdFilter !== "" ?
runIdFilter : undefined),
+ taskId: taskId ?? (taskIdFilter !== null && taskIdFilter !== "" ?
taskIdFilter : undefined),
+ tryNumber: tryNumberNumber,
Review Comment:
All those checks seem unnecessary and overly complicated. I think
`undefined` and `null` are automatically stripped from the query.
```
owner: userFilter ?? undefined,
```
For instance.
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -191,6 +241,10 @@ export const Events = () => {
</IconButton>
</ButtonGroup>
</Flex>
+
+ {/* Only show filters in the main Browse > Audit Log view, not in
context-specific views */}
+ {dagId === undefined && runId === undefined && taskId === undefined &&
<EventsFilters />}
Review Comment:
We need filters on the context specific views.
<img width="1917" height="839" alt="Screenshot 2025-08-11 at 12 10 29"
src="https://github.com/user-attachments/assets/f67e3bf8-fd50-4f2c-a241-b982a8435bb5";
/>
We just need to hide dag_id, task_id, and run_id input depending on the `
const { dagId, runId, taskId } = useParams();` values.
##########
airflow-core/src/airflow/ui/src/pages/Events/EventsFilters.tsx:
##########
@@ -0,0 +1,226 @@
+/*!
+ * 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 { Box, HStack, Input } from "@chakra-ui/react";
+import { useCallback } from "react";
+import { useTranslation } from "react-i18next";
+import { useSearchParams } from "react-router-dom";
+
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { SearchParamsKeys } from "src/constants/searchParams";
+
+import { ResetButton } from "./ResetButton";
+import { getFilterCount, formatDateTimeLocalValue } from "./filterUtils";
+
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+} = SearchParamsKeys;
+
+export const EventsFilters = () => {
+ const { t: translate } = useTranslation(["browse", "common"]);
+ const [searchParams, setSearchParams] = useSearchParams();
+ const { setTableURLState, tableURLState } = useTableURLState();
+
+ const { pagination, sorting } = tableURLState;
+
+ const resetPagination = useCallback(() => {
+ setTableURLState({
+ pagination: { ...pagination, pageIndex: 0 },
+ sorting,
+ });
+ }, [pagination, setTableURLState, sorting]);
+
+ // Extract filter values from URL
+ const afterFilter = searchParams.get(AFTER_PARAM) ?? "";
+ const beforeFilter = searchParams.get(BEFORE_PARAM) ?? "";
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM) ?? "";
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM) ?? "";
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM) ?? "";
+ const runIdFilter = searchParams.get(RUN_ID_PARAM) ?? "";
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM) ?? "";
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM) ?? "";
+ const userFilter = searchParams.get(USER_PARAM) ?? "";
+
+ const handleInputChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ searchParams.set(paramName, value);
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleDateTimeChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ // Convert datetime-local value to ISO string
+ try {
+ const date = new Date(value);
+
+ if (!isNaN(date.getTime())) {
+ searchParams.set(paramName, date.toISOString());
+ }
+ } catch {
+ // Invalid date, remove the parameter
+ searchParams.delete(paramName);
+ }
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleClearFilters = useCallback(() => {
+ searchParams.delete(AFTER_PARAM);
+ searchParams.delete(BEFORE_PARAM);
+ searchParams.delete(DAG_ID_PARAM);
+ searchParams.delete(EVENT_TYPE_PARAM);
+ searchParams.delete(MAP_INDEX_PARAM);
+ searchParams.delete(RUN_ID_PARAM);
+ searchParams.delete(TASK_ID_PARAM);
+ searchParams.delete(TRY_NUMBER_PARAM);
+ searchParams.delete(USER_PARAM);
+
+ resetPagination();
+ setSearchParams(searchParams);
+ }, [resetPagination, searchParams, setSearchParams]);
+
+ const filterCount = getFilterCount({
+ after: afterFilter,
+ before: beforeFilter,
+ dagId: dagIdFilter,
+ eventType: eventTypeFilter,
+ mapIndex: mapIndexFilter,
+ runId: runIdFilter,
+ taskId: taskIdFilter,
+ tryNumber: tryNumberFilter,
+ user: userFilter,
+ });
+
+ return (
+ <HStack justifyContent="space-between">
+ <HStack flexWrap="wrap" gap={4}>
+ {/* Timestamp Range Filters */}
+ <Input
+ onChange={handleDateTimeChange(AFTER_PARAM)}
+ placeholder={translate("auditLog.filters.startDateTime")}
+ size="sm"
+ type="datetime-local"
+ value={formatDateTimeLocalValue(afterFilter)}
+ width="200px"
Review Comment:
Remove all the hardcodeed `width`.
Remove the `flexWrap="wrap"` of. the hstack you should get this:
<img width="1895" height="495" alt="Screenshot 2025-08-11 at 12 16 10"
src="https://github.com/user-attachments/assets/62d3d668-269e-42d8-a4fa-53fa4946b534";
/>
##########
airflow-core/src/airflow/ui/src/pages/Events/EventsFilters.tsx:
##########
@@ -0,0 +1,226 @@
+/*!
+ * 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 { Box, HStack, Input } from "@chakra-ui/react";
+import { useCallback } from "react";
+import { useTranslation } from "react-i18next";
+import { useSearchParams } from "react-router-dom";
+
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { SearchParamsKeys } from "src/constants/searchParams";
+
+import { ResetButton } from "./ResetButton";
+import { getFilterCount, formatDateTimeLocalValue } from "./filterUtils";
+
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+} = SearchParamsKeys;
+
+export const EventsFilters = () => {
+ const { t: translate } = useTranslation(["browse", "common"]);
+ const [searchParams, setSearchParams] = useSearchParams();
+ const { setTableURLState, tableURLState } = useTableURLState();
+
+ const { pagination, sorting } = tableURLState;
+
+ const resetPagination = useCallback(() => {
+ setTableURLState({
+ pagination: { ...pagination, pageIndex: 0 },
+ sorting,
+ });
+ }, [pagination, setTableURLState, sorting]);
+
+ // Extract filter values from URL
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/ui/src/pages/Events/EventsFilters.tsx:
##########
@@ -0,0 +1,226 @@
+/*!
+ * 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 { Box, HStack, Input } from "@chakra-ui/react";
+import { useCallback } from "react";
+import { useTranslation } from "react-i18next";
+import { useSearchParams } from "react-router-dom";
+
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { SearchParamsKeys } from "src/constants/searchParams";
+
+import { ResetButton } from "./ResetButton";
+import { getFilterCount, formatDateTimeLocalValue } from "./filterUtils";
+
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+} = SearchParamsKeys;
+
+export const EventsFilters = () => {
+ const { t: translate } = useTranslation(["browse", "common"]);
+ const [searchParams, setSearchParams] = useSearchParams();
+ const { setTableURLState, tableURLState } = useTableURLState();
+
+ const { pagination, sorting } = tableURLState;
+
+ const resetPagination = useCallback(() => {
+ setTableURLState({
+ pagination: { ...pagination, pageIndex: 0 },
+ sorting,
+ });
+ }, [pagination, setTableURLState, sorting]);
+
+ // Extract filter values from URL
+ const afterFilter = searchParams.get(AFTER_PARAM) ?? "";
+ const beforeFilter = searchParams.get(BEFORE_PARAM) ?? "";
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM) ?? "";
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM) ?? "";
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM) ?? "";
+ const runIdFilter = searchParams.get(RUN_ID_PARAM) ?? "";
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM) ?? "";
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM) ?? "";
+ const userFilter = searchParams.get(USER_PARAM) ?? "";
+
+ const handleInputChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ searchParams.set(paramName, value);
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleDateTimeChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ // Convert datetime-local value to ISO string
+ try {
+ const date = new Date(value);
+
+ if (!isNaN(date.getTime())) {
+ searchParams.set(paramName, date.toISOString());
+ }
+ } catch {
+ // Invalid date, remove the parameter
+ searchParams.delete(paramName);
+ }
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleClearFilters = useCallback(() => {
Review Comment:
`onClearFIlter` does not cost much so bellow we get
`onClearFilters={onClearFilters}` which is cleaner.
##########
airflow-core/src/airflow/ui/src/pages/Events/filterUtils.ts:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+export type FilterOptions = {
+ after: string | null;
+ before: string | null;
+ dagId: string | null;
+ eventType: string | null;
+ mapIndex: string | null;
+ runId: string | null;
+ taskId: string | null;
+ tryNumber: string | null;
+ user: string | null;
+};
+
+export const getFilterCount = ({
+ after,
+ before,
+ dagId,
+ eventType,
+ mapIndex,
+ runId,
+ taskId,
+ tryNumber,
+ user,
+}: FilterOptions) => {
+ let count = 0;
+
+ if (Boolean(after)) {
+ count += 1;
+ }
+ if (Boolean(before)) {
+ count += 1;
+ }
+ if (Boolean(dagId)) {
+ count += 1;
+ }
+ if (Boolean(eventType)) {
+ count += 1;
+ }
+
+ if (Boolean(mapIndex)) {
+ count += 1;
+ }
+ if (Boolean(runId)) {
+ count += 1;
+ }
+ if (Boolean(taskId)) {
+ count += 1;
+ }
+ if (Boolean(tryNumber)) {
+ count += 1;
+ }
+ if (Boolean(user)) {
+ count += 1;
+ }
Review Comment:
Take a look at `const getFilterCount` in DagsFilter. You can't do
`Boolean(something)` because this will create a bug for all false values.
(For instance if I want to filter on map_index = 0, this will return
'false', no filter applied but there is a filter)
##########
airflow-core/src/airflow/ui/src/pages/Events/EventsFilters.tsx:
##########
@@ -0,0 +1,226 @@
+/*!
+ * 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 { Box, HStack, Input } from "@chakra-ui/react";
+import { useCallback } from "react";
+import { useTranslation } from "react-i18next";
+import { useSearchParams } from "react-router-dom";
+
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { SearchParamsKeys } from "src/constants/searchParams";
+
+import { ResetButton } from "./ResetButton";
+import { getFilterCount, formatDateTimeLocalValue } from "./filterUtils";
+
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+} = SearchParamsKeys;
+
+export const EventsFilters = () => {
+ const { t: translate } = useTranslation(["browse", "common"]);
+ const [searchParams, setSearchParams] = useSearchParams();
+ const { setTableURLState, tableURLState } = useTableURLState();
+
+ const { pagination, sorting } = tableURLState;
+
+ const resetPagination = useCallback(() => {
+ setTableURLState({
+ pagination: { ...pagination, pageIndex: 0 },
+ sorting,
+ });
+ }, [pagination, setTableURLState, sorting]);
+
+ // Extract filter values from URL
+ const afterFilter = searchParams.get(AFTER_PARAM) ?? "";
+ const beforeFilter = searchParams.get(BEFORE_PARAM) ?? "";
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM) ?? "";
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM) ?? "";
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM) ?? "";
+ const runIdFilter = searchParams.get(RUN_ID_PARAM) ?? "";
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM) ?? "";
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM) ?? "";
+ const userFilter = searchParams.get(USER_PARAM) ?? "";
+
+ const handleInputChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ searchParams.set(paramName, value);
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleDateTimeChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ // Convert datetime-local value to ISO string
+ try {
+ const date = new Date(value);
+
+ if (!isNaN(date.getTime())) {
+ searchParams.set(paramName, date.toISOString());
+ }
+ } catch {
+ // Invalid date, remove the parameter
+ searchParams.delete(paramName);
+ }
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleClearFilters = useCallback(() => {
+ searchParams.delete(AFTER_PARAM);
+ searchParams.delete(BEFORE_PARAM);
+ searchParams.delete(DAG_ID_PARAM);
+ searchParams.delete(EVENT_TYPE_PARAM);
+ searchParams.delete(MAP_INDEX_PARAM);
+ searchParams.delete(RUN_ID_PARAM);
+ searchParams.delete(TASK_ID_PARAM);
+ searchParams.delete(TRY_NUMBER_PARAM);
+ searchParams.delete(USER_PARAM);
+
+ resetPagination();
+ setSearchParams(searchParams);
+ }, [resetPagination, searchParams, setSearchParams]);
+
+ const filterCount = getFilterCount({
+ after: afterFilter,
+ before: beforeFilter,
+ dagId: dagIdFilter,
+ eventType: eventTypeFilter,
+ mapIndex: mapIndexFilter,
+ runId: runIdFilter,
+ taskId: taskIdFilter,
+ tryNumber: tryNumberFilter,
+ user: userFilter,
+ });
+
+ return (
+ <HStack justifyContent="space-between">
+ <HStack flexWrap="wrap" gap={4}>
+ {/* Timestamp Range Filters */}
+ <Input
+ onChange={handleDateTimeChange(AFTER_PARAM)}
+ placeholder={translate("auditLog.filters.startDateTime")}
+ size="sm"
+ type="datetime-local"
+ value={formatDateTimeLocalValue(afterFilter)}
+ width="200px"
Review Comment:
We have a `DateTimeInput` component that we should reuse.
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -141,31 +144,78 @@ const eventsColumn = (
},
];
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+}: SearchParamsKeysType = SearchParamsKeys;
+
export const Events = () => {
const { t: translate } = useTranslation("browse");
const { dagId, runId, taskId } = useParams();
+ const [searchParams] = useSearchParams();
const { setTableURLState, tableURLState } = useTableURLState();
const { pagination, sorting } = tableURLState;
const [sort] = sorting;
const { onClose, onOpen, open } = useDisclosure();
+ // Get filter values from search params
+ const afterFilter = searchParams.get(AFTER_PARAM);
+ const beforeFilter = searchParams.get(BEFORE_PARAM);
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM);
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM);
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM);
+ const runIdFilter = searchParams.get(RUN_ID_PARAM);
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM);
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM);
+ const userFilter = searchParams.get(USER_PARAM);
+
const orderBy = sort ? [`${sort.desc ? "-" : ""}${sort.id}`] : ["-when"];
+ // Convert string filters to appropriate types for API
+ const mapIndexNumber =
+ mapIndexFilter !== null && mapIndexFilter !== "" ?
parseInt(mapIndexFilter, 10) : undefined;
+ const tryNumberNumber =
+ tryNumberFilter !== null && tryNumberFilter !== "" ?
parseInt(tryNumberFilter, 10) : undefined;
+
+ // Handle date conversion - ensure valid ISO strings
+ const afterDate =
+ afterFilter !== null && afterFilter !== "" &&
!isNaN(Date.parse(afterFilter)) ? afterFilter : undefined;
+ const beforeDate =
+ beforeFilter !== null && beforeFilter !== "" &&
!isNaN(Date.parse(beforeFilter))
+ ? beforeFilter
+ : undefined;
+
const { data, error, isFetching, isLoading } =
useEventLogServiceGetEventLogs(
{
- dagId,
+ // Additional filters
+ after: afterDate,
+ before: beforeDate,
+ // URL params take precedence over filters for context-specific views
+ dagId: dagId ?? (dagIdFilter !== null && dagIdFilter !== "" ?
dagIdFilter : undefined),
+ event: eventTypeFilter !== null && eventTypeFilter !== "" ?
eventTypeFilter : undefined,
+ // Pagination and sorting
limit: pagination.pageSize,
+ mapIndex: mapIndexNumber,
offset: pagination.pageIndex * pagination.pageSize,
orderBy,
- runId,
- taskId,
+ owner: userFilter !== null && userFilter !== "" ? userFilter : undefined,
+ runId: runId ?? (runIdFilter !== null && runIdFilter !== "" ?
runIdFilter : undefined),
+ taskId: taskId ?? (taskIdFilter !== null && taskIdFilter !== "" ?
taskIdFilter : undefined),
+ tryNumber: tryNumberNumber,
Review Comment:
```
runId: runId ?? runIdFilter ?? undefined,
```
##########
airflow-core/src/airflow/ui/src/pages/Events/EventsFilters.tsx:
##########
@@ -0,0 +1,226 @@
+/*!
+ * 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 { Box, HStack, Input } from "@chakra-ui/react";
+import { useCallback } from "react";
+import { useTranslation } from "react-i18next";
+import { useSearchParams } from "react-router-dom";
+
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { SearchParamsKeys } from "src/constants/searchParams";
+
+import { ResetButton } from "./ResetButton";
+import { getFilterCount, formatDateTimeLocalValue } from "./filterUtils";
+
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+} = SearchParamsKeys;
+
+export const EventsFilters = () => {
+ const { t: translate } = useTranslation(["browse", "common"]);
+ const [searchParams, setSearchParams] = useSearchParams();
+ const { setTableURLState, tableURLState } = useTableURLState();
+
+ const { pagination, sorting } = tableURLState;
+
+ const resetPagination = useCallback(() => {
+ setTableURLState({
+ pagination: { ...pagination, pageIndex: 0 },
+ sorting,
+ });
+ }, [pagination, setTableURLState, sorting]);
+
+ // Extract filter values from URL
+ const afterFilter = searchParams.get(AFTER_PARAM) ?? "";
+ const beforeFilter = searchParams.get(BEFORE_PARAM) ?? "";
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM) ?? "";
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM) ?? "";
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM) ?? "";
+ const runIdFilter = searchParams.get(RUN_ID_PARAM) ?? "";
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM) ?? "";
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM) ?? "";
+ const userFilter = searchParams.get(USER_PARAM) ?? "";
+
+ const handleInputChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ searchParams.set(paramName, value);
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleDateTimeChange = useCallback(
+ (paramName: string) => (event: React.ChangeEvent<HTMLInputElement>) => {
+ const { value } = event.target;
+
+ if (value) {
+ // Convert datetime-local value to ISO string
+ try {
+ const date = new Date(value);
+
+ if (!isNaN(date.getTime())) {
+ searchParams.set(paramName, date.toISOString());
+ }
+ } catch {
+ // Invalid date, remove the parameter
+ searchParams.delete(paramName);
+ }
+ } else {
+ searchParams.delete(paramName);
+ }
+ resetPagination();
+ setSearchParams(searchParams);
+ },
+ [resetPagination, searchParams, setSearchParams],
+ );
+
+ const handleClearFilters = useCallback(() => {
+ searchParams.delete(AFTER_PARAM);
+ searchParams.delete(BEFORE_PARAM);
+ searchParams.delete(DAG_ID_PARAM);
+ searchParams.delete(EVENT_TYPE_PARAM);
+ searchParams.delete(MAP_INDEX_PARAM);
+ searchParams.delete(RUN_ID_PARAM);
+ searchParams.delete(TASK_ID_PARAM);
+ searchParams.delete(TRY_NUMBER_PARAM);
+ searchParams.delete(USER_PARAM);
+
+ resetPagination();
+ setSearchParams(searchParams);
+ }, [resetPagination, searchParams, setSearchParams]);
+
+ const filterCount = getFilterCount({
+ after: afterFilter,
+ before: beforeFilter,
+ dagId: dagIdFilter,
+ eventType: eventTypeFilter,
+ mapIndex: mapIndexFilter,
+ runId: runIdFilter,
+ taskId: taskIdFilter,
+ tryNumber: tryNumberFilter,
+ user: userFilter,
+ });
+
+ return (
+ <HStack justifyContent="space-between">
+ <HStack flexWrap="wrap" gap={4}>
+ {/* Timestamp Range Filters */}
+ <Input
+ onChange={handleDateTimeChange(AFTER_PARAM)}
+ placeholder={translate("auditLog.filters.startDateTime")}
+ size="sm"
+ type="datetime-local"
+ value={formatDateTimeLocalValue(afterFilter)}
+ width="200px"
+ />
+ <Input
+ onChange={handleDateTimeChange(BEFORE_PARAM)}
+ placeholder={translate("auditLog.filters.endDateTime")}
+ size="sm"
+ type="datetime-local"
+ value={formatDateTimeLocalValue(beforeFilter)}
+ width="200px"
+ />
+
+ {/* Event Type Filter */}
+ <Input
+ onChange={handleInputChange(EVENT_TYPE_PARAM)}
+ placeholder={translate("auditLog.filters.eventType")}
+ size="sm"
+ value={eventTypeFilter}
+ width="150px"
+ />
Review Comment:
We will need to maybe create a `<DebounceInput` component or simply add a
debounce to all inputs, similarly to the `const SearchBar`.
Otherwise this will trigger a backend request on each keystroke, which can
make a lot of unecessary request.
##########
airflow-core/src/airflow/ui/src/pages/Events/EventsFilters.tsx:
##########
@@ -0,0 +1,226 @@
+/*!
+ * 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 { Box, HStack, Input } from "@chakra-ui/react";
+import { useCallback } from "react";
+import { useTranslation } from "react-i18next";
+import { useSearchParams } from "react-router-dom";
+
+import { useTableURLState } from "src/components/DataTable/useTableUrlState";
+import { SearchParamsKeys } from "src/constants/searchParams";
+
+import { ResetButton } from "./ResetButton";
+import { getFilterCount, formatDateTimeLocalValue } from "./filterUtils";
+
+const {
+ AFTER: AFTER_PARAM,
+ BEFORE: BEFORE_PARAM,
+ DAG_ID: DAG_ID_PARAM,
+ EVENT_TYPE: EVENT_TYPE_PARAM,
+ MAP_INDEX: MAP_INDEX_PARAM,
+ RUN_ID: RUN_ID_PARAM,
+ TASK_ID: TASK_ID_PARAM,
+ TRY_NUMBER: TRY_NUMBER_PARAM,
+ USER: USER_PARAM,
+} = SearchParamsKeys;
+
+export const EventsFilters = () => {
+ const { t: translate } = useTranslation(["browse", "common"]);
+ const [searchParams, setSearchParams] = useSearchParams();
+ const { setTableURLState, tableURLState } = useTableURLState();
+
+ const { pagination, sorting } = tableURLState;
+
+ const resetPagination = useCallback(() => {
+ setTableURLState({
+ pagination: { ...pagination, pageIndex: 0 },
+ sorting,
+ });
+ }, [pagination, setTableURLState, sorting]);
+
+ // Extract filter values from URL
+ const afterFilter = searchParams.get(AFTER_PARAM) ?? "";
+ const beforeFilter = searchParams.get(BEFORE_PARAM) ?? "";
+ const dagIdFilter = searchParams.get(DAG_ID_PARAM) ?? "";
+ const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM) ?? "";
+ const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM) ?? "";
+ const runIdFilter = searchParams.get(RUN_ID_PARAM) ?? "";
+ const taskIdFilter = searchParams.get(TASK_ID_PARAM) ?? "";
+ const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM) ?? "";
+ const userFilter = searchParams.get(USER_PARAM) ?? "";
Review Comment:
```suggestion
const afterFilter = searchParams.get(AFTER_PARAM);
const beforeFilter = searchParams.get(BEFORE_PARAM);
const dagIdFilter = searchParams.get(DAG_ID_PARAM);
const eventTypeFilter = searchParams.get(EVENT_TYPE_PARAM);
const mapIndexFilter = searchParams.get(MAP_INDEX_PARAM);
const runIdFilter = searchParams.get(RUN_ID_PARAM);
const taskIdFilter = searchParams.get(TASK_ID_PARAM);
const tryNumberFilter = searchParams.get(TRY_NUMBER_PARAM);
const userFilter = searchParams.get(USER_PARAM);
```
It's much easier to have `null` there and know that `null` mean the value
was not fill, instead of having `""` everywhere in the code, not knowing if
this was a user input or an empty filter.
You can take a look at `DagsFilters`
##########
airflow-core/src/airflow/ui/src/pages/Events/Events.tsx:
##########
@@ -191,6 +241,10 @@ export const Events = () => {
</IconButton>
</ButtonGroup>
</Flex>
+
+ {/* Only show filters in the main Browse > Audit Log view, not in
context-specific views */}
+ {dagId === undefined && runId === undefined && taskId === undefined &&
<EventsFilters />}
Review Comment:
For instance if `dagId` from the path is defined, hide via conditional
rendering.
```
{/* DAG ID Filter */}
<Input
onChange={handleInputChange(DAG_ID_PARAM)}
placeholder={translate("common:dagId")}
size="sm"
value={dagIdFilter}
width="150px"
/>
```
##########
airflow-core/src/airflow/ui/src/pages/Events/filterUtils.ts:
##########
@@ -0,0 +1,94 @@
+/*!
+ * 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.
+ */
+export type FilterOptions = {
+ after: string | null;
+ before: string | null;
+ dagId: string | null;
+ eventType: string | null;
+ mapIndex: string | null;
+ runId: string | null;
+ taskId: string | null;
+ tryNumber: string | null;
+ user: string | null;
+};
+
+export const getFilterCount = ({
+ after,
+ before,
+ dagId,
+ eventType,
+ mapIndex,
+ runId,
+ taskId,
+ tryNumber,
+ user,
+}: FilterOptions) => {
+ let count = 0;
+
+ if (Boolean(after)) {
+ count += 1;
+ }
+ if (Boolean(before)) {
+ count += 1;
+ }
+ if (Boolean(dagId)) {
+ count += 1;
+ }
+ if (Boolean(eventType)) {
+ count += 1;
+ }
+
+ if (Boolean(mapIndex)) {
+ count += 1;
+ }
+ if (Boolean(runId)) {
+ count += 1;
+ }
+ if (Boolean(taskId)) {
+ count += 1;
+ }
+ if (Boolean(tryNumber)) {
+ count += 1;
+ }
+ if (Boolean(user)) {
+ count += 1;
+ }
+
+ return count;
+};
+
+export const formatDateTimeLocalValue = (isoString: string | null) => {
+ if (isoString === null || isoString === "") {
+ return "";
+ }
+
+ try {
+ // Parse the ISO string and convert to local datetime-local format
+ const date = new Date(isoString);
+ const year = date.getFullYear();
+ const month = String(date.getMonth() + 1).padStart(2, "0");
+ const day = String(date.getDate()).padStart(2, "0");
+ const hours = String(date.getHours()).padStart(2, "0");
+ const minutes = String(date.getMinutes()).padStart(2, "0");
+
+ return `${year}-${month}-${day}T${hours}:${minutes}`;
+ } catch {
+ return "";
+ }
+};
Review Comment:
We don't need that, we already have code to handle datetime input, cf above,
also you can for instance check the backfill form.
--
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]
