From 82bb9865ef25a3910ecf297063cef79df47ecbff Mon Sep 17 00:00:00 2001 From: alex-mcgovern Date: Thu, 30 Jan 2025 15:19:46 +0000 Subject: [PATCH 1/3] feat(alerts): tabs for filtering table --- src/App.tsx | 5 +- .../alerts/components/search-field-alerts.tsx | 3 +- .../switch-malicious-alerts-filter.tsx | 34 ----- .../alerts/components/table-alerts.tsx | 123 ++++++++---------- .../alerts/components/tabs-alerts.tsx | 118 +++++++++++++++++ .../hooks/use-alerts-filter-search-params.ts | 29 ++++- src/hooks/useAlertSearch.ts | 23 ---- src/routes/__tests__/route-dashboard.test.tsx | 88 +++++++++---- src/routes/route-dashboard.tsx | 10 +- src/types.ts | 34 ----- 10 files changed, 272 insertions(+), 195 deletions(-) delete mode 100644 src/features/alerts/components/switch-malicious-alerts-filter.tsx create mode 100644 src/features/alerts/components/tabs-alerts.tsx delete mode 100644 src/hooks/useAlertSearch.ts delete mode 100644 src/types.ts diff --git a/src/App.tsx b/src/App.tsx index e9f9922b..e03ecedf 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -17,7 +17,10 @@ function App() {
-
+
diff --git a/src/features/alerts/components/search-field-alerts.tsx b/src/features/alerts/components/search-field-alerts.tsx index 7d084de6..7a1a7a7b 100644 --- a/src/features/alerts/components/search-field-alerts.tsx +++ b/src/features/alerts/components/search-field-alerts.tsx @@ -7,7 +7,7 @@ import { import { useAlertsFilterSearchParams } from "../hooks/use-alerts-filter-search-params"; import { SearchMd } from "@untitled-ui/icons-react"; -export function SearchFieldAlerts() { +export function SearchFieldAlerts({ className }: { className?: string }) { const { setSearch, state } = useAlertsFilterSearchParams(); return ( @@ -16,6 +16,7 @@ export function SearchFieldAlerts() { aria-label="Search alerts" value={state.search ?? ""} onChange={(value) => setSearch(value.toLowerCase().trim())} + className={className} > - { - switch (isSelected) { - case true: - return setView(AlertsFilterView.MALICIOUS); - case false: - return setView(AlertsFilterView.ALL); - default: - return isSelected satisfies never; - } - }} - > - Malicious Packages - - - -

Filter by malicious packages

- - - ); -} diff --git a/src/features/alerts/components/table-alerts.tsx b/src/features/alerts/components/table-alerts.tsx index 683a8be4..a22e4b56 100644 --- a/src/features/alerts/components/table-alerts.tsx +++ b/src/features/alerts/components/table-alerts.tsx @@ -6,7 +6,6 @@ import { Table, TableBody, TableHeader, - Badge, Button, ResizableTableContainer, } from "@stacklok/ui-kit"; @@ -16,14 +15,12 @@ import { parsingPromptText, getIssueDetectedType, } from "@/lib/utils"; -import { useAlertSearch } from "@/hooks/useAlertSearch"; import { useNavigate } from "react-router-dom"; import { useClientSidePagination } from "@/hooks/useClientSidePagination"; import { TableAlertTokenUsage } from "./table-alert-token-usage"; import { Key01, PackageX } from "@untitled-ui/icons-react"; -import { SearchFieldAlerts } from "./search-field-alerts"; import { useQueryGetWorkspaceAlertTable } from "../hooks/use-query-get-workspace-alerts-table"; -import { SwitchMaliciousAlertsFilter } from "./switch-malicious-alerts-filter"; +import { useAlertsFilterSearchParams } from "../hooks/use-alerts-filter-search-params"; const getTitle = (alert: AlertConversation) => { const prompt = alert.conversation; @@ -75,84 +72,70 @@ function IssueDetectedCellContent({ alert }: { alert: AlertConversation }) { } export function TableAlerts() { - const { page, nextPage, prevPage } = useAlertSearch(); const navigate = useNavigate(); + const { state, prevPage, nextPage } = useAlertsFilterSearchParams(); + const { data: filteredAlerts = [] } = useQueryGetWorkspaceAlertTable(); const { dataView, hasNextPage, hasPreviousPage } = useClientSidePagination( filteredAlerts, - page, + state.page, 15, ); return ( <> -
-
-

All Alerts

- - {filteredAlerts.length} - -
- -
- - -
-
-
- - - - - - Time - - Type - Event - Issue Detected - Token usage - - - - {dataView.map((alert) => { - return ( - - navigate(`/prompt/${alert.conversation.chat_id}`) - } - > - - {formatDistanceToNow(new Date(alert.timestamp), { - addSuffix: true, - })} - - - - - {getTitle(alert)} - -
- -
- - - - - - ); - })} - -
- -
+ + + + + + Time + + Type + Event + Issue Detected + Token usage + + + + {dataView.map((alert) => { + return ( + + navigate(`/prompt/${alert.conversation.chat_id}`) + } + > + + {formatDistanceToNow(new Date(alert.timestamp), { + addSuffix: true, + })} + + + + + {getTitle(alert)} + +
+ +
+ + + + + + ); + })} + +
+
-
+
diff --git a/src/features/alerts/components/tabs-alerts.tsx b/src/features/alerts/components/tabs-alerts.tsx new file mode 100644 index 00000000..a14f9f35 --- /dev/null +++ b/src/features/alerts/components/tabs-alerts.tsx @@ -0,0 +1,118 @@ +import { useQueryGetWorkspaceAlerts } from "../hooks/use-query-get-workspace-alerts"; +import { isAlertMalicious } from "../lib/is-alert-malicious"; +import { multiFilter } from "@/lib/multi-filter"; +import { isAlertCritical } from "../lib/is-alert-critical"; +import { isAlertSecret } from "../lib/is-alert-secret"; +import { V1GetWorkspaceAlertsResponse } from "@/api/generated"; +import { + Tab as BaseTab, + Tabs, + TabList, + TabPanel, + Badge, + Card, + CardBody, +} from "@stacklok/ui-kit"; +import { + AlertsFilterView, + useAlertsFilterSearchParams, +} from "../hooks/use-alerts-filter-search-params"; +import { SearchFieldAlerts } from "./search-field-alerts"; +import { tv } from "tailwind-variants"; + +type AlertsCount = { + all: number; + malicious: number; + secrets: number; +}; + +function select(data: V1GetWorkspaceAlertsResponse | undefined): AlertsCount { + const all: number = multiFilter(data, [isAlertCritical]).length; + + const malicious: number = multiFilter(data, [ + isAlertCritical, + isAlertMalicious, + ]).length; + + const secrets: number = multiFilter(data, [ + isAlertCritical, + isAlertSecret, + ]).length; + + return { + all, + malicious, + secrets, + }; +} + +const tabStyle = tv({ + base: [ + "my-1 mx-0.5 first:ml-1 last:mr-1", + "rounded bg-transparent h-[calc(2rem-2px)] flex text-secondary items-center gap-1 !border-0", + "hover:bg-gray-50 hover:text-secondary", + "selected:bg-base hover:selected:bg-base selected:shadow-sm selected:border-gray-200 selected:text-secondary", + ], +}); + +function Tab({ + id, + title, + count, +}: { + title: string; + id: AlertsFilterView; + count: number; +}) { + return ( + + {title} + + {count} + + + ); +} + +export function TabsAlerts({ children }: { children: React.ReactNode }) { + const { state, setView } = useAlertsFilterSearchParams(); + + const { data } = useQueryGetWorkspaceAlerts({ + select, + }); + + return ( + setView(key.toString() as AlertsFilterView)} + selectedKey={state.view} + defaultSelectedKey={AlertsFilterView.ALL} + > +
+ + + + + + + +
+ + + {children} + + + + ); +} diff --git a/src/features/alerts/hooks/use-alerts-filter-search-params.ts b/src/features/alerts/hooks/use-alerts-filter-search-params.ts index 6fe5c200..a6e700dc 100644 --- a/src/features/alerts/hooks/use-alerts-filter-search-params.ts +++ b/src/features/alerts/hooks/use-alerts-filter-search-params.ts @@ -10,14 +10,15 @@ export enum AlertsFilterView { const alertsFilterSchema = z.object({ search: z.string().optional(), - view: z.nativeEnum(AlertsFilterView), + view: z.nativeEnum(AlertsFilterView).optional().default(AlertsFilterView.ALL), + page: z.coerce.number().optional().default(0), }); -type AlertsFilterSchema = z.output; +type AlertsFilterSchema = z.input; -const DEFAULT_FILTER: AlertsFilterSchema = { +const DEFAULT_FILTER = { view: AlertsFilterView.ALL, -}; +} as const satisfies AlertsFilterSchema; export const useAlertsFilterSearchParams = () => { const [searchParams, setSearchParams] = useSearchParams( @@ -29,6 +30,8 @@ export const useAlertsFilterSearchParams = () => { setSearchParams((prev) => { if (view) prev.set("view", view); if (!view) prev.delete("view"); + + prev.delete("page"); return prev; }); }, @@ -46,7 +49,23 @@ export const useAlertsFilterSearchParams = () => { [setSearchParams], ); + const nextPage = useCallback(() => { + setSearchParams((prev) => { + const page = Number(prev.get("page") ?? 0); + prev.set("page", (page + 1).toString()); + return prev; + }); + }, [setSearchParams]); + + const prevPage = useCallback(() => { + setSearchParams((prev) => { + const page = Number(prev.get("page") ?? 0); + prev.set("page", (page - 1).toString()); + return prev; + }); + }, [setSearchParams]); + const state = alertsFilterSchema.parse(Object.fromEntries(searchParams)); - return { state, setView, setSearch }; + return { state, setView, setSearch, nextPage, prevPage }; }; diff --git a/src/hooks/useAlertSearch.ts b/src/hooks/useAlertSearch.ts deleted file mode 100644 index cfc1fb08..00000000 --- a/src/hooks/useAlertSearch.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { create } from "zustand"; -import { AlertSearchState } from "../types"; - -export const useAlertSearch = create((set) => ({ - isMaliciousFilterActive: false, - search: "", - setSearch: (search: string) => { - set({ search, page: 0 }); - }, - setIsMaliciousFilterActive: (isActive: boolean) => { - set({ - isMaliciousFilterActive: isActive, - page: 0, - }); - }, - page: 0, - nextPage: () => { - set((state) => ({ page: state.page + 1 })); - }, - prevPage: () => { - set((state) => ({ page: state.page - 1 })); - }, -})); diff --git a/src/routes/__tests__/route-dashboard.test.tsx b/src/routes/__tests__/route-dashboard.test.tsx index 224dacc0..68591485 100644 --- a/src/routes/__tests__/route-dashboard.test.tsx +++ b/src/routes/__tests__/route-dashboard.test.tsx @@ -122,11 +122,11 @@ describe("Dashboard", () => { render(); expect( - screen.getByRole("heading", { - name: /all alerts/i, + screen.getByRole("grid", { + name: /alerts table/i, }), ).toBeVisible(); - expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("0"); + expect(screen.getByTestId(/tab-all-count/i)).toHaveTextContent("0"); expect( screen.getByRole("columnheader", { @@ -157,11 +157,6 @@ describe("Dashboard", () => { }), ).toBeVisible(); - expect( - screen.getByRole("switch", { - name: /malicious packages/i, - }), - ).toBeVisible(); expect(screen.getByRole("searchbox")).toBeVisible(); await waitFor(() => { @@ -219,7 +214,7 @@ describe("Dashboard", () => { ).toBeGreaterThan(1); }); - expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("2"); + expect(screen.getByTestId(/tab-all-count/i)).toHaveTextContent("2"); expect( screen.getAllByRole("gridcell", { name: /chat/i, @@ -227,29 +222,78 @@ describe("Dashboard", () => { ).toBeGreaterThanOrEqual(1); userEvent.click( - screen.getByRole("switch", { - name: /malicious packages/i, + screen.getByRole("tab", { + name: /malicious/i, }), ); - await waitFor(() => - expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("1"), + await waitFor(() => { + expect( + screen.queryAllByRole("gridcell", { + name: /blocked secret exposure/i, + }).length, + ).toBe(0); + }); + + userEvent.click( + screen.getByRole("tab", { + name: /all/i, + }), ); + await waitFor(() => { + expect( + screen.queryAllByRole("gridcell", { + name: /blocked secret exposure/i, + }).length, + ).toBe(1); + }); + }); + + it("should filter by secrets", async () => { + mockAlertsWithMaliciousPkg(); + render(); + + await waitFor(() => { + expect( + within(screen.getByTestId("alerts-table")).getAllByRole("row").length, + ).toBeGreaterThan(1); + }); + + expect(screen.getByTestId(/tab-all-count/i)).toHaveTextContent("2"); expect( - screen.queryAllByRole("gridcell", { - name: /blocked secret exposure/i, + screen.getAllByRole("gridcell", { + name: /chat/i, }).length, - ).toBe(0); + ).toBeGreaterThanOrEqual(1); userEvent.click( - screen.getByRole("switch", { - name: /malicious packages/i, + screen.getByRole("tab", { + name: /secrets/i, }), ); - await waitFor(() => - expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("2"), + + await waitFor(() => { + expect( + screen.queryAllByRole("gridcell", { + name: /blocked malicious package/i, + }).length, + ).toBe(0); + }); + + userEvent.click( + screen.getByRole("tab", { + name: /all/i, + }), ); + + await waitFor(() => { + expect( + screen.queryAllByRole("gridcell", { + name: /blocked malicious package/i, + }).length, + ).toBe(1); + }); }); it("should search by secrets alert", async () => { @@ -262,7 +306,7 @@ describe("Dashboard", () => { ).toBeGreaterThan(1); }); - expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("2"); + expect(screen.getByTestId(/tab-all-count/i)).toHaveTextContent("2"); expect( screen.getAllByRole("gridcell", { name: /chat/i, @@ -272,7 +316,7 @@ describe("Dashboard", () => { await userEvent.type(screen.getByRole("searchbox"), "codegate-secrets"); waitFor(() => - expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("1"), + expect(screen.getByTestId(/tab-all-count/i)).toHaveTextContent("1"), ); const row = within(screen.getByTestId("alerts-table")).getAllByRole( "row", diff --git a/src/routes/route-dashboard.tsx b/src/routes/route-dashboard.tsx index c8345091..2625e332 100644 --- a/src/routes/route-dashboard.tsx +++ b/src/routes/route-dashboard.tsx @@ -1,21 +1,21 @@ -import { Separator } from "@stacklok/ui-kit"; import { TableAlerts } from "@/features/alerts/components/table-alerts"; import { AlertsSummaryMaliciousPkg } from "@/features/alerts/components/alerts-summary-malicious-pkg"; import { AlertsSummaryWorkspaceTokenUsage } from "@/features/alerts/components/alerts-summary-workspace-token-usage"; import { AlertsSummaryMaliciousSecrets } from "@/features/alerts/components/alerts-summary-secrets"; +import { TabsAlerts } from "@/features/alerts/components/tabs-alerts"; export function RouteDashboard() { return (
-
+
- - - + + +
); } diff --git a/src/types.ts b/src/types.ts deleted file mode 100644 index c1fa2ddf..00000000 --- a/src/types.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { Conversation } from "./api/generated"; - -export type PromptState = { - prompts: Conversation[]; - loading: boolean; - currentPromptId: string; - setCurrentPromptId: (id: string) => void; - fetchPrompts: () => void; -}; - -export type AlertSearchState = { - isMaliciousFilterActive: boolean; - search: string; - setSearch: (search: string) => void; - setIsMaliciousFilterActive: (isChecked: boolean) => void; - page: number; - nextPage: () => void; - prevPage: () => void; -}; - -export type TriggerType = - | "codegate-version" - | "codegate-context-retriever" - | "system-prompt" - | "code-snippet-extractor" - | "codegate-secrets" - | string; - -export type MaliciousPkgType = { - name: string; - type: string; - status: string; - description: string; -}; From f28b9ff9dcf7931abb188e27d3a47a11803914de Mon Sep 17 00:00:00 2001 From: alex-mcgovern Date: Fri, 31 Jan 2025 08:46:47 +0000 Subject: [PATCH 2/3] fix: header spacing bug --- src/features/header/components/header.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/features/header/components/header.tsx b/src/features/header/components/header.tsx index 40e11259..ce7f7c2e 100644 --- a/src/features/header/components/header.tsx +++ b/src/features/header/components/header.tsx @@ -37,7 +37,7 @@ export function Header({ hasError }: { hasError?: boolean }) {
-
+
From 1c953bca83f5b5b8a5dc07c57cb9c3329cd42d09 Mon Sep 17 00:00:00 2001 From: alex-mcgovern Date: Fri, 31 Jan 2025 09:25:07 +0000 Subject: [PATCH 3/3] fix: wrong count in tab --- .../components/__tests__/tabs-alerts.test.tsx | 72 +++++++++++++++++++ .../alerts/components/tabs-alerts.tsx | 2 +- 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 src/features/alerts/components/__tests__/tabs-alerts.test.tsx diff --git a/src/features/alerts/components/__tests__/tabs-alerts.test.tsx b/src/features/alerts/components/__tests__/tabs-alerts.test.tsx new file mode 100644 index 00000000..913647d4 --- /dev/null +++ b/src/features/alerts/components/__tests__/tabs-alerts.test.tsx @@ -0,0 +1,72 @@ +import { server } from "@/mocks/msw/node"; +import { http, HttpResponse } from "msw"; +import { makeMockAlert } from "../../mocks/alert.mock"; +import { render, waitFor } from "@/lib/test-utils"; +import { TabsAlerts } from "../tabs-alerts"; + +test("shows correct count of all packages", async () => { + server.use( + http.get("*/workspaces/:name/alerts", () => { + return HttpResponse.json([ + ...Array.from({ length: 13 }).map(() => + makeMockAlert({ type: "secret" }), + ), + ...Array.from({ length: 13 }).map(() => + makeMockAlert({ type: "malicious" }), + ), + ]); + }), + ); + + const { getByRole } = render( + +
foo
+ , + ); + + await waitFor(() => { + expect(getByRole("tab", { name: /all/i })).toHaveTextContent("26"); + }); +}); + +test("shows correct count of malicious packages", async () => { + server.use( + http.get("*/workspaces/:name/alerts", () => { + return HttpResponse.json( + Array.from({ length: 13 }).map(() => + makeMockAlert({ type: "malicious" }), + ), + ); + }), + ); + + const { getByRole } = render( + +
foo
+ , + ); + + await waitFor(() => { + expect(getByRole("tab", { name: /malicious/i })).toHaveTextContent("13"); + }); +}); + +test("shows correct count of secret packages", async () => { + server.use( + http.get("*/workspaces/:name/alerts", () => { + return HttpResponse.json( + Array.from({ length: 13 }).map(() => makeMockAlert({ type: "secret" })), + ); + }), + ); + + const { getByRole } = render( + +
foo
+ , + ); + + await waitFor(() => { + expect(getByRole("tab", { name: /secrets/i })).toHaveTextContent("13"); + }); +}); diff --git a/src/features/alerts/components/tabs-alerts.tsx b/src/features/alerts/components/tabs-alerts.tsx index a14f9f35..f5bf0523 100644 --- a/src/features/alerts/components/tabs-alerts.tsx +++ b/src/features/alerts/components/tabs-alerts.tsx @@ -96,7 +96,7 @@ export function TabsAlerts({ children }: { children: React.ReactNode }) {