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

feat: implement new table design #189

Merged
merged 20 commits into from
Jan 27, 2025
Merged

feat: implement new table design #189

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
001139a
rename timestamp coloumn to time
kantord Jan 23, 2025
4c2c604
make 'time' the first column
kantord Jan 23, 2025
474f9a2
use relative date time format in alerts table
kantord Jan 23, 2025
e68e4c2
rename trigger type column to type
kantord Jan 23, 2025
70ceabb
clamp trigger markdown to a single lineoverflowoverflow
kantord Jan 23, 2025
fa48c6e
remove code and file columns from alerts table
kantord Jan 23, 2025
a956b9e
rename trigger token column to event
kantord Jan 23, 2025
bc6542f
.
kantord Jan 23, 2025
04860b3
implement event column content correctly
kantord Jan 23, 2025
5e92cbe
implement type column values
kantord Jan 23, 2025
f7c9c73
add note about ongoing discussion on message type
kantord Jan 24, 2025
3a778b2
fix mapping alerts type
kantord Jan 24, 2025
2a742d8
Merge branch 'main' into alerts-table-refactor
kantord Jan 24, 2025
c07eeac
display detected problem properly
kantord Jan 24, 2025
7d55f72
Merge branch 'main' into alerts-table-refactor
kantord Jan 24, 2025
b75fc79
configure alerts table width
kantord Jan 24, 2025
60bda23
fix table column sizing using <ResizableTableContainer />
kantord Jan 27, 2025
ca3e01c
Merge branch 'main' into alerts-table-refactor
kantord Jan 27, 2025
e0fec1a
fix failing test
kantord Jan 27, 2025
25369f8
make icon blue in alerts table
kantord Jan 27, 2025
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
127 changes: 64 additions & 63 deletions src/components/AlertsTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { format } from "date-fns";
import { formatDistanceToNow } from "date-fns";
import {
Cell,
Column,
Expand All @@ -14,56 +14,68 @@ import {
Button,
} from "@stacklok/ui-kit";
import { Switch } from "@stacklok/ui-kit";
import { AlertConversation } from "@/api/generated";
import { AlertConversation, QuestionType } from "@/api/generated";
import { Tooltip, TooltipTrigger } from "@stacklok/ui-kit";
import { getMaliciousPackage } from "@/lib/utils";
import { Search } from "lucide-react";
import { Markdown } from "./Markdown";
import {
sanitizeQuestionPrompt,
parsingPromptText,
getIssueDetectedType,
} from "@/lib/utils";
import { KeyRoundIcon, PackageX, Search } from "lucide-react";
import { useAlertSearch } from "@/hooks/useAlertSearch";
import { useCallback } from "react";
import { useNavigate, useSearchParams } from "react-router-dom";
import { useFilteredAlerts } from "@/hooks/useAlertsData";
import { useClientSidePagination } from "@/hooks/useClientSidePagination";

const wrapObjectOutput = (input: AlertConversation["trigger_string"]) => {
const data = getMaliciousPackage(input);
if (data === null) return "N/A";
if (typeof data === "string") {
return (
<div className="bg-gray-25 rounded-lg overflow-auto p-4">
<Markdown>{data}</Markdown>
</div>
);
const getTitle = (alert: AlertConversation) => {
const prompt = alert.conversation;
const title = parsingPromptText(
sanitizeQuestionPrompt({
question: prompt.question_answers?.[0]?.question.message ?? "",
answer: prompt.question_answers?.[0]?.answer?.message ?? "",
}),
prompt.conversation_timestamp,
);

return title;
};

function TypeCellContent({ alert }: { alert: AlertConversation }) {
const conversationType = alert.conversation.type;

switch (conversationType) {
case QuestionType.CHAT:
return "Chat";
case QuestionType.FIM:
return "Code Suggestion";
default:
return "Unknown";
}
if (!data.type || !data.name) return "N/A";
}

return (
<div className="max-h-40 w-fit overflow-y-auto whitespace-pre-wrap p-2">
<label className="font-medium">Package:</label>
&nbsp;
<a
href={`https://www.insight.stacklok.com/report/${data.type}/${data.name}`}
target="_blank"
rel="noopener noreferrer"
className="text-brand-500 hover:underline"
>
{data.type}/{data.name}
</a>
{data.status && (
function IssueDetectedCellContent({ alert }: { alert: AlertConversation }) {
const issueDetected = getIssueDetectedType(alert);

switch (issueDetected) {
case "leaked_secret":
return (
<>
<br />
<label className="font-medium">Status:</label> {data.status}
<KeyRoundIcon className="size-4" />
Blocked secret exposure
</>
)}
{data.description && (
);
case "malicious_package":
return (
<>
<br />
<label className="font-medium">Description:</label> {data.description}
<PackageX className="size-4" />
Blocked malicious package
</>
)}
</div>
);
};
);
default:
return "";
}
}

kantord marked this conversation as resolved.
Show resolved Hide resolved
export function AlertsTable() {
const {
Expand Down Expand Up @@ -164,13 +176,12 @@ export function AlertsTable() {
<Table data-testid="alerts-table" aria-label="Alerts table">
<TableHeader>
<Row>
<Column isRowHeader width={150}>
Trigger Type
<Column isRowHeader className="w-[150px]">
Time
kantord marked this conversation as resolved.
Show resolved Hide resolved
</Column>
<Column width={300}>Trigger Token</Column>
<Column width={150}>File</Column>
<Column width={250}>Code</Column>
<Column width={100}>Timestamp</Column>
<Column className="w-[150px]">Type</Column>
<Column>Event</Column>
<Column width={325}>Issue Detected</Column>
</Row>
</TableHeader>
<TableBody>
Expand All @@ -182,28 +193,18 @@ export function AlertsTable() {
navigate(`/prompt/${alert.conversation.chat_id}`)
}
>
<Cell className="truncate">{alert.trigger_type}</Cell>
<Cell className="overflow-auto whitespace-nowrap max-w-80">
{wrapObjectOutput(alert.trigger_string)}
</Cell>
<Cell className="truncate">
{alert.code_snippet?.filepath || "N/A"}
</Cell>
<Cell className="overflow-auto whitespace-nowrap max-w-80">
{alert.code_snippet?.code ? (
<pre className="max-h-40 overflow-auto bg-gray-100 p-2 whitespace-pre-wrap">
<code>{alert.code_snippet.code}</code>
</pre>
) : (
"N/A"
)}
{formatDistanceToNow(new Date(alert.timestamp), {
addSuffix: true,
})}
</Cell>
<Cell className="truncate">
<div data-testid="date">
{format(new Date(alert.timestamp ?? ""), "y/MM/dd")}
</div>
<div data-testid="time">
{format(new Date(alert.timestamp ?? ""), "hh:mm:ss a")}
<TypeCellContent alert={alert} />
</Cell>
<Cell className="truncate">{getTitle(alert)}</Cell>
<Cell>
<div className="truncate flex gap-2 text-blue-800 items-center">
<IssueDetectedCellContent alert={alert} />
</div>
</Cell>
</Row>
Expand Down
12 changes: 12 additions & 0 deletions src/lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,15 @@ export function getMaliciousPackage(

return null;
}

export function getIssueDetectedType(
alert: AlertConversation,
): "malicious_package" | "leaked_secret" {
const maliciousPackage = getMaliciousPackage(alert.trigger_string);

if (maliciousPackage !== null && typeof maliciousPackage === "object") {
return "malicious_package";
Copy link
Collaborator

Choose a reason for hiding this comment

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

for malicious package we can show the link to insights, it was already in place, I guess it could be very useful to show.

@jtenniswood maybe you didn't have an real example on that:
Screenshot 2025-01-27 at 09 45 05

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm, if the entire table row is clickable, does it make sense to put another link to it?

I think that we should create an alert detail page 🤔 then we could put all these additional information there.

Actually I think that we can convert the current conversation page into an alert detail page? Basically keeping the current content but adding a header or sth similar where we can add details about the alert (I guess this could be hidden for conversations that don't have any alerts?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, if the entire table row is clickable, does it make sense to put another link to it?

I agree, but I also see the value to show the detail about a malicious package, maybe instead of a clickable row we could add a column e.g. go to chat
I guess that James is not aware of this malicious package detail

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, I totally agree, I prefer that over a clickable row any day. It's much clearer user experience in my opinion

Copy link
Member Author

Choose a reason for hiding this comment

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

I will mention this to James

Copy link

Choose a reason for hiding this comment

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

I wonder if that sub content should be on the sub page? I have also heard people wanting to be linked to the place where it happens in the conversation (also for secrets leaking).

}

return "leaked_secret";
}
92 changes: 38 additions & 54 deletions src/routes/__tests__/route-dashboard.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,28 +141,24 @@

expect(
screen.getByRole("columnheader", {
name: /trigger type/i,
name: /type/i,
}),
).toBeVisible();
expect(
screen.getByRole("columnheader", {
name: /trigger token/i,
name: /event/i,
}),
).toBeVisible();

expect(
screen.getByRole("columnheader", {
name: /file/i,
}),
).toBeVisible();
expect(
screen.getByRole("columnheader", {
name: /code/i,
name: /time/i,
}),
).toBeVisible();

expect(
screen.getByRole("columnheader", {
name: /timestamp/i,
name: /issue detected/i,
}),
).toBeVisible();

Expand All @@ -176,18 +172,14 @@
const firstRow = within(screen.getByTestId("alerts-table")).getAllByRole(
"row",
)[1] as HTMLElement;
const secondRow = within(screen.getByTestId("alerts-table")).getAllByRole(
"row",
)[2] as HTMLElement;

expect(within(firstRow).getByText(/ghp_token/i)).toBeVisible();
expect(within(firstRow).getByText(/codegate-secrets/i)).toBeVisible();
expect(within(firstRow).getAllByText(/n\/a/i).length).toEqual(2);
expect(within(firstRow).getByText(/2025\/01\/14/i)).toBeVisible();
expect(within(firstRow).getByTestId(/time/i)).toBeVisible();

// check trigger_string null
expect(within(secondRow).getAllByText(/n\/a/i).length).toEqual(3);
expect(within(firstRow).getByText(/chat/i)).toBeVisible();
expect(within(firstRow).getByText(/[0-9]+.*ago/i)).toBeVisible();
expect(
screen.getAllByRole("gridcell", {
name: /blocked secret exposure/i,
}).length,
).toBeGreaterThanOrEqual(1);
});

it("should render malicious pkg", async () => {
Expand All @@ -208,20 +200,22 @@
),
).toBeVisible();

expect(screen.getByText(/package:/i)).toBeVisible();
expect(
screen.getByRole("link", {
name: /pypi\/invokehttp/i,
screen.getByRole("gridcell", {
name: /blocked malicious package/i,
}),
).toHaveAttribute(
"href",
"https://www.insight.stacklok.com/report/pypi/invokehttp",
);
expect(
screen.getByText(/malicious python http for humans\./i),
).toBeVisible();
});

it("renders event column", async () => {
mockAlertsWithMaliciousPkg();
render(<RouteDashboard />);

await waitFor(() => {
expect(screen.getByText(/are there malicious/i)).toBeVisible();
});
});

it("should filter by malicious pkg", async () => {
mockAlertsWithMaliciousPkg();
render(<RouteDashboard />);
Expand All @@ -232,10 +226,10 @@

expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("2");
expect(
screen.getByRole("row", {
name: /codegate-secrets/i,
}),
).toBeVisible();
screen.getAllByRole("row", {

Check failure on line 229 in src/routes/__tests__/route-dashboard.test.tsx

View workflow job for this annotation

GitHub Actions / Unit Tests / Vitest

src/routes/__tests__/route-dashboard.test.tsx > Dashboard > should filter by malicious pkg 

TestingLibraryElementError: Unable to find an accessible element with the role "row" and name `/chat/i`

Here are the accessible roles:

  region:

  Name "Notifications alt+T":
  <section
    aria-atomic="false"
    aria-label="Notifications alt+T"
    aria-live="polite"
    aria-relevant="additions text"
    tabindex="-1"
  />

  --------------------------------------------------
  banner:

  Name "":
  <header
    class="flex items-center bg-gray-50 px-6 py-4"
  />

  Name "":
  <header
    class="flex items-center bg-gray-50 px-6 py-4 shrink-0"
  />

  Name "":
  <header
    class="flex bg-gray-50 px-6 py-4 items-center"
  />

  Name "":
  <header
    class="flex items-center bg-gray-50 px-6 py-4"
  />

  --------------------------------------------------
  heading:

  Name "CodeGate Status":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl flex justify-between items-center"
    slot="title"
  />

  Name "Security issues detected":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl"
    slot="title"
  />

  Name "Malicious packages by type":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl"
    slot="title"
  />

  Name "Alerts by date":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl"
    slot="title"
  />

  Name "All Alerts":
  <h2
    class="font-bold text-lg"
  />

  --------------------------------------------------
  button:

  Name "":
  <button
    class="outline outline-0 outline-offset-2 outline-brand-700 forced-colors:outline-[Highlight] inline-flex shrink-0 cursor-pointer items-center justify-center gap-1 rounded-sm border px-4 py-2 text-center text-base font-medium [&_svg]:size-4 [&_svg]:shrink-0 border-transparent bg-transparent text-primary hover:bg-gray-100 pressed:bg-gray-200 disabled:bg-transparent disabled:text-disabled focus-within:&:has([data-focus-visible]):outline-2 invalid:!outline-red-700 focus-visible:outline-2 size-7 ml-auto -mr-2"
    data-rac=""
    id="react-aria-:r3j:"
    type="button"
  />

  Name "5 minutes Check for updates":
  <button
    aria-expanded="false"
    aria-haspopup="listbox"
    aria-labelledby="react-aria-:r3u: react-aria-:r3q:"
    class="rounded-sm text-base outline outline-0 outline-offset-2 outline-brand-500 placeholder:text-placeholder disabled:text-disabled forced-colors:outline-[Highlight] flex w-full cursor-pointer items-center gap-4 py-2 pl-3 text-start group-data-[invalid]:!border-red-400 group-data-[invalid]:!text-invalid group-data-[invalid]:forced-colors:text-[Mark] bg-transparent h-7 max-w-36 pr-0 [&>span>span]:text-right [&>span>span]:justify-end !gap-0 text-secondary"
    data-rac=""
    id="react-aria-:r3p:"
    type="button"
  />

  Name "Clear search":
  <button
    aria-label="Clear search"
    class="outline outline-0 outline-offset-2 outline-brand-700 forced-colors:outline-[Highlight] inline-flex shrink-0 cursor-pointer items-center justify-center gap-1 rounded-sm border py-2 text-center text-base font-medium aspect-square px-2 [&_svg]:mx-auto border-transparent bg-transparent hover:bg-gray-100 pressed:bg-gray-200 disabled:bg-transparent disabled:text-disabled focus-within:&:has([data-focus-visible]):outline-2 invalid:!outline-red-700 focus-visible:outline-2 size-7 text-secondary last-of-type:mr-1 [&:not(:last-of-type)]:mr-px [&_svg]:size-4 [&_svg]:shrink-0 group-empty:invisible"
    data-rac=""
    id="react-aria-:r48:"
    slot="clear"
    tabindex="-1"
    type="button"
  />

  Name "Previous":
  <button
    class="outline outline-0 outline-offset-2 outline-brand-700 forced-colors:outline-[Highlight] inline-flex h-10 shrink-0 cursor-pointer items-center justify-center gap-1 rounded-sm border px-4 py-2 text-center text-base font-medium [&_svg]:size-4 [&_svg]:shrink-0 border-brand-800 bg-brand-800 text-white shadow-sm hover:border-brand-700 hover:bg-brand-700 pressed:border-brand-900 pressed:bg-brand-900 disabled:border-gray-200 disabled:bg-base disabled:text-disabled focus-within:&:has([data-focus-visible]):outline-2 invalid:!outline-
name: /chat/i,
}).length,
).toBeGreaterThanOrEqual(1);

userEvent.click(
screen.getByRole("switch", {
Expand All @@ -247,15 +241,11 @@
expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("1"),
);

expect(screen.getByText(/package:/i)).toBeVisible();
expect(
screen.getByRole("link", {
name: /pypi\/invokehttp/i,
}),
).toBeVisible();
expect(
screen.getByText(/malicious python http for humans\./i),
).toBeVisible();
screen.queryAllByRole("gridcell", {
name: /blocked secret exposure/i,
}).length,
).toBe(0);

userEvent.click(
screen.getByRole("switch", {
Expand All @@ -277,15 +267,10 @@

expect(screen.getByTestId(/alerts-count/i)).toHaveTextContent("2");
expect(
screen.getByRole("row", {
name: /codegate-secrets/i,
}),
).toBeVisible();
expect(
screen.getByRole("row", {
name: /codegate-context-retriever/i,
}),
).toBeVisible();
screen.getAllByRole("row", {

Check failure on line 270 in src/routes/__tests__/route-dashboard.test.tsx

View workflow job for this annotation

GitHub Actions / Unit Tests / Vitest

src/routes/__tests__/route-dashboard.test.tsx > Dashboard > should search by secrets alert 

TestingLibraryElementError: Unable to find an accessible element with the role "row" and name `/chat/i`

Here are the accessible roles:

  region:

  Name "Notifications alt+T":
  <section
    aria-atomic="false"
    aria-label="Notifications alt+T"
    aria-live="polite"
    aria-relevant="additions text"
    tabindex="-1"
  />

  --------------------------------------------------
  banner:

  Name "":
  <header
    class="flex items-center bg-gray-50 px-6 py-4"
  />

  Name "":
  <header
    class="flex items-center bg-gray-50 px-6 py-4 shrink-0"
  />

  Name "":
  <header
    class="flex bg-gray-50 px-6 py-4 items-center"
  />

  Name "":
  <header
    class="flex items-center bg-gray-50 px-6 py-4"
  />

  --------------------------------------------------
  heading:

  Name "CodeGate Status":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl flex justify-between items-center"
    slot="title"
  />

  Name "Security issues detected":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl"
    slot="title"
  />

  Name "Malicious packages by type":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl"
    slot="title"
  />

  Name "Alerts by date":
  <h3
    class="font-title font-semibold text-primary mb-0 truncate text-xl"
    slot="title"
  />

  Name "All Alerts":
  <h2
    class="font-bold text-lg"
  />

  --------------------------------------------------
  button:

  Name "":
  <button
    class="outline outline-0 outline-offset-2 outline-brand-700 forced-colors:outline-[Highlight] inline-flex shrink-0 cursor-pointer items-center justify-center gap-1 rounded-sm border px-4 py-2 text-center text-base font-medium [&_svg]:size-4 [&_svg]:shrink-0 border-transparent bg-transparent text-primary hover:bg-gray-100 pressed:bg-gray-200 disabled:bg-transparent disabled:text-disabled focus-within:&:has([data-focus-visible]):outline-2 invalid:!outline-red-700 focus-visible:outline-2 size-7 ml-auto -mr-2"
    data-rac=""
    id="react-aria-:r4m:"
    type="button"
  />

  Name "5 minutes Check for updates":
  <button
    aria-expanded="false"
    aria-haspopup="listbox"
    aria-labelledby="react-aria-:r51: react-aria-:r4t:"
    class="rounded-sm text-base outline outline-0 outline-offset-2 outline-brand-500 placeholder:text-placeholder disabled:text-disabled forced-colors:outline-[Highlight] flex w-full cursor-pointer items-center gap-4 py-2 pl-3 text-start group-data-[invalid]:!border-red-400 group-data-[invalid]:!text-invalid group-data-[invalid]:forced-colors:text-[Mark] bg-transparent h-7 max-w-36 pr-0 [&>span>span]:text-right [&>span>span]:justify-end !gap-0 text-secondary"
    data-rac=""
    id="react-aria-:r4s:"
    type="button"
  />

  Name "Clear search":
  <button
    aria-label="Clear search"
    class="outline outline-0 outline-offset-2 outline-brand-700 forced-colors:outline-[Highlight] inline-flex shrink-0 cursor-pointer items-center justify-center gap-1 rounded-sm border py-2 text-center text-base font-medium aspect-square px-2 [&_svg]:mx-auto border-transparent bg-transparent hover:bg-gray-100 pressed:bg-gray-200 disabled:bg-transparent disabled:text-disabled focus-within:&:has([data-focus-visible]):outline-2 invalid:!outline-red-700 focus-visible:outline-2 size-7 text-secondary last-of-type:mr-1 [&:not(:last-of-type)]:mr-px [&_svg]:size-4 [&_svg]:shrink-0 group-empty:invisible"
    data-rac=""
    id="react-aria-:r5b:"
    slot="clear"
    tabindex="-1"
    type="button"
  />

  Name "Previous":
  <button
    class="outline outline-0 outline-offset-2 outline-brand-700 forced-colors:outline-[Highlight] inline-flex h-10 shrink-0 cursor-pointer items-center justify-center gap-1 rounded-sm border px-4 py-2 text-center text-base font-medium [&_svg]:size-4 [&_svg]:shrink-0 border-brand-800 bg-brand-800 text-white shadow-sm hover:border-brand-700 hover:bg-brand-700 pressed:border-brand-900 pressed:bg-brand-900 disabled:border-gray-200 disabled:bg-base disabled:text-disabled focus-within:&:has([data-focus-visible]):outline-2 invalid:!outline-
name: /chat/i,
}).length,
).toBeGreaterThanOrEqual(1);

await userEvent.type(screen.getByRole("searchbox"), "codegate-secrets");

Expand All @@ -295,8 +280,7 @@
const row = within(screen.getByTestId("alerts-table")).getAllByRole(
"row",
)[1] as HTMLElement;
expect(within(row).getByText(/ghp_token/i)).toBeVisible();
expect(within(row).getByText(/codegate-secrets/i)).toBeVisible();
expect(within(row).getByText(/chat/i)).toBeVisible();
});

it("should sort alerts by date desc", async () => {
Expand All @@ -312,8 +296,8 @@
"row",
)[2] as HTMLElement;

expect(within(firstRow).getByText(/2025\/01\/14/i)).toBeVisible();
expect(within(secondRow).getByText(/2025\/01\/07/i)).toBeVisible();
expect(within(firstRow).getByText(/[0-9]+.*ago/i)).toBeVisible();
expect(within(secondRow).getByText(/[0-9]+.*ago/i)).toBeVisible();
});

it("only displays a limited number of items in the table", async () => {
Expand Down
Loading