fix(dashboard-layout): Deleting widgets resets layouts (#30242)

Avoid using indices for widget keys. When deleting widgets, the number of widgets
changes and the layout loses reference to the widget (because the key is now different
due to the index dependency)

static/app/views/dashboardsV2/detail.tsx

@@ -28,7 +28,7 @@ import {trackAnalyticsEvent} from 'sentry/utils/analytics';
 import withApi from 'sentry/utils/withApi';
 import withOrganization from 'sentry/utils/withOrganization';
 
-import GridLayoutDashboard, {generateWidgetId} from './gridLayout/dashboard';
+import GridLayoutDashboard, {constructGridItemKey} from './gridLayout/dashboard';
 import {getDashboardLayout, saveDashboardLayout} from './gridLayout/utils';
 import Controls from './controls';
 import DnDKitDashboard from './dashboard';
@@ -313,7 +313,7 @@ class DashboardDetail extends Component<Props, State> {
       dashboardId,
       layout.map((widgetLayout, index) => ({
         ...widgetLayout,
-        i: generateWidgetId(newWidgets[index], index),
+        i: constructGridItemKey(newWidgets[index]),
       }))
     );
   };

static/app/views/dashboardsV2/gridLayout/dashboard.tsx

@@ -33,7 +33,7 @@ import {DataSet} from 'sentry/views/dashboardsV2/widget/utils';
 import SortableWidget from './sortableWidget';
 
 export const DRAG_HANDLE_CLASS = 'widget-drag';
-const SAVED_WIDGET_PREFIX = 'grid-item';
+const WIDGET_PREFIX = 'grid-item';
 const NUM_COLS = 6;
 const ROW_HEIGHT = 120;
 const WIDGET_MARGINS: [number, number] = [16, 16];
@@ -145,7 +145,10 @@ class Dashboard extends Component<Props> {
   };
 
   handleAddComplete = (widget: Widget) => {
-    this.props.onUpdate([...this.props.dashboard.widgets, widget]);
+    this.props.onUpdate([
+      ...this.props.dashboard.widgets,
+      {...widget, tempId: Date.now().toString()},
+    ]);
   };
 
   handleUpdateComplete = (index: number) => (nextWidget: Widget) => {
@@ -154,9 +157,10 @@ class Dashboard extends Component<Props> {
     this.props.onUpdate(nextList);
   };
 
-  handleDeleteWidget = (index: number) => () => {
-    const nextList = [...this.props.dashboard.widgets];
-    nextList.splice(index, 1);
+  handleDeleteWidget = (widgetToDelete: Widget) => () => {
+    const nextList = this.props.dashboard.widgets.filter(
+      widget => widget !== widgetToDelete
+    );
     this.props.onUpdate(nextList);
   };
 
@@ -217,7 +221,7 @@ class Dashboard extends Component<Props> {
   renderWidget(widget: Widget, index: number) {
     const {isEditing} = this.props;
 
-    const key = generateWidgetId(widget, index);
+    const key = constructGridItemKey(widget);
     const dragId = key;
 
     return (
@@ -226,7 +230,7 @@ class Dashboard extends Component<Props> {
           widget={widget}
           dragId={dragId}
           isEditing={isEditing}
-          onDelete={this.handleDeleteWidget(index)}
+          onDelete={this.handleDeleteWidget(widget)}
           onEdit={this.handleEditWidget(widget, index)}
         />
       </GridItem>
@@ -280,8 +284,8 @@ const GridItem = styled('div')`
   }
 `;
 
-export function generateWidgetId(widget: Widget, index: number) {
-  return widget.id ? `${SAVED_WIDGET_PREFIX}-${widget.id}` : `index-${index}`;
+export function constructGridItemKey(widget: Widget) {
+  return `${WIDGET_PREFIX}-${widget.id ?? widget.tempId}`;
 }
 
 /**

static/app/views/dashboardsV2/types.tsx

@@ -36,6 +36,7 @@ export type Widget = {
   interval: string;
   queries: WidgetQuery[];
   widgetType?: WidgetType;
+  tempId?: string;
 };
 
 /**

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

@@ -186,7 +186,7 @@ describe('Dashboards > Detail', function () {
   });
 
   describe('custom dashboards', function () {
-    let wrapper, initialData, widgets, mockPut;
+    let wrapper, initialData, widgets, mockVisit, mockPut;
 
     beforeEach(function () {
       initialData = initializeOrg({organization});
@@ -222,8 +222,7 @@ describe('Dashboards > Detail', function () {
           }
         ),
       ];
-      // TODO(nar): Assign to mockVisit when restoring 'can remove widgets' test
-      MockApiClient.addMockResponse({
+      mockVisit = MockApiClient.addMockResponse({
         url: '/organizations/org-slug/dashboards/1/visit/',
         method: 'POST',
         body: [],
@@ -285,7 +284,60 @@ describe('Dashboards > Detail', function () {
       MockApiClient.clearMockResponses();
     });
 
-    // TODO(nar): Add deletion test
+    it('can remove widgets', async function () {
+      const updateMock = MockApiClient.addMockResponse({
+        url: '/organizations/org-slug/dashboards/1/',
+        method: 'PUT',
+        body: {widgets: [widgets[0]]},
+      });
+      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(mockVisit).toHaveBeenCalledTimes(1);
+
+      // Enter edit mode.
+      wrapper.find('Controls Button[data-test-id="dashboard-edit"]').simulate('click');
+
+      // Remove the second and third widgets
+      wrapper
+        .find('WidgetCard')
+        .at(1)
+        .find('IconClick[data-test-id="widget-delete"]')
+        .simulate('click');
+
+      wrapper
+        .find('WidgetCard')
+        .at(1)
+        .find('IconClick[data-test-id="widget-delete"]')
+        .simulate('click');
+
+      // Save changes
+      wrapper.find('Controls Button[data-test-id="dashboard-commit"]').simulate('click');
+      await tick();
+
+      expect(updateMock).toHaveBeenCalled();
+      expect(updateMock).toHaveBeenCalledWith(
+        '/organizations/org-slug/dashboards/1/',
+        expect.objectContaining({
+          data: expect.objectContaining({
+            title: 'Custom Errors',
+            widgets: [widgets[0]],
+          }),
+        })
+      );
+
+      // Visit should not be called again on dashboard update
+      expect(mockVisit).toHaveBeenCalledTimes(1);
+    });
 
     it('can enter edit mode for widgets', async function () {
       wrapper = mountWithTheme(