Skip to content

feat(aci): redirect alerts nav to monitors #127

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(aci): redirect alerts nav to monitors #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
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
View file
Open in desktop
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>
);
}
15 changes: 13 additions & 2 deletions static/app/views/nav/secondary/sections/issues/issuesSecondaryNav.tsx
View file
Open in desktop
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');

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}
activeTo={alertsLink}
Comment on lines 105 to +107
Copy link

@propel-test-bot propel-test-bot bot Nov 13, 2025

Choose a reason for hiding this comment

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

[BestPractice]

The activeTo prop is set to the same value as to, which may cause navigation issues. When redirecting to the workflow engine UI, users will be taken to a different base path (/organizations/${org}/monitors), but activeTo will still reference that path. This means the nav item will show as active even when users navigate to sub-routes under /issues/alerts/ (the original path). Consider using a more specific pattern:

Suggested Change
Suggested change
<SecondaryNav.Item
to={`${baseUrl}/alerts/rules/`}
activeTo={`${baseUrl}/alerts/`}
to={alertsLink}
activeTo={alertsLink}
<SecondaryNav.Item
to={alertsLink}
activeTo={shouldRedirectToWorkflowEngineUI ? alertsLink : `${baseUrl}/alerts/`}
analyticsItemName="issues_alerts"
>

This ensures the active state matches the actual navigation behavior for both redirect and non-redirect scenarios.

⚡ Committable suggestion

Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Context for Agents 
[**BestPractice**]

The `activeTo` prop is set to the same value as `to`, which may cause navigation issues. When redirecting to the workflow engine UI, users will be taken to a different base path (`/organizations/${org}/monitors`), but `activeTo` will still reference that path. This means the nav item will show as active even when users navigate to sub-routes under `/issues/alerts/` (the original path). Consider using a more specific pattern:

<details>
<summary>Suggested Change</summary>

```suggestion
      <SecondaryNav.Item
        to={alertsLink}
        activeTo={shouldRedirectToWorkflowEngineUI ? alertsLink : `${baseUrl}/alerts/`}
        analyticsItemName="issues_alerts"
      >
```

This ensures the active state matches the actual navigation behavior for both redirect and non-redirect scenarios.

⚡ **Committable suggestion**

Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

</details>

File: static/app/views/nav/secondary/sections/issues/issuesSecondaryNav.tsx
Line: 107

analyticsItemName="issues_alerts"
>
{t('Alerts')}
Expand Down