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 all 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
176 changes: 90 additions & 86 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 @@ -12,58 +12,71 @@ import {
SearchFieldClearButton,
Badge,
Button,
ResizableTableContainer,
} 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 text-blue-700" />
Blocked secret exposure
</>
)}
{data.description && (
);
case "malicious_package":
return (
<>
<br />
<label className="font-medium">Description:</label> {data.description}
<PackageX className="size-4 text-blue-700" />
Blocked malicious package
</>
)}
</div>
);
};
);
default:
return "";
}
}

export function AlertsTable() {
const {
Expand Down Expand Up @@ -161,55 +174,46 @@ export function AlertsTable() {
</div>
</div>
<div className="overflow-x-auto">
<Table data-testid="alerts-table" aria-label="Alerts table">
<TableHeader>
<Row>
<Column isRowHeader width={150}>
Trigger Type
</Column>
<Column width={300}>Trigger Token</Column>
<Column width={150}>File</Column>
<Column width={250}>Code</Column>
<Column width={100}>Timestamp</Column>
</Row>
</TableHeader>
<TableBody>
{dataView.map((alert) => (
<Row
key={alert.alert_id}
className="h-20"
onAction={() =>
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"
)}
</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")}
</div>
</Cell>
<ResizableTableContainer>
<Table data-testid="alerts-table" aria-label="Alerts table">
<TableHeader>
<Row>
<Column isRowHeader width={150}>
Time
</Column>
<Column width={150}>Type</Column>
<Column>Event</Column>
<Column width={325}>Issue Detected</Column>
</Row>
))}
</TableBody>
</Table>
</TableHeader>
<TableBody>
{dataView.map((alert) => (
<Row
key={alert.alert_id}
className="h-20"
onAction={() =>
navigate(`/prompt/${alert.conversation.chat_id}`)
}
>
<Cell className="truncate">
{formatDistanceToNow(new Date(alert.timestamp), {
addSuffix: true,
})}
</Cell>
<Cell className="truncate">
<TypeCellContent alert={alert} />
</Cell>
<Cell className="truncate">{getTitle(alert)}</Cell>
<Cell>
<div className="truncate flex gap-2 items-center">
<IssueDetectedCellContent alert={alert} />
</div>
</Cell>
</Row>
))}
</TableBody>
</Table>
</ResizableTableContainer>
</div>

<div className="flex justify-center w-full p-4">
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 @@ describe("Dashboard", () => {

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 @@ describe("Dashboard", () => {
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 @@ describe("Dashboard", () => {
),
).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 @@ describe("Dashboard", () => {

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

userEvent.click(
screen.getByRole("switch", {
Expand All @@ -247,15 +241,11 @@ describe("Dashboard", () => {
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 @@ describe("Dashboard", () => {

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("gridcell", {
name: /chat/i,
}).length,
).toBeGreaterThanOrEqual(1);

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

Expand All @@ -295,8 +280,7 @@ describe("Dashboard", () => {
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 @@ describe("Dashboard", () => {
"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