From a158db6d4c1f89885f1070d3473552218c933662 Mon Sep 17 00:00:00 2001
From: Mark Story <mark@mark-story.com>
Date: Thu, 14 Jan 2021 14:26:26 -0500
Subject: [PATCH] fix(dashboards) Add a confirm step to deletion

Add a confirmation to dashboard deletion. I needed a ?. in
getSeriesSelection() to work around undefined sneaking in during tests.
---
 .../sentry/app/components/charts/utils.tsx    |  2 +-
 .../app/views/dashboardsV2/controls.tsx       | 16 +++---
 .../spec/views/dashboardsV2/detail.spec.jsx   | 49 +++++++++++--------
 3 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/src/sentry/static/sentry/app/components/charts/utils.tsx b/src/sentry/static/sentry/app/components/charts/utils.tsx
index 91a42ac93cfd89..07125f87b87e85 100644
--- a/src/sentry/static/sentry/app/components/charts/utils.tsx
+++ b/src/sentry/static/sentry/app/components/charts/utils.tsx
@@ -128,7 +128,7 @@ export function getSeriesSelection(
   location: Location,
   parameter = 'unselectedSeries'
 ): EChartOption.Legend['selected'] {
-  const unselectedSeries = decodeList(location.query[parameter]) ?? [];
+  const unselectedSeries = decodeList(location?.query[parameter]) ?? [];
   return unselectedSeries.reduce((selection, series) => {
     selection[series] = false;
     return selection;
diff --git a/src/sentry/static/sentry/app/views/dashboardsV2/controls.tsx b/src/sentry/static/sentry/app/views/dashboardsV2/controls.tsx
index af69d0858c0884..b84cba6ecb097d 100644
--- a/src/sentry/static/sentry/app/views/dashboardsV2/controls.tsx
+++ b/src/sentry/static/sentry/app/views/dashboardsV2/controls.tsx
@@ -4,6 +4,7 @@ import styled from '@emotion/styled';
 
 import Button from 'app/components/button';
 import ButtonBar from 'app/components/buttonBar';
+import Confirm from 'app/components/confirm';
 import SelectControl from 'app/components/forms/selectControl';
 import {IconAdd, IconEdit} from 'app/icons';
 import {t} from 'app/locale';
@@ -58,16 +59,15 @@ class Controls extends React.Component<Props> {
       return (
         <ButtonBar gap={1} key="edit-controls">
           {cancelButton}
-          <Button
-            data-test-id="dashboard-delete"
-            onClick={e => {
-              e.preventDefault();
-              onDelete();
-            }}
+          <Confirm
             priority="danger"
+            message={t('Are you sure you want to delete this dashboard?')}
+            onConfirm={onDelete}
           >
-            {t('Delete')}
-          </Button>
+            <Button data-test-id="dashboard-delete" priority="danger">
+              {t('Delete')}
+            </Button>
+          </Confirm>
           <Button
             data-test-id="dashboard-commit"
             onClick={e => {
diff --git a/tests/js/spec/views/dashboardsV2/detail.spec.jsx b/tests/js/spec/views/dashboardsV2/detail.spec.jsx
index 7556512d316189..ec105fd1a00f40 100644
--- a/tests/js/spec/views/dashboardsV2/detail.spec.jsx
+++ b/tests/js/spec/views/dashboardsV2/detail.spec.jsx
@@ -3,14 +3,10 @@ import {browserHistory} from 'react-router';
 
 import {mountWithTheme} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
+import {mountGlobalModal} from 'sentry-test/modal';
 
-import {openAddDashboardWidgetModal} from 'app/actionCreators/modal';
 import DashboardDetail from 'app/views/dashboardsV2/detail';
 
-jest.mock('app/actionCreators/modal', () => ({
-  openAddDashboardWidgetModal: jest.fn(),
-}));
-
 describe('Dashboards > Detail', function () {
   const organization = TestStubs.Organization({
     features: ['global-views', 'dashboards-v2', 'discover-query'],
@@ -70,8 +66,17 @@ describe('Dashboards > Detail', function () {
       // Enter edit mode.
       wrapper.find('Controls Button[data-test-id="dashboard-edit"]').simulate('click');
 
-      // Click delete, request should be made.
+      const modal = await mountGlobalModal();
+
+      // Click delete, confirm will show
       wrapper.find('Controls Button[data-test-id="dashboard-delete"]').simulate('click');
+      await tick();
+
+      await modal.update();
+
+      // Click confirm
+      modal.find('button[aria-label="Confirm"]').simulate('click');
+
       expect(deleteMock).toHaveBeenCalled();
     });
 
@@ -128,14 +133,20 @@ describe('Dashboards > Detail', function () {
     beforeEach(function () {
       initialData = initializeOrg({organization});
       widgets = [
-        TestStubs.Widget([{conditions: 'event.type:error', fields: ['count()']}], {
-          title: 'Errors',
-          interval: '1d',
-        }),
-        TestStubs.Widget([{conditions: 'event.type:transaction', fields: ['count()']}], {
-          title: 'Transactions',
-          interval: '1d',
-        }),
+        TestStubs.Widget(
+          [{name: '', conditions: 'event.type:error', fields: ['count()']}],
+          {
+            title: 'Errors',
+            interval: '1d',
+          }
+        ),
+        TestStubs.Widget(
+          [{name: '', conditions: 'event.type:transaction', fields: ['count()']}],
+          {
+            title: 'Transactions',
+            interval: '1d',
+          }
+        ),
       ];
 
       MockApiClient.addMockResponse({
@@ -241,14 +252,10 @@ describe('Dashboards > Detail', function () {
         .simulate('click');
 
       await tick();
-      wrapper.update();
+      await wrapper.update();
+      const modal = await mountGlobalModal();
 
-      expect(openAddDashboardWidgetModal).toHaveBeenCalled();
-      expect(openAddDashboardWidgetModal).toHaveBeenCalledWith(
-        expect.objectContaining({
-          widget: widgets[0],
-        })
-      );
+      expect(modal.find('AddDashboardWidgetModal').props().widget).toEqual(widgets[0]);
     });
   });
 });