Browse Source

fix(dashboards): Move router calls after clearing editing state (#66004)

Scott Cooper 1 year ago
parent
commit
b210525979

+ 78 - 58
static/app/views/dashboards/detail.tsx

@@ -535,23 +535,25 @@ class DashboardDetail extends Component<Props, State> {
       this.handleAddMetricWidget();
       this.handleAddMetricWidget();
       return;
       return;
     }
     }
-    this.setState({
-      modifiedDashboard: cloneDashboard(dashboard),
-    });
-
-    if (dashboardId) {
-      router.push(
-        normalizeUrl({
-          pathname: `/organizations/${organization.slug}/dashboard/${dashboardId}/widget/new/`,
-          query: {
-            ...location.query,
-            source: DashboardWidgetSource.DASHBOARDS,
-            dataset,
-          },
-        })
-      );
-      return;
-    }
+    this.setState(
+      {
+        modifiedDashboard: cloneDashboard(dashboard),
+      },
+      () => {
+        if (dashboardId) {
+          router.push(
+            normalizeUrl({
+              pathname: `/organizations/${organization.slug}/dashboard/${dashboardId}/widget/new/`,
+              query: {
+                ...location.query,
+                source: DashboardWidgetSource.DASHBOARDS,
+                dataset,
+              },
+            })
+          );
+        }
+      }
+    );
   };
   };
 
 
   onCommit = () => {
   onCommit = () => {
@@ -584,18 +586,21 @@ class DashboardDetail extends Component<Props, State> {
             (newDashboard: DashboardDetails) => {
             (newDashboard: DashboardDetails) => {
               addSuccessMessage(t('Dashboard created'));
               addSuccessMessage(t('Dashboard created'));
               trackAnalytics('dashboards2.create.complete', {organization});
               trackAnalytics('dashboards2.create.complete', {organization});
-              this.setState({
-                dashboardState: DashboardState.VIEW,
-              });
-
-              // redirect to new dashboard
-              browserHistory.replace(
-                normalizeUrl({
-                  pathname: `/organizations/${organization.slug}/dashboard/${newDashboard.id}/`,
-                  query: {
-                    query: omit(location.query, Object.values(DashboardFilterKeys)),
-                  },
-                })
+              this.setState(
+                {
+                  dashboardState: DashboardState.VIEW,
+                },
+                () => {
+                  // redirect to new dashboard
+                  browserHistory.replace(
+                    normalizeUrl({
+                      pathname: `/organizations/${organization.slug}/dashboard/${newDashboard.id}/`,
+                      query: {
+                        query: omit(location.query, Object.values(DashboardFilterKeys)),
+                      },
+                    })
+                  );
+                }
               );
               );
             },
             },
             () => undefined
             () => undefined
@@ -620,22 +625,24 @@ class DashboardDetail extends Component<Props, State> {
               }
               }
               addSuccessMessage(t('Dashboard updated'));
               addSuccessMessage(t('Dashboard updated'));
               trackAnalytics('dashboards2.edit.complete', {organization});
               trackAnalytics('dashboards2.edit.complete', {organization});
-              this.setState({
-                dashboardState: DashboardState.VIEW,
-                modifiedDashboard: null,
-              });
-
-              if (dashboard && newDashboard.id !== dashboard.id) {
-                browserHistory.replace(
-                  normalizeUrl({
-                    pathname: `/organizations/${organization.slug}/dashboard/${newDashboard.id}/`,
-                    query: {
-                      ...location.query,
-                    },
-                  })
-                );
-                return;
-              }
+              this.setState(
+                {
+                  dashboardState: DashboardState.VIEW,
+                  modifiedDashboard: null,
+                },
+                () => {
+                  if (dashboard && newDashboard.id !== dashboard.id) {
+                    browserHistory.replace(
+                      normalizeUrl({
+                        pathname: `/organizations/${organization.slug}/dashboard/${newDashboard.id}/`,
+                        query: {
+                          ...location.query,
+                        },
+                      })
+                    );
+                  }
+                }
+              );
             },
             },
             // `updateDashboard` does its own error handling
             // `updateDashboard` does its own error handling
             () => undefined
             () => undefined
