Browse Source

fix(dashboards): fixes tags not always being loaded in dashboards (#33466)

Causes tags to always be fetched on dashboard mount (instead of just edit mode). This is because a widget edit modal can be opened without having to enter edit mode.

Also updates the widget builder to fetch tags if tag store is empty. This is needed when a user visits an edit link directly because tags don't get loaded otherwise.
edwardgou-sentry 2 years ago
parent
commit
4e9bebcc41

+ 4 - 11
static/app/views/dashboardsV2/dashboard.tsx

@@ -140,15 +140,13 @@ class Dashboard extends Component<Props, State> {
   }
 
   async componentDidMount() {
-    const {isEditing, organization, newWidget} = this.props;
+    const {organization, newWidget} = this.props;
     if (organization.features.includes('dashboard-grid-layout')) {
       window.addEventListener('resize', this.debouncedHandleResize);
     }
 
-    // Load organization tags when in edit mode.
-    if (isEditing) {
-      this.fetchTags();
-    }
+    // Always load organization tags on dashboards
+    this.fetchTags();
 
     if (newWidget) {
       this.addNewWidget();
@@ -159,13 +157,8 @@ class Dashboard extends Component<Props, State> {
   }
 
   async componentDidUpdate(prevProps: Props) {
-    const {selection, isEditing, newWidget} = this.props;
+    const {selection, newWidget} = this.props;
 
-    // Load organization tags when going into edit mode.
-    // We use tags on the add widget modal.
-    if (prevProps.isEditing !== isEditing && isEditing) {
-      this.fetchTags();
-    }
     if (newWidget && newWidget !== prevProps.newWidget) {
       this.addNewWidget();
     }

+ 8 - 1
static/app/views/dashboardsV2/widgetBuilder/widgetBuilder.tsx

@@ -9,6 +9,7 @@ import set from 'lodash/set';
 import {validateWidget} from 'sentry/actionCreators/dashboards';
 import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
 import {fetchOrgMembers} from 'sentry/actionCreators/members';
+import {loadOrganizationTags} from 'sentry/actionCreators/tags';
 import {generateOrderOptions} from 'sentry/components/dashboards/widgetQueriesForm';
 import * as Layout from 'sentry/components/layouts/thirds';
 import List from 'sentry/components/list';
@@ -25,7 +26,7 @@ import {
   SelectValue,
   TagCollection,
 } from 'sentry/types';
-import {defined} from 'sentry/utils';
+import {defined, objectIsEmpty} from 'sentry/utils';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
 import {
   explodeField,
@@ -186,6 +187,12 @@ function WidgetBuilder({
     location.query.defaultWidgetQuery
   );
 
+  useEffect(() => {
+    if (objectIsEmpty(tags)) {
+      loadOrganizationTags(api, organization.slug, selection);
+    }
+  }, []);
+
   const isEditing = defined(widgetIndex);
   const widgetIndexNum = Number(widgetIndex);
   const isValidWidgetIndex =

+ 5 - 0
tests/js/spec/views/dashboardsV2/dashboard.spec.tsx

@@ -45,6 +45,11 @@ describe('Dashboards > Dashboard', () => {
       method: 'GET',
       body: [],
     });
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/tags/',
+      method: 'GET',
+      body: TestStubs.Tags(),
+    });
   });
   it('dashboard adds new widget if component is mounted with newWidget prop', async () => {
     const mockHandleAddCustomWidget = jest.fn();

+ 21 - 2
tests/js/spec/views/dashboardsV2/gridLayout/dashboard.spec.tsx

@@ -54,7 +54,7 @@ describe('Dashboards > Dashboard', () => {
     ],
   };
 
-  let initialData;
+  let initialData, tagsMock;
 
   beforeEach(() => {
     initialData = initializeOrg({organization, router: {}, project: 1, projects: []});
@@ -105,13 +105,32 @@ describe('Dashboards > Dashboard', () => {
         },
       ],
     });
-    MockApiClient.addMockResponse({
+    tagsMock = MockApiClient.addMockResponse({
       url: '/organizations/org-slug/tags/',
       method: 'GET',
       body: TestStubs.Tags(),
     });
   });
 
+  it('fetches tags', () => {
+    mountWithTheme(
+      <Dashboard
+        paramDashboardId="1"
+        dashboard={mockDashboard}
+        organization={initialData.organization}
+        onUpdate={() => undefined}
+        handleUpdateWidgetList={() => undefined}
+        handleAddCustomWidget={() => undefined}
+        router={initialData.router}
+        location={initialData.location}
+        widgetLimitReached={false}
+        isEditing={false}
+      />,
+      initialData.routerContext
+    );
+    expect(tagsMock).toHaveBeenCalled();
+  });
+
   it('dashboard adds new widget if component is mounted with newWidget prop', async () => {
     const mockHandleAddCustomWidget = jest.fn();
     const mockCallbackToUnsetNewWidget = jest.fn();

+ 20 - 0
tests/js/spec/views/dashboardsV2/widgetBuilder/widgetBuilder.spec.tsx

@@ -8,6 +8,7 @@ import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import * as indicators from 'sentry/actionCreators/indicator';
 import * as modals from 'sentry/actionCreators/modal';
+import TagStore from 'sentry/stores/tagStore';
 import {TOP_N} from 'sentry/utils/discover/types';
 import {SessionMetric} from 'sentry/utils/metrics/fields';
 import {
@@ -109,6 +110,7 @@ describe('WidgetBuilder', function () {
   let eventsStatsMock: jest.Mock | undefined;
   let eventsv2Mock: jest.Mock | undefined;
   let metricsDataMock: jest.Mock | undefined;
+  let tagsMock: jest.Mock | undefined;
 
   beforeEach(function () {
     MockApiClient.addMockResponse({
@@ -225,6 +227,13 @@ describe('WidgetBuilder', function () {
         field: `sum(${SessionMetric.SESSION})`,
       }),
     });
+
+    tagsMock = MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/tags/',
+      method: 'GET',
+      body: TestStubs.Tags(),
+    });
+    TagStore.reset();
   });
 
   afterEach(function () {
@@ -1319,6 +1328,17 @@ describe('WidgetBuilder', function () {
     });
   });
 
+  it('fetches tags when tag store is empty', function () {
+    renderTestComponent();
+    expect(tagsMock).toHaveBeenCalled();
+  });
+
+  it('does not fetch tags when tag store is not empty', function () {
+    TagStore.loadTagsSuccess(TestStubs.Tags());
+    renderTestComponent();
+    expect(tagsMock).not.toHaveBeenCalled();
+  });
+
   describe('Sort by selectors', function () {
     it('renders', async function () {
       renderTestComponent({