fix(dashboard): Set modified dashboard to null after update (#31511)

Set modified dashboard to null after update, so that
updated dashboard is used for re-render instead of
stale modified dashboard state. Also fix some
dashboard acceptance flakiness.

static/app/views/dashboardsV2/detail.tsx

@@ -336,6 +336,9 @@ class DashboardDetail extends Component<Props, State> {
       (newDashboard: DashboardDetails) => {
         if (onDashboardUpdate) {
           onDashboardUpdate(newDashboard);
+          this.setState({
+            modifiedDashboard: null,
+          });
         }
         addSuccessMessage(t('Dashboard updated'));
         if (dashboard && newDashboard.id !== dashboard.id) {

static/app/views/dashboardsV2/widgetCard/widgetCardContextMenu.tsx

@@ -199,12 +199,13 @@ function WidgetCardContextMenu({
     menuOptions.push(
       <Confirm
         key="delete-widget"
-        data-test-id="delete-widget"
         priority="danger"
         message={t('Are you sure you want to delete this widget?')}
         onConfirm={onDelete}
       >
-        <StyledMenuItem danger>{t('Delete Widget')}</StyledMenuItem>
+        <StyledMenuItem data-test-id="delete-widget" danger>
+          {t('Delete Widget')}
+        </StyledMenuItem>
       </Confirm>
     );
   }

tests/acceptance/page_objects/dashboard_detail.py

@@ -14,12 +14,15 @@ class DashboardDetailPage(BasePage):
         self.dashboard = kwargs.get("dashboard", None)
 
     def wait_until_loaded(self):
-        self.browser.wait_until_not(".loading-indicator")
+        self.browser.wait_until_not('[data-test-id="events-request-loading"]')
+        self.browser.wait_until_not('[data-test-id="loading-indicator"]')
         self.browser.wait_until_not('[data-test-id="loading-placeholder"]')
 
     def visit_default_overview(self):
         self.browser.get(f"/organizations/{self.organization.slug}/dashboard/default-overview/")
         self.wait_until_loaded()
+        self.browser.driver.execute_script("window.scrollTo(0, document.body.scrollHeight)")
+        self.wait_until_loaded()
 
     def visit_create_dashboard(self):
         self.browser.get(f"/organizations/{self.organization.slug}/dashboards/new/")

tests/acceptance/test_organization_dashboards.py

@@ -43,7 +43,23 @@ class OrganizationDashboardsAcceptanceTest(AcceptanceTestCase):
             data={"event_id": "a" * 32, "message": "oh no", "timestamp": min_ago},
             project_id=self.project.id,
         )
-        self.page = DashboardDetailPage(self.browser, self.client, organization=self.organization)
+        self.dashboard = Dashboard.objects.create(
+            title="Dashboard 1", created_by=self.user, organization=self.organization
+        )
+        self.existing_widget = DashboardWidget.objects.create(
+            dashboard=self.dashboard,
+            order=0,
+            title="Existing Widget",
+            display_type=DashboardWidgetDisplayTypes.LINE_CHART,
+            widget_type=DashboardWidgetTypes.DISCOVER,
+            interval="1d",
+        )
+        DashboardWidgetQuery.objects.create(
+            widget=self.existing_widget, fields=["count()"], order=0
+        )
+        self.page = DashboardDetailPage(
+            self.browser, self.client, organization=self.organization, dashboard=self.dashboard
+        )
         self.login_as(self.user)
 
     def test_view_dashboard(self):
@@ -97,6 +113,32 @@ class OrganizationDashboardsAcceptanceTest(AcceptanceTestCase):
 
             self.browser.snapshot("dashboards - widget library")
 
+    def test_duplicate_widget_in_view_mode(self):
+        with self.feature(FEATURE_NAMES + EDIT_FEATURE):
+            self.page.visit_dashboard_detail()
+
+            self.browser.element('[data-test-id="context-menu"]').click()
+            self.browser.element('[data-test-id="duplicate-widget"]').click()
+            self.page.wait_until_loaded()
+
+            self.browser.elements('[data-test-id="context-menu"]')[0].click()
+            self.browser.element('[data-test-id="duplicate-widget"]').click()
+            self.page.wait_until_loaded()
+
+            self.browser.snapshot("dashboard widget - duplicate")
+
+    def test_delete_widget_in_view_mode(self):
+        with self.feature(FEATURE_NAMES + EDIT_FEATURE):
+            self.page.visit_dashboard_detail()
+
+            self.browser.element('[data-test-id="context-menu"]').click()
+            self.browser.element('[data-test-id="delete-widget"]').click()
+            self.browser.element('[data-test-id="confirm-button"]').click()
+
+            self.page.wait_until_loaded()
+
+            self.browser.snapshot("dashboard widget - delete")
+
 
 class OrganizationDashboardLayoutAcceptanceTest(AcceptanceTestCase):
     def setUp(self):
@@ -418,6 +460,54 @@ class OrganizationDashboardLayoutAcceptanceTest(AcceptanceTestCase):
             self.page.wait_until_loaded()
             self.browser.snapshot("dashboards - default layout when widgets do not have layout set")
 
+    def test_duplicate_widget_in_view_mode(self):
+        existing_widget = DashboardWidget.objects.create(
+            dashboard=self.dashboard,
+            order=0,
+            title="Big Number Widget",
+            display_type=DashboardWidgetDisplayTypes.BIG_NUMBER,
+            widget_type=DashboardWidgetTypes.DISCOVER,
+            interval="1d",
+        )
+        DashboardWidgetQuery.objects.create(
+            widget=existing_widget, fields=["count_unique(issue)"], order=0
+        )
+        with self.feature(FEATURE_NAMES + EDIT_FEATURE):
+            self.page.visit_dashboard_detail()
+
+            self.browser.element('[data-test-id="context-menu"]').click()
+            self.browser.element('[data-test-id="duplicate-widget"]').click()
+            self.page.wait_until_loaded()
+
+            self.browser.elements('[data-test-id="context-menu"]')[0].click()
+            self.browser.element('[data-test-id="duplicate-widget"]').click()
+            self.page.wait_until_loaded()
+
+            self.browser.snapshot("dashboard widget - duplicate")
+
+    def test_delete_widget_in_view_mode(self):
+        existing_widget = DashboardWidget.objects.create(
+            dashboard=self.dashboard,
+            order=0,
+            title="Big Number Widget",
+            display_type=DashboardWidgetDisplayTypes.BIG_NUMBER,
+            widget_type=DashboardWidgetTypes.DISCOVER,
+            interval="1d",
+        )
+        DashboardWidgetQuery.objects.create(
+            widget=existing_widget, fields=["count_unique(issue)"], order=0
+        )
+        with self.feature(FEATURE_NAMES + EDIT_FEATURE):
+            self.page.visit_dashboard_detail()
+
+            self.browser.element('[data-test-id="context-menu"]').click()
+            self.browser.element('[data-test-id="delete-widget"]').click()
+            self.browser.element('[data-test-id="confirm-button"]').click()
+
+            self.page.wait_until_loaded()
+
+            self.browser.snapshot("dashboard widget - delete")
+
     def test_cancel_without_changes_does_not_trigger_confirm_with_widget_library_through_header(
         self,
     ):

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

@@ -688,7 +688,7 @@ describe('Dashboards > Detail', function () {
     });
 
     it('disables add library widgets when max widgets reached', async function () {
-      types.MAX_WIDGETS = 4;
+      types.MAX_WIDGETS = 3;
 
       initialData = initializeOrg({
         organization: TestStubs.Organization({
@@ -716,26 +716,6 @@ describe('Dashboards > Detail', function () {
       wrapper.update();
 
       expect(wrapper.find('WidgetCard')).toHaveLength(3);
-      expect(
-        wrapper.find('Controls Button[data-test-id="add-widget-library"]').props()
-          .disabled
-      ).toEqual(false);
-      expect(wrapper.find('Controls Tooltip').prop('disabled')).toBe(true);
-
-      const card = wrapper.find('WidgetCard').first();
-      card.find('DropdownMenu MoreOptions svg').simulate('click');
-
-      card.update();
-      wrapper.update();
-
-      wrapper
-        .find(`DropdownMenu MenuItem[data-test-id="duplicate-widget"] MenuTarget`)
-        .simulate('click');
-
-      await tick();
-      wrapper.update();
-
-      expect(wrapper.find('WidgetCard')).toHaveLength(4);
       expect(
         wrapper.find('Controls Button[data-test-id="add-widget-library"]').props()
           .disabled
@@ -755,39 +735,6 @@ describe('Dashboards > Detail', function () {
       ).toEqual(true);
     });
 
-    it('duplicates widgets', async function () {
-      wrapper = mountWithTheme(
-        <ViewEditDashboard
-          organization={initialData.organization}
-          params={{orgId: 'org-slug', dashboardId: '1'}}
-          router={initialData.router}
-          location={initialData.router.location}
-        />,
-        initialData.routerContext
-      );
-      await tick();
-      wrapper.update();
-
-      expect(wrapper.find('WidgetCard')).toHaveLength(3);
-
-      const card = wrapper.find('WidgetCard').first();
-      card.find('DropdownMenu MoreOptions svg').simulate('click');
-
-      card.update();
-      wrapper.update();
-
-      wrapper
-        .find(`DropdownMenu MenuItem[data-test-id="duplicate-widget"] MenuTarget`)
-        .simulate('click');
-
-      await tick();
-      wrapper.update();
-
-      expect(wrapper.find('WidgetCard')).toHaveLength(4);
-      const newCard = wrapper.find('WidgetCard').at(1);
-      expect(newCard.props().title).toEqual(card.props().title);
-    });
-
     it('opens edit modal when editing widget from context menu', async function () {
       wrapper = mountWithTheme(
         <ViewEditDashboard
@@ -832,39 +779,5 @@ describe('Dashboards > Detail', function () {
         })
       );
     });
-
-    it('deletes widget', async function () {
-      wrapper = mountWithTheme(
-        <ViewEditDashboard
-          organization={initialData.organization}
-          params={{orgId: 'org-slug', dashboardId: '1'}}
-          router={initialData.router}
-          location={initialData.router.location}
-        />,
-        initialData.routerContext
-      );
-      await tick();
-      wrapper.update();
-
-      expect(wrapper.find('WidgetCard')).toHaveLength(3);
-
-      const card = wrapper.find('WidgetCard').first();
-      card.find('DropdownMenu MoreOptions svg').simulate('click');
-
-      card.update();
-      wrapper.update();
-
-      wrapper
-        .find(`DropdownMenu Confirm[data-test-id="delete-widget"]`)
-        .simulate('click');
-
-      const modal = await mountGlobalModal();
-      modal.find(`button[data-test-id="confirm-button"]`).simulate('click');
-
-      await tick();
-      wrapper.update();
-
-      expect(wrapper.find('WidgetCard')).toHaveLength(2);
-    });
   });
 });

tests/js/spec/views/dashboardsV2/widgetCard.spec.tsx

@@ -1,4 +1,5 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
+import {mountGlobalModal} from 'sentry-test/modal';
 import {mountWithTheme, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import * as modal from 'sentry/actionCreators/modal';
@@ -362,6 +363,14 @@ describe('Dashboards > WidgetCard', function () {
 
     userEvent.click(screen.getByTestId('context-menu'));
     expect(screen.getByText('Delete Widget')).toBeInTheDocument();
+    userEvent.click(screen.getByText('Delete Widget'));
+    // Confirm Modal
+    mountGlobalModal();
+    await screen.findByRole('dialog');
+
+    userEvent.click(screen.getByTestId('confirm-button'));
+
+    expect(mock).toHaveBeenCalled();
   });
 
   it('calls eventsV2 with a limit of 20 items', async function () {