-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
fix(aci): Include project alerts on monitor list page #111690
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| import {AutomationFixture} from 'sentry-fixture/automations'; | ||
| import {IssueStreamDetectorFixture} from 'sentry-fixture/detectors'; | ||
|
|
||
| import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; | ||
|
|
||
| import {DetectorListConnectedAutomations} from 'sentry/views/detectors/components/detectorListConnectedAutomations'; | ||
|
|
||
| describe('DetectorListConnectedAutomations', () => { | ||
| beforeEach(() => { | ||
| MockApiClient.clearMockResponses(); | ||
| }); | ||
|
|
||
| it('renders combined count of direct and project alerts', async () => { | ||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/workflows/', | ||
| body: [ | ||
| AutomationFixture({id: '1', name: 'Direct Alert'}), | ||
| AutomationFixture({id: '2', name: 'Project Alert'}), | ||
| ], | ||
| }); | ||
|
|
||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/detectors/', | ||
| body: [IssueStreamDetectorFixture({workflowIds: ['2']})], | ||
| }); | ||
|
|
||
| render(<DetectorListConnectedAutomations automationIds={['1']} projectId="1" />); | ||
|
|
||
| expect(await screen.findByText('2 alerts')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('deduplicates automation ids from both sources', async () => { | ||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/workflows/', | ||
| body: [AutomationFixture({id: '1', name: 'Shared Alert'})], | ||
| }); | ||
|
|
||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/detectors/', | ||
| body: [IssueStreamDetectorFixture({workflowIds: ['1']})], | ||
| }); | ||
|
|
||
| render(<DetectorListConnectedAutomations automationIds={['1']} projectId="1" />); | ||
|
|
||
| expect(await screen.findByText('1 alert')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders empty cell when no automations are connected', async () => { | ||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/workflows/', | ||
| body: [], | ||
| }); | ||
|
|
||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/detectors/', | ||
| body: [IssueStreamDetectorFixture({workflowIds: []})], | ||
| }); | ||
|
|
||
| render(<DetectorListConnectedAutomations automationIds={[]} projectId="1" />); | ||
|
|
||
| // EmptyCell renders an em dash | ||
| expect(await screen.findByText('—')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows automation names in hovercard on hover', async () => { | ||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/workflows/', | ||
| body: [ | ||
| AutomationFixture({id: '1', name: 'Direct Alert'}), | ||
| AutomationFixture({id: '2', name: 'Project Alert'}), | ||
| ], | ||
| }); | ||
|
|
||
| MockApiClient.addMockResponse({ | ||
| url: '/organizations/org-slug/detectors/', | ||
| body: [IssueStreamDetectorFixture({workflowIds: ['2']})], | ||
| }); | ||
|
|
||
| render(<DetectorListConnectedAutomations automationIds={['1']} projectId="1" />); | ||
|
|
||
| await userEvent.hover(await screen.findByText('2 alerts')); | ||
|
|
||
| const directAlert = await screen.findByText('Direct Alert'); | ||
| const projectAlert = screen.getByText('Project Alert'); | ||
|
|
||
| expect(directAlert.closest('a')).toHaveAttribute( | ||
| 'href', | ||
| '/organizations/org-slug/monitors/alerts/1/' | ||
| ); | ||
| expect(projectAlert.closest('a')).toHaveAttribute( | ||
| 'href', | ||
| '/organizations/org-slug/monitors/alerts/2/' | ||
| ); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,7 +52,10 @@ export function DetectorListRow({detector, selected, onSelect}: DetectorListRowP | |
| <DetectorAssigneeCell assignee={detector.owner} /> | ||
| </SimpleTable.RowCell> | ||
| <SimpleTable.RowCell data-column-name="connected-automations"> | ||
| <DetectorListConnectedAutomations automationIds={detector.workflowIds} /> | ||
| <DetectorListConnectedAutomations | ||
| automationIds={detector.workflowIds} | ||
| projectId={detector.projectId} | ||
|
Comment on lines
+56
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: should we just pass the detector as the prop so we can strictly type it and have access to any other data we might need? |
||
| /> | ||
| </SimpleTable.RowCell> | ||
| {additionalColumns.map(col => ( | ||
| <Fragment key={col.id}>{col.renderCell(detector)}</Fragment> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,9 +5,14 @@ import {useDetectorsQuery} from 'sentry/views/detectors/hooks'; | |
| * Issue stream detectors are used to connect automations to "all issues in a project". | ||
| */ | ||
| export function useIssueStreamDetectorsForProject(projectId: string | undefined) { | ||
| return useDetectorsQuery({ | ||
| query: 'type:issue_stream', | ||
| projects: [Number(projectId)], | ||
| includeIssueStreamDetectors: true, | ||
| }); | ||
| return useDetectorsQuery( | ||
| { | ||
| query: 'type:issue_stream', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just curious.. how hard would it be to rename the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| projects: [Number(projectId)], | ||
| includeIssueStreamDetectors: true, | ||
| }, | ||
| { | ||
| staleTime: Infinity, | ||
| } | ||
| ); | ||
| } | ||

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.
Nit: could we use an Indexed Access Type for these?
Project["id"]kind of thing, so we can rely on a single type defintion?