From 91400edeca173169e27d4fc417efc66b1cade4d2 Mon Sep 17 00:00:00 2001 From: Radoslaw Szwajkowski Date: Wed, 26 Jun 2024 18:48:41 +0200 Subject: [PATCH] :bug: Add task detail links to tasks page (#1980) 1. extract base component to re-use for Analysis Details and Task Details 2. add 2 new routes: a) /tasks/:taskId b) /tasks/:taskId/attachments/:attachmentId 3. user can enter details screen by: a) task status name (link) b) "Task details" action (per row kebab actions) 4. persist page state in session storage for Applications and Task Manager pages to fix problems with returning from task details page. Resolves: #1975 --------- Signed-off-by: Radoslaw Szwajkowski --- client/public/locales/en/translation.json | 5 +- client/src/app/Paths.ts | 7 + client/src/app/Routes.tsx | 11 ++ .../analysis-details/AnalysisDetails.tsx | 138 +++++------------- .../applications-table/applications-table.tsx | 7 +- client/src/app/pages/tasks/TaskDetails.tsx | 40 +++++ .../src/app/pages/tasks/TaskDetailsBase.tsx | 96 ++++++++++++ client/src/app/pages/tasks/tasks-page.tsx | 43 ++++-- client/src/app/pages/tasks/useTaskActions.tsx | 10 +- 9 files changed, 244 insertions(+), 113 deletions(-) create mode 100644 client/src/app/pages/tasks/TaskDetails.tsx create mode 100644 client/src/app/pages/tasks/TaskDetailsBase.tsx diff --git a/client/public/locales/en/translation.json b/client/public/locales/en/translation.json index fb35753ff..f20cca117 100644 --- a/client/public/locales/en/translation.json +++ b/client/public/locales/en/translation.json @@ -55,6 +55,7 @@ "selectNone": "Select none", "selectPage": "Select page", "submitReview": "Submit review", + "taskDetails": "Task details", "unlink": "Unlink", "view": "View", "viewErrorReport": "View error report", @@ -463,6 +464,7 @@ "tagCategory": "Tag category", "tagCategoryDeleted": "Tag category deleted", "tagCategories": "Tag categories", + "tasks": "Tasks", "teamMember": "team member", "terminated": "Terminated", "ticket": "Ticket", @@ -483,7 +485,8 @@ "titles": { "archetypeDrawer": "Archetype details", "taskManager": "Task Manager", - "task": "Task" + "task": "Task", + "taskWithId": "Task {{taskId}}" }, "toastr": { "success": { diff --git a/client/src/app/Paths.ts b/client/src/app/Paths.ts index 7f3551cc8..8bd2e6f1d 100644 --- a/client/src/app/Paths.ts +++ b/client/src/app/Paths.ts @@ -41,6 +41,8 @@ export const DevPaths = { dependencies: "/dependencies", tasks: "/tasks", + taskDetails: "/tasks/:taskId", + taskDetailsAttachment: "/tasks/:taskId/attachments/:attachmentId", } as const; export type DevPathValues = (typeof DevPaths)[keyof typeof DevPaths]; @@ -102,3 +104,8 @@ export interface AnalysisDetailsAttachmentRoute { taskId: string; attachmentId: string; } + +export interface TaskDetailsAttachmentRoute { + taskId: string; + attachmentId: string; +} diff --git a/client/src/app/Routes.tsx b/client/src/app/Routes.tsx index ba970d233..64cb995cc 100644 --- a/client/src/app/Routes.tsx +++ b/client/src/app/Routes.tsx @@ -65,6 +65,7 @@ const AssessmentSummary = lazy( ); const TaskManager = lazy(() => import("./pages/tasks/tasks-page")); +const TaskDetails = lazy(() => import("./pages/tasks/TaskDetails")); export interface IRoute { path: T; @@ -195,6 +196,16 @@ export const devRoutes: IRoute[] = [ { path: Paths.tasks, comp: TaskManager, + exact: true, + }, + { + path: Paths.taskDetails, + comp: TaskDetails, + exact: true, + }, + { + path: Paths.taskDetailsAttachment, + comp: TaskDetails, exact: false, }, ]; diff --git a/client/src/app/pages/applications/analysis-details/AnalysisDetails.tsx b/client/src/app/pages/applications/analysis-details/AnalysisDetails.tsx index ecc51b5e0..a6361ec67 100644 --- a/client/src/app/pages/applications/analysis-details/AnalysisDetails.tsx +++ b/client/src/app/pages/applications/analysis-details/AnalysisDetails.tsx @@ -1,116 +1,56 @@ import React from "react"; -import { useHistory, useLocation, useParams } from "react-router-dom"; +import { useParams } from "react-router-dom"; import { useTranslation } from "react-i18next"; -import { PageSection } from "@patternfly/react-core"; - import { AnalysisDetailsAttachmentRoute, Paths } from "@app/Paths"; -import { PageHeader } from "@app/components/PageHeader"; import { formatPath } from "@app/utils/utils"; -import { - DocumentId, - SimpleDocumentViewer, -} from "@app/components/simple-document-viewer"; -import { useFetchApplicationById } from "@app/queries/applications"; -import { useFetchTaskByID } from "@app/queries/tasks"; + import "@app/components/simple-document-viewer/SimpleDocumentViewer.css"; +import { TaskDetailsBase } from "@app/pages/tasks/TaskDetailsBase"; +import { useFetchApplicationById } from "@app/queries/applications"; -export const AnalysisDetails: React.FC = () => { +export const AnalysisDetails = () => { const { t } = useTranslation(); - const { applicationId, taskId, attachmentId } = useParams(); - const { search } = useLocation(); - const hasMergedParam = new URLSearchParams(search).has("merged"); - - const history = useHistory(); - const onDocumentChange = (documentId: DocumentId) => - typeof documentId === "number" - ? history.push( - formatPath(Paths.applicationsAnalysisDetailsAttachment, { - applicationId: applicationId, - taskId: taskId, - attachmentId: documentId, - }) - ) - : history.push({ - pathname: formatPath(Paths.applicationsAnalysisDetails, { - applicationId: applicationId, - taskId: taskId, - }), - search: documentId === "MERGED_VIEW" ? "?merged=true" : undefined, - }); + const detailsPath = formatPath(Paths.applicationsAnalysisDetails, { + applicationId: applicationId, + taskId: taskId, + }); const { application } = useFetchApplicationById(applicationId); - const { task } = useFetchTaskByID(Number(taskId)); - - const taskName = task?.name ?? t("terms.unknown"); const appName: string = application?.name ?? t("terms.unknown"); - const attachmentName = task?.attached?.find( - ({ id }) => String(id) === attachmentId - )?.name; - const resolvedAttachmentId = attachmentName - ? Number(attachmentId) - : undefined; - const resolvedLogMode = hasMergedParam ? "MERGED_VIEW" : "LOG_VIEW"; return ( - <> - - - - -
- -
- - + `Analysis details for ${taskName}`} + formatAttachmentPath={(attachmentId) => + formatPath(Paths.applicationsAnalysisDetailsAttachment, { + applicationId, + taskId, + attachmentId, + }) + } + taskId={Number(taskId)} + attachmentId={attachmentId} + /> ); }; diff --git a/client/src/app/pages/applications/applications-table/applications-table.tsx b/client/src/app/pages/applications/applications-table/applications-table.tsx index 049c739dd..457797ddf 100644 --- a/client/src/app/pages/applications/applications-table/applications-table.tsx +++ b/client/src/app/pages/applications/applications-table/applications-table.tsx @@ -319,7 +319,12 @@ export const ApplicationsTable: React.FC = () => { isSortEnabled: true, isPaginationEnabled: true, isActiveItemEnabled: true, - persistTo: { activeItem: "urlParams" }, + persistTo: { + activeItem: "urlParams", + filter: "sessionStorage", + pagination: "sessionStorage", + sort: "sessionStorage", + }, isLoading: isFetchingApplications, sortableColumns: ["name", "businessService", "tags", "effort"], initialSort: { columnKey: "name", direction: "asc" }, diff --git a/client/src/app/pages/tasks/TaskDetails.tsx b/client/src/app/pages/tasks/TaskDetails.tsx new file mode 100644 index 000000000..86381a285 --- /dev/null +++ b/client/src/app/pages/tasks/TaskDetails.tsx @@ -0,0 +1,40 @@ +import React from "react"; +import { useParams } from "react-router-dom"; +import { useTranslation } from "react-i18next"; + +import { Paths, TaskDetailsAttachmentRoute } from "@app/Paths"; +import "@app/components/simple-document-viewer/SimpleDocumentViewer.css"; +import { formatPath } from "@app/utils/utils"; +import { TaskDetailsBase } from "./TaskDetailsBase"; + +export const TaskDetails = () => { + const { t } = useTranslation(); + const { taskId, attachmentId } = useParams(); + const detailsPath = formatPath(Paths.taskDetails, { taskId }); + return ( + `Task details for task ${taskId}, ${taskName}`} + formatAttachmentPath={(attachmentId) => + formatPath(Paths.taskDetailsAttachment, { + taskId, + attachmentId, + }) + } + taskId={Number(taskId)} + attachmentId={attachmentId} + /> + ); +}; + +export default TaskDetails; diff --git a/client/src/app/pages/tasks/TaskDetailsBase.tsx b/client/src/app/pages/tasks/TaskDetailsBase.tsx new file mode 100644 index 000000000..27b30eb1b --- /dev/null +++ b/client/src/app/pages/tasks/TaskDetailsBase.tsx @@ -0,0 +1,96 @@ +import React from "react"; +import { useHistory, useLocation } from "react-router-dom"; +import { useTranslation } from "react-i18next"; + +import { PageSection } from "@patternfly/react-core"; + +import { PageHeader, PageHeaderProps } from "@app/components/PageHeader"; +import { + DocumentId, + SimpleDocumentViewer, +} from "@app/components/simple-document-viewer"; +import { useFetchTaskByID } from "@app/queries/tasks"; +import "@app/components/simple-document-viewer/SimpleDocumentViewer.css"; + +export const TaskDetailsBase: React.FC<{ + breadcrumbs: PageHeaderProps["breadcrumbs"]; + formatTitle: (taskName: string) => string; + detailsPath: string; + formatAttachmentPath: (attachmentId: number | string) => string; + taskId: number; + attachmentId?: string; +}> = ({ + breadcrumbs, + formatTitle, + detailsPath, + formatAttachmentPath, + taskId, + attachmentId, +}) => { + const { t } = useTranslation(); + + const { search } = useLocation(); + const hasMergedParam = new URLSearchParams(search).has("merged"); + + const history = useHistory(); + const onDocumentChange = (documentId: DocumentId) => + typeof documentId === "number" + ? history.push(formatAttachmentPath(documentId)) + : history.push({ + pathname: detailsPath, + search: documentId === "MERGED_VIEW" ? "?merged=true" : undefined, + }); + + const { task } = useFetchTaskByID(taskId); + + const taskName: string = task?.name ?? t("terms.unknown"); + const attachmentName = task?.attached?.find( + ({ id }) => String(id) === attachmentId + )?.name; + const resolvedAttachmentId = attachmentName + ? Number(attachmentId) + : undefined; + const resolvedLogMode = hasMergedParam ? "MERGED_VIEW" : "LOG_VIEW"; + + return ( + <> + + + + +
+ +
+ + + ); +}; diff --git a/client/src/app/pages/tasks/tasks-page.tsx b/client/src/app/pages/tasks/tasks-page.tsx index 1b178f149..915deccbc 100644 --- a/client/src/app/pages/tasks/tasks-page.tsx +++ b/client/src/app/pages/tasks/tasks-page.tsx @@ -1,6 +1,6 @@ import React, { ReactNode } from "react"; import { useTranslation } from "react-i18next"; -import { useHistory } from "react-router-dom"; +import { Link, useHistory } from "react-router-dom"; import { EmptyState, EmptyStateHeader, @@ -47,6 +47,8 @@ import { IconWithLabel, taskStateToIcon } from "@app/components/Icons"; import { ManageColumnsToolbar } from "../applications/applications-table/components/manage-columns-toolbar"; import dayjs from "dayjs"; import { useTaskActions } from "./useTaskActions"; +import { formatPath } from "@app/utils/utils"; +import { Paths } from "@app/Paths"; export const TasksPage: React.FC = () => { const { t } = useTranslation(); @@ -59,7 +61,11 @@ export const TasksPage: React.FC = () => { const tableControlState = useTableControlState({ tableName: "tasks-table", - persistTo: { filter: "urlParams" }, + persistTo: { + filter: "urlParams", + pagination: "sessionStorage", + sort: "sessionStorage", + }, persistenceKeyPrefix: TablePersistenceKeyPrefix.tasks, columnNames: { id: "ID", @@ -199,7 +205,8 @@ export const TasksPage: React.FC = () => { filterToolbarProps.setFilterValues({}); }; - const { cancelTask, togglePreemption } = useTaskActions(); + const { cancelTask, togglePreemption, canCancel, canTogglePreemption } = + useTaskActions(); const toCells = ({ id, @@ -218,7 +225,18 @@ export const TasksPage: React.FC = () => { application: application.name, kind: kind ?? addon, state: ( - + + {state ?? "No task"} + + } + /> ), priority, preemption: String(!!policy?.preemptEnabled), @@ -316,23 +334,28 @@ export const TasksPage: React.FC = () => { id={`row-actions-${task.id}`} > cancelTask(task.id), }, { title: task.policy?.preemptEnabled ? t("actions.disablePreemption") : t("actions.enablePreemption"), - isDisabled: "Running" === task.state, + isDisabled: !canTogglePreemption(task.state), onClick: () => togglePreemption(task), }, + { + title: t("actions.taskDetails"), + onClick: () => + history.push( + formatPath(Paths.taskDetails, { + taskId: task.id, + }) + ), + }, ]} /> diff --git a/client/src/app/pages/tasks/useTaskActions.tsx b/client/src/app/pages/tasks/useTaskActions.tsx index 3d29aca49..33b60e34a 100644 --- a/client/src/app/pages/tasks/useTaskActions.tsx +++ b/client/src/app/pages/tasks/useTaskActions.tsx @@ -4,10 +4,16 @@ import { useCancelTaskMutation, useUpdateTaskMutation, } from "@app/queries/tasks"; -import { Task } from "@app/api/models"; +import { Task, TaskState } from "@app/api/models"; import { NotificationsContext } from "@app/components/NotificationsContext"; import { useTranslation } from "react-i18next"; +const canCancel = (state: TaskState = "No task") => + !["Succeeded", "Failed", "Canceled"].includes(state); + +const canTogglePreemption = (state: TaskState = "No task") => + !["Succeeded", "Failed", "Canceled", "Running"].includes(state); + export const useTaskActions = () => { const { t } = useTranslation(); const { pushNotification } = React.useContext(NotificationsContext); @@ -49,5 +55,5 @@ export const useTaskActions = () => { }, }); - return { cancelTask, togglePreemption }; + return { cancelTask, togglePreemption, canCancel, canTogglePreemption }; };