From 996aa513b05742b3804c027649e50917230b5115 Mon Sep 17 00:00:00 2001 From: Devin Villarosa Date: Tue, 18 Feb 2025 16:45:29 -0800 Subject: [PATCH 1/2] [UI v2] feat: Integrate flow run row deletion to data table --- .../data-table/data-table.stories.tsx | 8 +- .../flow-runs/data-table/data-table.test.tsx | 94 ++++++++++++++ .../flow-runs/data-table/data-table.tsx | 122 +++++++++++++----- .../data-table/use-delete-flow-runs-dialog.ts | 4 +- 4 files changed, 193 insertions(+), 35 deletions(-) create mode 100644 ui-v2/src/components/flow-runs/data-table/data-table.test.tsx diff --git a/ui-v2/src/components/flow-runs/data-table/data-table.stories.tsx b/ui-v2/src/components/flow-runs/data-table/data-table.stories.tsx index 372fc8eaf2ad..19b21f53c3dd 100644 --- a/ui-v2/src/components/flow-runs/data-table/data-table.stories.tsx +++ b/ui-v2/src/components/flow-runs/data-table/data-table.stories.tsx @@ -1,7 +1,11 @@ import type { Meta, StoryObj } from "@storybook/react"; import { createFakeFlowRunWithDeploymentAndFlow } from "@/mocks/create-fake-flow-run"; -import { routerDecorator } from "@/storybook/utils"; +import { + reactQueryDecorator, + routerDecorator, + toastDecorator, +} from "@/storybook/utils"; import { FlowRunsDataTable } from "./data-table"; const MOCK_DATA = Array.from( @@ -11,7 +15,7 @@ const MOCK_DATA = Array.from( const meta: Meta = { title: "Components/FlowRuns/DataTable/FlowRunsDataTable", - decorators: [routerDecorator], + decorators: [routerDecorator, reactQueryDecorator, toastDecorator], args: { flowRuns: MOCK_DATA, flowRunsCount: MOCK_DATA.length }, component: FlowRunsDataTable, }; diff --git a/ui-v2/src/components/flow-runs/data-table/data-table.test.tsx b/ui-v2/src/components/flow-runs/data-table/data-table.test.tsx new file mode 100644 index 000000000000..6256f2bc3cde --- /dev/null +++ b/ui-v2/src/components/flow-runs/data-table/data-table.test.tsx @@ -0,0 +1,94 @@ +import { Toaster } from "@/components/ui/toaster"; +import { createFakeFlowRunWithDeploymentAndFlow } from "@/mocks/create-fake-flow-run"; +import { QueryClient } from "@tanstack/react-query"; +import { + RouterProvider, + createMemoryHistory, + createRootRoute, + createRouter, +} from "@tanstack/react-router"; +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { createWrapper } from "@tests/utils"; +import { describe, expect, it } from "vitest"; +import { FlowRunsDataTable, type FlowRunsDataTableProps } from "./data-table"; + +// Wraps component in test with a Tanstack router provider +const FlowRunsDataTableRouter = (props: FlowRunsDataTableProps) => { + const rootRoute = createRootRoute({ + component: () => , + }); + + const router = createRouter({ + routeTree: rootRoute, + history: createMemoryHistory({ + initialEntries: ["/"], + }), + context: { queryClient: new QueryClient() }, + }); + // @ts-expect-error - Type error from using a test router + return ; +}; + +describe("Flow Runs DataTable", () => { + const MOCK_DATA = [ + createFakeFlowRunWithDeploymentAndFlow(), + createFakeFlowRunWithDeploymentAndFlow(), + createFakeFlowRunWithDeploymentAndFlow(), + ]; + + it("able to delete a single row", async () => { + // Setup + const user = userEvent.setup(); + render( + <> + + + , + { wrapper: createWrapper() }, + ); + expect(screen.getByText(/3 flow runs/i)).toBeVisible(); + const row = screen.getAllByRole("row")[1]; + + // Act + await user.click( + within(row).getByRole("checkbox", { name: /select row/i }), + ); + expect(screen.getByText("1 selected")).toBeVisible(); + + await user.click(screen.getByRole("button", { name: /delete rows/i })); + await user.click(screen.getByRole("button", { name: /delete/i })); + + // Assert + expect(screen.getByText("Flow run deleted")).toBeVisible(); + }); + + it("able to select all rows and delete", async () => { + // Setup + const user = userEvent.setup(); + render( + <> + + + , + { wrapper: createWrapper() }, + ); + expect(screen.getByText(/3 flow runs/i)).toBeVisible(); + + // Act + await user.click(screen.getByRole("checkbox", { name: /select all/i })); + expect(screen.getByText("3 selected")).toBeVisible(); + + await user.click(screen.getByRole("button", { name: /delete rows/i })); + await user.click(screen.getByRole("button", { name: /delete/i })); + + // Assert + expect(screen.getByText("3 flow runs deleted")).toBeVisible(); + }); +}); diff --git a/ui-v2/src/components/flow-runs/data-table/data-table.tsx b/ui-v2/src/components/flow-runs/data-table/data-table.tsx index 0fe89be9bd55..04f6a1b95a75 100644 --- a/ui-v2/src/components/flow-runs/data-table/data-table.tsx +++ b/ui-v2/src/components/flow-runs/data-table/data-table.tsx @@ -4,18 +4,24 @@ import { type FlowRunWithDeploymentAndFlow, type FlowRunWithFlow, } from "@/api/flow-runs"; +import { Button } from "@/components/ui/button"; import { DataTable } from "@/components/ui/data-table"; import { StateBadge } from "@/components/ui/state-badge"; import { TagBadgeGroup } from "@/components/ui/tag-badge-group"; + import { + RowSelectionState, createColumnHelper, getCoreRowModel, getPaginationRowModel, useReactTable, } from "@tanstack/react-table"; -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import { Flow } from "@/api/flows"; +import { Checkbox } from "@/components/ui/checkbox"; +import { DeleteConfirmationDialog } from "@/components/ui/delete-confirmation-dialog"; +import { Icon } from "@/components/ui/icons"; import { Typography } from "@/components/ui/typography"; import { pluralize } from "@/utils"; import { DeploymentCell } from "./deployment-cell"; @@ -26,6 +32,7 @@ import { RunNameSearch } from "./run-name-search"; import { SortFilter } from "./sort-filter"; import { StartTimeCell } from "./start-time-cell"; import { StateFilter } from "./state-filter"; +import { useDeleteFlowRunsDialog } from "./use-delete-flow-runs-dialog"; export type FlowRunsDataTableRow = FlowRun & { flow: Flow; @@ -40,6 +47,27 @@ const createColumns = ({ showDeployment: boolean; }) => { const ret = [ + columnHelper.display({ + id: "select", + header: ({ table }) => ( + + table.toggleAllPageRowsSelected(Boolean(value)) + } + aria-label="Select all" + /> + ), + cell: ({ row }) => ( + row.toggleSelected(Boolean(value))} + aria-label="Select row" + /> + ), + enableSorting: false, + enableHiding: false, + }), columnHelper.display({ id: "name", header: "Name", @@ -104,7 +132,7 @@ const createColumns = ({ return ret; }; -type FlowRunsDataTableProps = { +export type FlowRunsDataTableProps = { flowRunsCount: number; flowRuns: Array; }; @@ -112,12 +140,20 @@ export const FlowRunsDataTable = ({ flowRunsCount, flowRuns, }: FlowRunsDataTableProps) => { + const [rowSelection, setRowSelection] = useState({}); + + const [deleteConfirmationDialogState, confirmDelete] = + useDeleteFlowRunsDialog(); + const showDeployment = useMemo( () => flowRuns.some((flowRun) => "deployment" in flowRun), [flowRuns], ); const table = useReactTable({ + getRowId: (row) => row.id, + onRowSelectionChange: setRowSelection, + state: { rowSelection }, data: flowRuns, columns: createColumns({ showDeployment, @@ -126,37 +162,61 @@ export const FlowRunsDataTable = ({ getPaginationRowModel: getPaginationRowModel(), // TODO: use server-side pagination }); + const numRowsSelected = Object.keys(rowSelection).length; + return ( -
-
-
- - {flowRunsCount} {pluralize(flowRunsCount, "Flow run")} - -
-
- -
-
- {}} - /> + <> +
+
+
+ {numRowsSelected > 0 ? ( +
+ + {numRowsSelected} selected + + +
+ ) : ( + + {flowRunsCount} {pluralize(flowRunsCount, "Flow run")} + + )} +
+
+ +
+
+ {}} + /> +
+
+ {}} + /> +
-
- {}} - /> -
-
- -
+ +
+ + ); }; diff --git a/ui-v2/src/components/flow-runs/data-table/use-delete-flow-runs-dialog.ts b/ui-v2/src/components/flow-runs/data-table/use-delete-flow-runs-dialog.ts index e6985aefa4b3..60e49c80d3ba 100644 --- a/ui-v2/src/components/flow-runs/data-table/use-delete-flow-runs-dialog.ts +++ b/ui-v2/src/components/flow-runs/data-table/use-delete-flow-runs-dialog.ts @@ -29,7 +29,7 @@ export const useDeleteFlowRunsDialog = () => { } else if (numFails === 1) { toast({ title: "Flow run failed to delete" }); } else if (numSuccess > 1) { - toast({ title: `${numSuccess} flow runs to deleted` }); + toast({ title: `${numSuccess} flow runs deleted` }); } else { toast({ title: "Flow run deleted" }); } @@ -41,7 +41,7 @@ export const useDeleteFlowRunsDialog = () => { const handleConfirmDelete = (flowRunIds: Array) => confirmDelete({ - title: "Delete Flow runs", + title: "Delete Flow Runs", description: "Are you sure you want to delete selected flow runs?", onConfirm: () => { void handleDeletes(flowRunIds); From 454ea3661b95a18df738e124179a10e35fecacc7 Mon Sep 17 00:00:00 2001 From: Devin Villarosa Date: Wed, 19 Feb 2025 10:33:22 -0800 Subject: [PATCH 2/2] addressing PR comments --- .../flow-runs/data-table/data-table.tsx | 38 ++++++++++++------- .../flow-runs/data-table/deployment-cell.tsx | 2 +- .../flow-runs/data-table/name-cell.tsx | 2 +- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/ui-v2/src/components/flow-runs/data-table/data-table.tsx b/ui-v2/src/components/flow-runs/data-table/data-table.tsx index 04f6a1b95a75..78e0c7f2c829 100644 --- a/ui-v2/src/components/flow-runs/data-table/data-table.tsx +++ b/ui-v2/src/components/flow-runs/data-table/data-table.tsx @@ -24,6 +24,7 @@ import { DeleteConfirmationDialog } from "@/components/ui/delete-confirmation-di import { Icon } from "@/components/ui/icons"; import { Typography } from "@/components/ui/typography"; import { pluralize } from "@/utils"; +import { CheckedState } from "@radix-ui/react-checkbox"; import { DeploymentCell } from "./deployment-cell"; import { DurationCell } from "./duration-cell"; import { NameCell } from "./name-cell"; @@ -48,16 +49,25 @@ const createColumns = ({ }) => { const ret = [ columnHelper.display({ + size: 40, id: "select", - header: ({ table }) => ( - - table.toggleAllPageRowsSelected(Boolean(value)) - } - aria-label="Select all" - /> - ), + header: ({ table }) => { + let checkedState: CheckedState = false; + if (table.getIsAllRowsSelected()) { + checkedState = true; + } else if (table.getIsSomePageRowsSelected()) { + checkedState = "indeterminate"; + } + return ( + + table.toggleAllPageRowsSelected(Boolean(value)) + } + aria-label="Select all" + /> + ); + }, cell: ({ row }) => ( , @@ -117,6 +128,7 @@ const createColumns = ({ if (showDeployment) { ret.push( columnHelper.display({ + size: 200, id: "deployment", header: "Deployment", cell: ({ row }) => { @@ -162,26 +174,26 @@ export const FlowRunsDataTable = ({ getPaginationRowModel: getPaginationRowModel(), // TODO: use server-side pagination }); - const numRowsSelected = Object.keys(rowSelection).length; + const selectedRows = Object.keys(rowSelection); return ( <>
- {numRowsSelected > 0 ? ( + {selectedRows.length > 0 ? (
- {numRowsSelected} selected + {selectedRows.length} selected diff --git a/ui-v2/src/components/flow-runs/data-table/deployment-cell.tsx b/ui-v2/src/components/flow-runs/data-table/deployment-cell.tsx index ed670b0b53b8..f189118cbbb3 100644 --- a/ui-v2/src/components/flow-runs/data-table/deployment-cell.tsx +++ b/ui-v2/src/components/flow-runs/data-table/deployment-cell.tsx @@ -9,7 +9,7 @@ type DeploymentCellProps = { deployment: Deployment }; export const DeploymentCell = ({ deployment }: DeploymentCellProps) => { return ( diff --git a/ui-v2/src/components/flow-runs/data-table/name-cell.tsx b/ui-v2/src/components/flow-runs/data-table/name-cell.tsx index f9f9235e5c8a..655269572ced 100644 --- a/ui-v2/src/components/flow-runs/data-table/name-cell.tsx +++ b/ui-v2/src/components/flow-runs/data-table/name-cell.tsx @@ -13,7 +13,7 @@ type NameCellProps = { export const NameCell = ({ flowRun }: NameCellProps) => { return ( -
+