Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions static/app/views/detectors/list/allMonitors.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
import WorkflowEngineListLayout from 'sentry/components/workflowEngine/layout/list';
import {t} from 'sentry/locale';
import {AlertsRedirectNotice} from 'sentry/views/detectors/list/common/alertsRedirectNotice';
import {DetectorListActions} from 'sentry/views/detectors/list/common/detectorListActions';
import {DetectorListContent} from 'sentry/views/detectors/list/common/detectorListContent';
import {DetectorListHeader} from 'sentry/views/detectors/list/common/detectorListHeader';
Expand All @@ -23,6 +24,9 @@ export default function AllMonitors() {
description={DESCRIPTION}
docsUrl={DOCS_URL}
>
<AlertsRedirectNotice>
{t('Alert Rules have been moved to Monitors and Alerts.')}
</AlertsRedirectNotice>
<DetectorListHeader />
<DetectorListContent {...detectorListQuery} />
</WorkflowEngineListLayout>
Expand Down
37 changes: 37 additions & 0 deletions static/app/views/detectors/list/common/alertsRedirectNotice.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {parseAsBoolean, useQueryState} from 'nuqs';

import {Alert} from '@sentry/scraps/alert';
import {Button} from '@sentry/scraps/button';

import {IconClose} from 'sentry/icons';
import {t} from 'sentry/locale';

export function AlertsRedirectNotice({children}: {children: React.ReactNode}) {
const [wasRedirectedFromAlerts, setWasRedirectedFromAlerts] = useQueryState(
'alertsRedirect',
parseAsBoolean.withOptions({history: 'replace'}).withDefault(false)
);

if (!wasRedirectedFromAlerts) {
return null;
}

return (
<Alert
type="info"
trailingItems={
<Button
size="zero"
borderless
icon={<IconClose />}
aria-label={t('Dismiss')}
onClick={() => {
setWasRedirectedFromAlerts(false);
}}
/>
}
>
{children}
</Alert>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import styled from '@emotion/styled';

import {t} from 'sentry/locale';
import useOrganization from 'sentry/utils/useOrganization';
import {useUser} from 'sentry/utils/useUser';
import {makeMonitorBasePathname} from 'sentry/views/detectors/pathnames';
import {ISSUE_TAXONOMY_CONFIG} from 'sentry/views/issueList/taxonomies';
import {useNavContext} from 'sentry/views/nav/context';
import {PRIMARY_NAV_GROUP_CONFIG} from 'sentry/views/nav/primary/config';
Expand Down Expand Up @@ -81,9 +83,18 @@ export function IssuesSecondaryNav() {
}

function ConfigureSection({baseUrl}: {baseUrl: string}) {
const user = useUser();
const organization = useOrganization();
const {layout} = useNavContext();
const isSticky = layout === NavLayout.SIDEBAR;

const shouldRedirectToWorkflowEngineUI =
!user.isStaff && organization.features.includes('workflow-engine-ui');
Comment on lines +91 to +92
Copy link

Choose a reason for hiding this comment

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

Bug: useUser() can return null/undefined, but user.isStaff is accessed without a null check, causing a TypeError.
Severity: CRITICAL | Confidence: 0.95

🔍 Detailed Analysis

The useUser() hook can return null or undefined despite its Readonly<User> type annotation, as evidenced by explicit type casting and defensive checks in other parts of the codebase. The issuesSecondaryNav.tsx component directly accesses user.isStaff without a null/undefined check, leading to a TypeError: Cannot read property 'isStaff' of null/undefined if user is not yet initialized or becomes null/undefined during early app lifecycle or race conditions.

💡 Suggested Fix

Add a null/undefined check for the user object before accessing user.isStaff. For example, if (user && !user.isStaff && organization.features.includes('workflow-engine-ui')).

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/nav/secondary/sections/issues/issuesSecondaryNav.tsx#L91-L92

Potential issue: The `useUser()` hook can return `null` or `undefined` despite its
`Readonly<User>` type annotation, as evidenced by explicit type casting and defensive
checks in other parts of the codebase. The `issuesSecondaryNav.tsx` component directly
accesses `user.isStaff` without a null/undefined check, leading to a `TypeError: Cannot
read property 'isStaff' of null/undefined` if `user` is not yet initialized or becomes
`null`/`undefined` during early app lifecycle or race conditions.

Did we get this right? 👍 / 👎 to inform future reviews.

Reference_id: 2663962


const alertsLink = shouldRedirectToWorkflowEngineUI
? `${makeMonitorBasePathname(organization.slug)}?alertsRedirect=true`
: `${baseUrl}/alerts/rules/`;

return (
<StickyBottomSection
id="issues-configure"
Expand All @@ -92,8 +103,8 @@ function ConfigureSection({baseUrl}: {baseUrl: string}) {
isSticky={isSticky}
>
<SecondaryNav.Item
to={`${baseUrl}/alerts/rules/`}
activeTo={`${baseUrl}/alerts/`}
to={alertsLink}
{...(!shouldRedirectToWorkflowEngineUI && {activeTo: `${baseUrl}/alerts/`})}
analyticsItemName="issues_alerts"
>
{t('Alerts')}
Expand Down
Loading