Skip to content

Commit da6bdb4

Browse files
authored
fix(dashboards) Add a confirm step to deletion (#23098)
Add a confirmation to dashboard deletion. I needed a ?. in getSeriesSelection() to work around undefined sneaking in during tests.
1 parent 3e70867 commit da6bdb4

File tree

3 files changed

+37
-30
lines changed

3 files changed

+37
-30
lines changed

src/sentry/static/sentry/app/components/charts/utils.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export function getSeriesSelection(
128128
location: Location,
129129
parameter = 'unselectedSeries'
130130
): EChartOption.Legend['selected'] {
131-
const unselectedSeries = decodeList(location.query[parameter]) ?? [];
131+
const unselectedSeries = decodeList(location?.query[parameter]) ?? [];
132132
return unselectedSeries.reduce((selection, series) => {
133133
selection[series] = false;
134134
return selection;

src/sentry/static/sentry/app/views/dashboardsV2/controls.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import styled from '@emotion/styled';
44

55
import Button from 'app/components/button';
66
import ButtonBar from 'app/components/buttonBar';
7+
import Confirm from 'app/components/confirm';
78
import SelectControl from 'app/components/forms/selectControl';
89
import {IconAdd, IconEdit} from 'app/icons';
910
import {t} from 'app/locale';
@@ -58,16 +59,15 @@ class Controls extends React.Component<Props> {
5859
return (
5960
<ButtonBar gap={1} key="edit-controls">
6061
{cancelButton}
61-
<Button
62-
data-test-id="dashboard-delete"
63-
onClick={e => {
64-
e.preventDefault();
65-
onDelete();
66-
}}
62+
<Confirm
6763
priority="danger"
64+
message={t('Are you sure you want to delete this dashboard?')}
65+
onConfirm={onDelete}
6866
>
69-
{t('Delete')}
70-
</Button>
67+
<Button data-test-id="dashboard-delete" priority="danger">
68+
{t('Delete')}
69+
</Button>
70+
</Confirm>
7171
<Button
7272
data-test-id="dashboard-commit"
7373
onClick={e => {

tests/js/spec/views/dashboardsV2/detail.spec.jsx

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,10 @@ import {browserHistory} from 'react-router';
33

44
import {mountWithTheme} from 'sentry-test/enzyme';
55
import {initializeOrg} from 'sentry-test/initializeOrg';
6+
import {mountGlobalModal} from 'sentry-test/modal';
67

7-
import {openAddDashboardWidgetModal} from 'app/actionCreators/modal';
88
import DashboardDetail from 'app/views/dashboardsV2/detail';
99

10-
jest.mock('app/actionCreators/modal', () => ({
11-
openAddDashboardWidgetModal: jest.fn(),
12-
}));
13-
1410
describe('Dashboards > Detail', function () {
1511
const organization = TestStubs.Organization({
1612
features: ['global-views', 'dashboards-v2', 'discover-query'],
@@ -70,8 +66,17 @@ describe('Dashboards > Detail', function () {
7066
// Enter edit mode.
7167
wrapper.find('Controls Button[data-test-id="dashboard-edit"]').simulate('click');
7268

73-
// Click delete, request should be made.
69+
const modal = await mountGlobalModal();
70+
71+
// Click delete, confirm will show
7472
wrapper.find('Controls Button[data-test-id="dashboard-delete"]').simulate('click');
73+
await tick();
74+
75+
await modal.update();
76+
77+
// Click confirm
78+
modal.find('button[aria-label="Confirm"]').simulate('click');
79+
7580
expect(deleteMock).toHaveBeenCalled();
7681
});
7782

@@ -128,14 +133,20 @@ describe('Dashboards > Detail', function () {
128133
beforeEach(function () {
129134
initialData = initializeOrg({organization});
130135
widgets = [
131-
TestStubs.Widget([{conditions: 'event.type:error', fields: ['count()']}], {
132-
title: 'Errors',
133-
interval: '1d',
134-
}),
135-
TestStubs.Widget([{conditions: 'event.type:transaction', fields: ['count()']}], {
136-
title: 'Transactions',
137-
interval: '1d',
138-
}),
136+
TestStubs.Widget(
137+
[{name: '', conditions: 'event.type:error', fields: ['count()']}],
138+
{
139+
title: 'Errors',
140+
interval: '1d',
141+
}
142+
),
143+
TestStubs.Widget(
144+
[{name: '', conditions: 'event.type:transaction', fields: ['count()']}],
145+
{
146+
title: 'Transactions',
147+
interval: '1d',
148+
}
149+
),
139150
];
140151

141152
MockApiClient.addMockResponse({
@@ -241,14 +252,10 @@ describe('Dashboards > Detail', function () {
241252
.simulate('click');
242253

243254
await tick();
244-
wrapper.update();
255+
await wrapper.update();
256+
const modal = await mountGlobalModal();
245257

246-
expect(openAddDashboardWidgetModal).toHaveBeenCalled();
247-
expect(openAddDashboardWidgetModal).toHaveBeenCalledWith(
248-
expect.objectContaining({
249-
widget: widgets[0],
250-
})
251-
);
258+
expect(modal.find('AddDashboardWidgetModal').props().widget).toEqual(widgets[0]);
252259
});
253260
});
254261
});

0 commit comments

Comments
 (0)