@@ -938,22 +945,35 @@ class DashboardDetail extends Component<Props, State> {
                                       newModifiedDashboard
                                       newModifiedDashboard
                                     ).then(
                                     ).then(
                                       (newDashboard: DashboardDetails) => {
                                       (newDashboard: DashboardDetails) => {
+                                        addSuccessMessage(t('Dashboard filters updated'));
+
+                                        const navigateToDashboard = () => {
+                                          browserHistory.replace(
+                                            normalizeUrl({
+                                              pathname: `/organizations/${organization.slug}/dashboard/${newDashboard.id}/`,
+                                              query: omit(
+                                                location.query,
+                                                Object.values(DashboardFilterKeys)
+                                              ),
+                                            })
+                                          );
+                                        };
+
                                         if (onDashboardUpdate) {
                                         if (onDashboardUpdate) {
                                           onDashboardUpdate(newDashboard);
                                           onDashboardUpdate(newDashboard);
-                                          this.setState({
-                                            modifiedDashboard: null,
-                                          });
+                                          this.setState(
+                                            {
+                                              modifiedDashboard: null,
+                                            },
+                                            () => {
+                                              // Wait for modifiedDashboard state to update before navigating
+                                              navigateToDashboard();
+                                            }
+                                          );
+                                          return;
                                         }
                                         }
-                                        addSuccessMessage(t('Dashboard filters updated'));
-                                        browserHistory.replace(
-                                          normalizeUrl({
-                                            pathname: `/organizations/${organization.slug}/dashboard/${newDashboard.id}/`,
-                                            query: omit(
-                                              location.query,
-                                              Object.values(DashboardFilterKeys)
-                                            ),
-                                          })
-                                        );
+
+                                        navigateToDashboard();
                                       },
                                       },
                                       // `updateDashboard` does its own error handling
                                       // `updateDashboard` does its own error handling
                                       () => undefined
                                       () => undefined

+ 6 - 6
static/app/views/dashboards/widgetBuilder/widgetBuilder.tsx

@@ -1,4 +1,4 @@
-import {useEffect, useMemo, useState} from 'react';
+import {useEffect, useMemo, useRef, useState} from 'react';
 import type {RouteComponentProps} from 'react-router';
 import type {RouteComponentProps} from 'react-router';
 import styled from '@emotion/styled';
 import styled from '@emotion/styled';
 import cloneDeep from 'lodash/cloneDeep';
 import cloneDeep from 'lodash/cloneDeep';
@@ -216,7 +216,7 @@ function WidgetBuilder({
 
 
   const api = useApi();
   const api = useApi();
 
 
-  const [isSubmitting, setIsSubmitting] = useState(false);
+  const isSubmittingRef = useRef(false);
 
 
   const [datasetConfig, setDataSetConfig] = useState<ReturnType<typeof getDatasetConfig>>(
   const [datasetConfig, setDataSetConfig] = useState<ReturnType<typeof getDatasetConfig>>(
     getDatasetConfig(DATA_SET_TO_WIDGET_TYPE[dataSet])
     getDatasetConfig(DATA_SET_TO_WIDGET_TYPE[dataSet])
@@ -355,14 +355,14 @@ function WidgetBuilder({
 
 
   useEffect(() => {
   useEffect(() => {
     const onUnload = () => {
     const onUnload = () => {
-      if (!isSubmitting && state.userHasModified) {
+      if (!isSubmittingRef.current && state.userHasModified) {
         return t('You have unsaved changes, are you sure you want to leave?');
         return t('You have unsaved changes, are you sure you want to leave?');
       }
       }
       return undefined;
       return undefined;
     };
     };
 
 
     router.setRouteLeaveHook(route, onUnload);
     router.setRouteLeaveHook(route, onUnload);
-  }, [isSubmitting, state.userHasModified, route, router]);
+  }, [state.userHasModified, route, router]);
 
 
   const widgetType = DATA_SET_TO_WIDGET_TYPE[state.dataSet];
   const widgetType = DATA_SET_TO_WIDGET_TYPE[state.dataSet];
 
 
@@ -773,7 +773,7 @@ function WidgetBuilder({
       return;
       return;
     }
     }
 
 
-    setIsSubmitting(true);
+    isSubmittingRef.current = true;
     let nextWidgetList = [...dashboard.widgets];
     let nextWidgetList = [...dashboard.widgets];
     const updateWidgetIndex = getUpdateWidgetIndex();
     const updateWidgetIndex = getUpdateWidgetIndex();
     nextWidgetList.splice(updateWidgetIndex, 1);
     nextWidgetList.splice(updateWidgetIndex, 1);
@@ -813,7 +813,7 @@ function WidgetBuilder({
       });
       });
     }
     }
 
 
-    setIsSubmitting(true);
+    isSubmittingRef.current = true;
     if (notDashboardsOrigin) {
     if (notDashboardsOrigin) {
       submitFromSelectedDashboard(widgetData);
       submitFromSelectedDashboard(widgetData);
       return;
       return;