Re: [PR] Add `Required Actions` tab in group task page [airflow]

["pierrejeambrun (via GitHub)" <gi...@apache.org>]

pierrejeambrun commented on code in PR #54322:
URL: https://github.com/apache/airflow/pull/54322#discussion_r2270238587


##########
airflow-core/src/airflow/ui/src/pages/GroupTaskInstance/GroupTaskInstance.tsx:
##########
@@ -18,28 +18,49 @@
  */
 import { ReactFlowProvider } from "@xyflow/react";
 import { useTranslation } from "react-i18next";
+import { FiUser } from "react-icons/fi";
 import { MdOutlineTask } from "react-icons/md";
 import { useParams } from "react-router-dom";
 
+import { useHumanInTheLoopServiceGetHitlDetails } from "openapi/queries";
 import { DetailsLayout } from "src/layouts/Details/DetailsLayout";
 import { useGridTiSummaries } from "src/queries/useGridTISummaries.ts";
 import { isStatePending, useAutoRefresh } from "src/utils";
 
 import { Header } from "./Header";
 
 export const GroupTaskInstance = () => {
-  const { dagId = "", runId = "", taskId = "" } = useParams();
+  const { dagId = "", groupId = "", runId = "" } = useParams();
   const { t: translate } = useTranslation("dag");
   const { data: gridTISummaries } = useGridTiSummaries({ dagId, runId });
-  const taskInstance = gridTISummaries?.task_instances.find((ti) => ti.task_id === taskId);
+  const taskInstance = gridTISummaries?.task_instances.find((ti) => ti.task_id === groupId);
 
   const refetchInterval = useAutoRefresh({ dagId });
 
-  const tabs = [{ icon: <MdOutlineTask />, label: translate("tabs.taskInstances"), value: "" }];
+  const { data: hitlData } = useHumanInTheLoopServiceGetHitlDetails(
+    {
+      dagId,
+      dagRunId: runId,
+      taskIdPattern: groupId,

Review Comment:
   Using `taskIdPattern` to retrieve 'group related' HILT will not work in every case. While this is true that task from a same group tend to share the same prefix, it also mean that any task outside the group that also maches the `pattern` will get returned wrongly in here and displayed in the group actions.
   
   For instance, based on your example, I added a TI, outside all groups, and since the name matches the 'validation' prefix, this will end up like this in the UI
   ```python
       validation_single_task = HITLBranchOperator(
           task_id="validation_single_task",
           subject="This is the validation for the single task",
           options=["1", "2", "3"],
       )
   ```
   
   <img width="1867" height="616" alt="Screenshot 2025-08-12 at 17 16 02" src="https://github.com/user-attachments/assets/a7d46ce5-bb89-4ee1-8d4c-6acd6b9b90f8" />
   
   We need to add a backend filter on `group_id` I think. To be sure to only retrieve 'group' related TIs.



##########
airflow-core/src/airflow/ui/src/pages/GroupTaskInstance/GroupTaskInstance.tsx:
##########
@@ -18,28 +18,49 @@
  */
 import { ReactFlowProvider } from "@xyflow/react";
 import { useTranslation } from "react-i18next";
+import { FiUser } from "react-icons/fi";
 import { MdOutlineTask } from "react-icons/md";
 import { useParams } from "react-router-dom";
 
+import { useHumanInTheLoopServiceGetHitlDetails } from "openapi/queries";
 import { DetailsLayout } from "src/layouts/Details/DetailsLayout";
 import { useGridTiSummaries } from "src/queries/useGridTISummaries.ts";
 import { isStatePending, useAutoRefresh } from "src/utils";
 
 import { Header } from "./Header";
 
 export const GroupTaskInstance = () => {
-  const { dagId = "", runId = "", taskId = "" } = useParams();
+  const { dagId = "", groupId = "", runId = "" } = useParams();
   const { t: translate } = useTranslation("dag");
   const { data: gridTISummaries } = useGridTiSummaries({ dagId, runId });
-  const taskInstance = gridTISummaries?.task_instances.find((ti) => ti.task_id === taskId);
+  const taskInstance = gridTISummaries?.task_instances.find((ti) => ti.task_id === groupId);
 
   const refetchInterval = useAutoRefresh({ dagId });
 
-  const tabs = [{ icon: <MdOutlineTask />, label: translate("tabs.taskInstances"), value: "" }];
+  const { data: hitlData } = useHumanInTheLoopServiceGetHitlDetails(
+    {
+      dagId,
+      dagRunId: runId,
+      taskIdPattern: groupId,
+    },
+    undefined,
+    {
+      enabled: Boolean(dagId && groupId),
+    },
+  );
+
+  const hasHitlForTask = (hitlData?.total_entries ?? 0) > 0;
+
+  const tabs = [
+    { icon: <MdOutlineTask />, label: translate("tabs.taskInstances"), value: "" },
+    { icon: <FiUser />, label: translate("tabs.requiredActions"), value: "required_actions" },
+  ];

Review Comment:
   You don't need to create the full list to then filter it again. 
   
   Example:
   
   ```typsecript
       ...(Boolean(taskId)
       ? []
       : [
           {
             accessorKey: "task_id",
             enableSorting: true,
             header: translate("common:taskId"),
             meta: {
               skeletonWidth: 10,
             },
           },
         ]),
   
   ```



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org