-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
refactor(alerts): consolidate queries #233
Conversation
alex-mcgovern
commented
Jan 30, 2025
- A follow on from fix(alerts): memoize selecting alerts #231 and test: alerts summary cards #226
- Continues refactoring data fetching for the alerts table, and queryParam state management to decouple these, and continue building a composable, modular system for data fetching
- Closes [Bug]: Duplicate requests to alerts #230
export function SwitchMaliciousAlertsFilter() { | ||
const { setView, state } = useAlertsFilterSearchParams(); | ||
|
||
return ( | ||
<TooltipTrigger> | ||
<Switch | ||
id="malicious-packages" | ||
isSelected={state.view === AlertsFilterView.MALICIOUS} | ||
onChange={(isSelected) => { | ||
switch (isSelected) { | ||
case true: | ||
return setView(AlertsFilterView.MALICIOUS); | ||
case false: | ||
return setView(AlertsFilterView.ALL); | ||
default: | ||
return isSelected satisfies never; | ||
} | ||
}} | ||
> | ||
Malicious Packages | ||
</Switch> | ||
|
||
<Tooltip> | ||
<p>Filter by malicious packages</p> | ||
</Tooltip> | ||
</TooltipTrigger> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spoke with James, and we will soon remove this and replace with tabs for "all" | "malicious" | "secrets" | "PII"
etc
export function doesAlertIncludeSearch( | ||
alert: V1GetWorkspaceAlertsResponse[number], | ||
searchQuery: string | null, | ||
): alert is AlertConversation { | ||
if (alert == null) return false; | ||
if (searchQuery === null) return true; | ||
|
||
const trigger_type: string = alert.trigger_type; | ||
const trigger_string: string | null = | ||
typeof alert.trigger_string === "string" ? alert.trigger_string : null; | ||
|
||
let malicious_pkg_name: string | null = null; | ||
let malicious_pkg_type: string | null = null; | ||
|
||
if ( | ||
alert.trigger_string !== null && | ||
typeof alert.trigger_string === "object" && | ||
"name" in alert.trigger_string && | ||
typeof alert.trigger_string.name === "string" && | ||
"type" in alert.trigger_string && | ||
typeof alert.trigger_string.type === "string" | ||
) { | ||
malicious_pkg_name = alert.trigger_string.name; | ||
malicious_pkg_type = alert.trigger_string.type; | ||
} | ||
|
||
return [ | ||
trigger_type, | ||
trigger_string, | ||
malicious_pkg_name, | ||
malicious_pkg_type, | ||
].some((i) => i?.toLowerCase().includes(searchQuery)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be simplified with zod 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup this part is a bit tricky, but also for BE too...let's try to figured out it later, maybe we can try to do something in BE too
alert?.trigger_category === "critical" && | ||
alert.trigger_string !== null && | ||
typeof alert.trigger_string === "object" && | ||
"status" in alert.trigger_string && | ||
alert.trigger_string.status === "malicious" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be simplified with zod
Pull Request Test Coverage Report for Build 13051157214Details
💛 - Coveralls |