Skip to content

Commit 11e4a63

Browse files
authored
fix(aci): prevent deletion of system-created monitors in UI (#103838)
system-created monitors should not be deleted by anyone, regardless of permissions closes https://linear.app/getsentry/issue/NEW-647/disable-bulk-delete-in-ui-when-selection-includes-error-monitors
1 parent 643d334 commit 11e4a63

File tree

3 files changed

+36
-3
lines changed

3 files changed

+36
-3
lines changed

static/app/views/detectors/components/detectorListTable/actions.tsx

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ interface DetectorsTableActionsProps {
1919
allResultsVisible: boolean;
2020
canEdit: boolean;
2121
detectorLimitReached: boolean;
22+
hasSystemCreatedDetectors: boolean;
2223
pageSelected: boolean;
2324
queryCount: string;
2425
selected: Set<string>;
@@ -36,11 +37,14 @@ export function DetectorsTableActions({
3637
showEnable,
3738
showDisable,
3839
canEdit,
40+
hasSystemCreatedDetectors,
3941
detectorLimitReached,
4042
}: DetectorsTableActionsProps) {
4143
const [allInQuerySelected, setAllInQuerySelected] = useState(false);
4244
const anySelected = selected.size > 0;
4345

46+
const canDelete = canEdit && !hasSystemCreatedDetectors;
47+
4448
const {selection} = usePageFilters();
4549
const {query} = useLocationQuery({
4650
fields: {
@@ -199,14 +203,18 @@ export function DetectorsTableActions({
199203
</Tooltip>
200204
)}
201205
<Tooltip
202-
title={t('You do not have permission to delete the selected monitors.')}
203-
disabled={canEdit}
206+
title={
207+
hasSystemCreatedDetectors
208+
? t('Monitors managed by Sentry cannot be deleted.')
209+
: t('You do not have permission to delete the selected monitors.')
210+
}
211+
disabled={canDelete}
204212
>
205213
<Button
206214
size="xs"
207215
priority="danger"
208216
onClick={handleDelete}
209-
disabled={isDeleting || !canEdit}
217+
disabled={isDeleting || !canDelete}
210218
>
211219
{t('Delete')}
212220
</Button>

static/app/views/detectors/components/detectorListTable/index.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
useMonitorViewContext,
3737
type MonitorListAdditionalColumn,
3838
} from 'sentry/views/detectors/monitorViewContext';
39+
import {detectorTypeIsUserCreateable} from 'sentry/views/detectors/utils/detectorTypeConfig';
3940
import {useCanEditDetectors} from 'sentry/views/detectors/utils/useCanEditDetector';
4041
import {CronServiceIncidents} from 'sentry/views/insights/crons/components/serviceIncidents';
4142

@@ -139,6 +140,9 @@ function DetectorListTable({
139140

140141
const selectedDetectors = detectors.filter(d => selected.has(d.id));
141142
const canEditDetectors = useCanEditDetectors({detectors: selectedDetectors});
143+
const hasSystemCreatedDetectors = selectedDetectors.some(
144+
d => !detectorTypeIsUserCreateable(d.type)
145+
);
142146

143147
const elementRef = useRef<HTMLDivElement>(null);
144148
const {width: containerWidth} = useDimensions<HTMLDivElement>({elementRef});
@@ -212,6 +216,7 @@ function DetectorListTable({
212216
showDisable={canDisable}
213217
showEnable={canEnable}
214218
canEdit={canEditDetectors}
219+
hasSystemCreatedDetectors={hasSystemCreatedDetectors}
215220
// TODO: Check if metric detector limit is reached
216221
detectorLimitReached={false}
217222
/>

static/app/views/detectors/list/allMonitors.spec.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,26 @@ describe('DetectorsList', () => {
461461
});
462462
});
463463

464+
it('can not delete system-created detectors', async () => {
465+
MockApiClient.addMockResponse({
466+
url: '/organizations/org-slug/detectors/',
467+
body: [
468+
ErrorDetectorFixture({
469+
name: 'System Created Detector',
470+
}),
471+
],
472+
});
473+
render(<AllMonitors />, {organization});
474+
await screen.findByText('System Created Detector');
475+
476+
const rows = screen.getAllByTestId('detector-list-row');
477+
const firstRowCheckbox = within(rows[0]!).getByRole('checkbox');
478+
await userEvent.click(firstRowCheckbox);
479+
480+
// Verify that delete button is disabled
481+
expect(screen.getByRole('button', {name: 'Delete'})).toBeDisabled();
482+
});
483+
464484
it('shows option to select all query results when page is selected', async () => {
465485
const deleteRequest = MockApiClient.addMockResponse({
466486
url: '/organizations/org-slug/detectors/',

0 commit comments

Comments
 (0)