Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add Tasks tab to the Applications table application details drawer #2123

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

✨ Add Tasks tab to the Applications table application details drawer #2123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c08d206
Add Tasks tab to the Applications table
EstyBiton Sep 17, 2024
7fc6cbb
breadcrumb navigation back to the application table
EstyBiton Sep 25, 2024
48565da
Update breadcrumbs for improved navigation
EstyBiton Sep 26, 2024
fc085f3
Add Tasks tab to the Applications table application details drawer
EstyBiton Oct 8, 2024
033323d
Fix tasks tab in detail drawer
EstyBiton Nov 3, 2024
cdd77a7
Merge branch 'main' into tasks-tab-drawer
rszwajko Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions client/src/app/Paths.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export const DevPaths = {
applications: "/applications",
applicationsAnalysisDetails:
"/applications/:applicationId/analysis-details/:taskId",
applicationsTaskDetails: "/applications/:applicationId/tasks/:taskId",
applicationsAnalysisDetailsAttachment:
"/applications/:applicationId/analysis-details/:taskId/attachments/:attachmentId",
applicationsAnalysisTab: "/applications/analysis-tab",
Expand Down Expand Up @@ -108,4 +109,5 @@ export interface AnalysisDetailsAttachmentRoute {
export interface TaskDetailsAttachmentRoute {
taskId: string;
attachmentId: string;
applicationId: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this change the interface is the same as AnalysisDetailsAttachmentRoute (just above).

}
5 changes: 5 additions & 0 deletions client/src/app/Routes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ export const devRoutes: IRoute<DevPathValues>[] = [
comp: AnalysisDetails,
exact: true,
},
{
path: Paths.applicationsTaskDetails,
comp: TaskDetails,
exact: true,
},
{
path: Paths.applicationsAnalysisDetailsAttachment,
comp: AnalysisDetails,
Expand Down
1 change: 1 addition & 0 deletions client/src/app/components/task-manager/TaskManagerDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ const TaskItem: React.FC<{
: `${task.id} (${task.addon}) - ${task.applicationName} - ${
task.priority ?? 0
}`;

const taskActionItems = useTaskActions(task._);

return (
Expand Down
238 changes: 237 additions & 1 deletion ...app/pages/applications/components/application-detail-drawer/application-detail-drawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,29 @@ import { useFetchArchetypes } from "@app/queries/archetypes";
import { useFetchAssessments } from "@app/queries/assessments";
import { DecoratedApplication } from "../../applications-table/useDecoratedApplications";
import { TaskStates } from "@app/queries/tasks";
import { Toolbar, ToolbarContent, ToolbarItem } from "@patternfly/react-core";
import { Table, Tbody, Td, Th, Thead, Tr } from "@patternfly/react-table";
import {
useTableControlState,
useTableControlProps,
} from "@app/hooks/table-controls";
import { SimplePagination } from "@app/components/SimplePagination";
import { useServerTasks } from "@app/queries/tasks";
import { FilterToolbar, FilterType } from "@app/components/FilterToolbar";
import {
getHubRequestParams,
deserializeFilterUrlParams,
} from "@app/hooks/table-controls";
import { useSelectionState } from "@migtools/lib-ui";
import { TablePersistenceKeyPrefix } from "@app/Constants";
import { TaskActionColumn } from "../../../../pages/tasks/TaskActionColumn";
import {
ConditionalTableBody,
TableHeaderContentWithControls,
TableRowContentWithControls,
} from "@app/components/TableControls";
import { IconWithLabel, TaskStateIcon } from "@app/components/Icons";
import { taskStateToLabel } from "@app/pages/tasks/tasks-page";

export interface IApplicationDetailDrawerProps
extends Pick<IPageDrawerContentProps, "onCloseClick"> {
Expand All @@ -73,18 +96,20 @@ export interface IApplicationDetailDrawerProps
onEditClick: () => void;
}

enum TabKey {
export enum TabKey {
Details = 0,
Tags,
Reports,
Facts,
Reviews,
Tasks,
}

export const ApplicationDetailDrawer: React.FC<
IApplicationDetailDrawerProps
> = ({ application, task, onCloseClick, onEditClick }) => {
const { t } = useTranslation();

const [activeTabKey, setActiveTabKey] = React.useState<TabKey>(
TabKey.Details
);
Expand Down Expand Up @@ -152,6 +177,14 @@ export const ApplicationDetailDrawer: React.FC<
<ReviewFields application={application} />
</Tab>
)}
{!application ? null : (
<Tab
eventKey={TabKey.Tasks}
title={<TabTitleText>{t("terms.tasks")}</TabTitleText>}
>
<TabTasksContent application={application} task={task} />
</Tab>
)}
</Tabs>
</div>
</PageDrawerContent>
Expand Down Expand Up @@ -521,3 +554,206 @@ const TabReportsContent: React.FC<{
</>
);
};

const TabTasksContent: React.FC<{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we should be able to reuse the code from TasksPage. In the current state this requires a refactoring that would extract the "base" component. The best strategy would be to use a different PR for that and merge it first. This way we (and our test suite) ensure that the refactoring caused no regression.

After the refactoring both TasksPage and TabTasksContent should use the base and provide the custom parts. There are few differences:

  1. identifiers i.e. tableName, persistenceKeyPrefix
  2. visible columns - note that hiding (via initialColumns) looks easier then maintaining 2 lists of columns
  3. query params - implicitFilters
  4. possibly more

If "base" component variant would be not achievable we can break the TasksPage into smaller parts and try to reuse those.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rszwajko Thank you for the feedback! I would really like to continue working on this task, but I currently have a challenge. My internship with the company has concluded, and I no longer have access to the computer where the product environment was properly set up. Unfortunately, running Konveyor on Minikube in the Windows operating system presents a significant issue: the local Konveyor UI is inaccessible.
If the issue with installing Konveyor is resolved, I’d be happy to continue working on this task. Please let me know.

application: DecoratedApplication;
task: TaskDashboard | null;
}> = ({ application, task }) => {
const { t } = useTranslation();
const history = useHistory();
const urlParams = new URLSearchParams(window.location.search);
const filters = urlParams.get("filters");
const deserializedFilterValues = deserializeFilterUrlParams({ filters });
const tableControlState = useTableControlState({
tableName: "tasks-apps-table",
persistTo: "urlParams",
persistenceKeyPrefix: TablePersistenceKeyPrefix.tasks,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems we have a collision with TasksPage

columnNames: {
taskId: "Task ID",
taskKind: "Task Kind",
status: "Status",
},
isFilterEnabled: true,
isSortEnabled: true,
isPaginationEnabled: true,
sortableColumns: ["taskId", "taskKind", "status"],
initialSort: { columnKey: "taskId", direction: "asc" },
initialFilterValues: deserializedFilterValues,
filterCategories: [
{
categoryKey: "id",
title: "ID",
type: FilterType.numsearch,
placeholderText: t("actions.filterBy", {
what: "ID...",
}),
getServerFilterValue: (value) => {
console.log("this id:", value);
return value ? value : [];
},
},
{
categoryKey: "kind",
title: t("terms.kind"),
type: FilterType.search,
placeholderText: t("actions.filterBy", {
what: t("terms.kind") + "...",
}),
getServerFilterValue: (value) => (value ? [`*${value[0]}*`] : []),
},
{
categoryKey: "state",
title: t("terms.status"),
type: FilterType.search,
placeholderText: t("actions.filterBy", {
what: t("terms.status") + "...",
}),
getServerFilterValue: (value) => (value ? [`*${value[0]}*`] : []),
},
],
initialItemsPerPage: 10,
});

const {
result: { data: currentPageItems = [], total: totalItemCount },
isFetching,
fetchError,
} = useServerTasks(
getHubRequestParams({
...tableControlState,
hubSortFieldKeys: {
taskId: "id",
taskKind: "kind",
status: "status",
},
implicitFilters: [
{
field: "application.id",
operator: "=",
value: application.id,
},
],
}),
5000
);
const tableControls = useTableControlProps({
...tableControlState,
idProperty: "id",
currentPageItems: currentPageItems,
totalItemCount,
isLoading: isFetching,
variant: "compact",
selectionState: useSelectionState({
items: currentPageItems,
isEqual: (a, b) => a.name === b.name,
}),
});

const {
numRenderedColumns,
propHelpers: {
toolbarProps,
filterToolbarProps,
paginationToolbarItemProps,
paginationProps,
tableProps,
getThProps,
getTrProps,
getTdProps,
},
} = tableControls;

const clearFilters = () => {
const currentPath = history.location.pathname;
const newSearch = new URLSearchParams(history.location.search);
newSearch.delete("filters");
history.push(`${currentPath}`);
};
return (
<>
<Toolbar
{...toolbarProps}
className={spacing.mtSm}
clearAllFilters={clearFilters}
>
<ToolbarContent>
<FilterToolbar {...filterToolbarProps} />
<ToolbarItem {...paginationToolbarItemProps}>
<SimplePagination
idPrefix="task-apps-table"
isTop
isCompact
paginationProps={paginationProps}
/>
</ToolbarItem>
</ToolbarContent>
</Toolbar>
<Table {...tableProps} aria-label="task applications table">
<Thead>
<Tr>
<TableHeaderContentWithControls {...tableControls}>
<Th {...getThProps({ columnKey: "taskId" })} />
<Th
{...getThProps({ columnKey: "taskKind" })}
modifier="nowrap"
/>
<Th {...getThProps({ columnKey: "status" })} modifier="nowrap" />
</TableHeaderContentWithControls>
</Tr>
</Thead>

<ConditionalTableBody
isLoading={isFetching}
isError={!!fetchError}
isNoData={totalItemCount === 0}
numRenderedColumns={numRenderedColumns}
>
<Tbody>
{currentPageItems?.map((task, rowIndex) => (
<Tr key={task.id} {...getTrProps({ item: task })}>
<TableRowContentWithControls
{...tableControls}
item={task}
rowIndex={rowIndex}
>
<Td {...getTdProps({ columnKey: "taskId" })}>{task.id}</Td>
<Td {...getTdProps({ columnKey: "taskKind" })}>
{task.kind}
</Td>
<Td {...getTdProps({ columnKey: "status" })}>
<IconWithLabel
icon={<TaskStateIcon state={task.state} />}
label={
<Link
to={formatPath(Paths.applicationsTaskDetails, {
applicationId: application.id,
taskId: task.id,
})}
>
{t(taskStateToLabel[task.state ?? "No task"])}
</Link>
}
/>
</Td>
<Td
key={`row-actions-${task.id}`}
isActionCell
id={`row-actions-${task.id}`}
>
<TaskActionColumn task={task} />
</Td>
</TableRowContentWithControls>
</Tr>
))}
</Tbody>
</ConditionalTableBody>
</Table>
<SimplePagination
idPrefix="task-apps-table"
isTop={false}
isCompact
paginationProps={paginationProps}
/>
</>
);
};
31 changes: 25 additions & 6 deletions client/src/app/pages/tasks/TaskDetails.tsx
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that after the changes the file duplicates the functionality of AnalysisDetails - in fact analysis details is just a task that is highlighted for user convenience. The code behind is/was generic - if you replace the IDs in the URL you should be able to view any task. Ideally you should be able to re-use this code after extracting to some "base" class - similar how both TaskDetails and AnalysisDetails use TaskDetailsBase. IMHO the refactoring (extracting base functionality) is small enough to do in this PR.

It could look the following

const  AnalysisDetails = () =>   
  <GoodNameForBaseComponent  
    formatDetailsPath={(applicationId,taskId) => formatPath(Paths.applicationsAnalysisDetails, {
        applicationId,
        taskId,
      });
   formatTitle={(taskName) => `Analysis details for ${taskName}`}
// whatever else is needed
  />  

const  ApplicationTaskDetails = () =>   
  <GoodNameForBaseComponent  
    formatDetailsPath={(applicationId,taskId) => ...);
   formatTitle={(taskName) => ...}
// whatever else is needed
  />

Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,43 @@ import { Paths, TaskDetailsAttachmentRoute } from "@app/Paths";
import "@app/components/simple-document-viewer/SimpleDocumentViewer.css";
import { formatPath } from "@app/utils/utils";
import { TaskDetailsBase } from "./TaskDetailsBase";
import { useFetchApplicationById } from "@app/queries/applications";

export const TaskDetails = () => {
const { t } = useTranslation();
const { taskId, attachmentId } = useParams<TaskDetailsAttachmentRoute>();
const detailsPath = formatPath(Paths.taskDetails, { taskId });
const { taskId, attachmentId, applicationId } =
useParams<TaskDetailsAttachmentRoute>();
const currentPath = window.location.pathname;
const isFromApplication = currentPath.includes("application") ? true : false;
const { application } = useFetchApplicationById(applicationId);

const appName: string = application?.name ?? t("terms.unknown");
console.log(appName);
const detailsPath = isFromApplication
? formatPath(Paths.applicationsTaskDetails, {
applicationId: applicationId,
taskId: taskId,
})
: formatPath(Paths.taskDetails, { taskId });

return (
<TaskDetailsBase
breadcrumbs={[
{
title: t("terms.tasks"),
path: Paths.tasks,
title: t(isFromApplication ? "terms.applications" : "terms.tasks"),
path: isFromApplication ? Paths.applications : Paths.tasks,
},
isFromApplication
? {
title: appName,
path: `${Paths.applications}/?activeItem=${applicationId}`,
}
: null,
{
title: t("titles.taskWithId", { taskId }),
path: detailsPath,
},
]}
].filter(Boolean)}
detailsPath={detailsPath}
formatTitle={(taskName) => `Task details for task ${taskId}, ${taskName}`}
formatAttachmentPath={(attachmentId) =>
Expand All @@ -36,5 +56,4 @@ export const TaskDetails = () => {
/>
);
};

export default TaskDetails;
3 changes: 1 addition & 2 deletions client/src/app/pages/tasks/tasks-page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { formatPath } from "@app/utils/utils";
import { Paths } from "@app/Paths";
import { TaskActionColumn } from "./TaskActionColumn";

const taskStateToLabel: Record<TaskState, string> = {
export const taskStateToLabel: Record<TaskState, string> = {
"No task": "taskState.NoTask",
"not supported": "",
Canceled: "taskState.Canceled",
Expand All @@ -71,7 +71,6 @@ export const TasksPage: React.FC = () => {

const urlParams = new URLSearchParams(window.location.search);
const filters = urlParams.get("filters") ?? "";

const deserializedFilterValues = deserializeFilterUrlParams({ filters });

const tableControlState = useTableControlState({
Expand Down
Loading
Loading