feat(ui): Refresh Incidents activities and chart when status changes [SEN-683]

src/sentry/static/sentry/app/views/organizationIncidents/details/activity/index.jsx

@@ -24,9 +24,10 @@ function makeDefaultErrorJson() {
  * Allows user to leave a comment on an incidentId as well as
  * fetch and render existing activity items.
  */
-class ActivityContainer extends React.Component {
+class ActivityContainer extends React.PureComponent {
   static propTypes = {
     api: PropTypes.object.isRequired,
+    incidentStatus: PropTypes.number,
   };
 
   state = {
@@ -43,6 +44,20 @@ class ActivityContainer extends React.Component {
     this.fetchData();
   }
 
+  componentDidUpdate(prevProps) {
+    // Only refetch if incidentStatus changes.
+    //
+    // This component can mount before incident details is fully loaded.
+    // In which case, `incidentStatus` is null and we will be fetching via `cDM`
+    // There's no need to fetch this gets updated due to incident details being loaded
+    if (
+      prevProps.incidentStatus !== null &&
+      prevProps.incidentStatus !== this.props.incidentStatus
+    ) {
+      this.fetchData();
+    }
+  }
+
   async fetchData() {
     const {api, params} = this.props;
     const {incidentId, orgId} = params;

src/sentry/static/sentry/app/views/organizationIncidents/details/body.jsx

@@ -14,39 +14,22 @@ import theme from 'app/utils/theme';
 import Activity from './activity';
 import IncidentsSuspects from './suspects';
 
-const TABS = {
-  activity: {name: t('Activity'), component: Activity},
-};
-
 export default class DetailsBody extends React.Component {
   static propTypes = {
     incident: SentryTypes.Incident,
   };
-  constructor(props) {
-    super(props);
-    this.state = {
-      activeTab: Object.keys(TABS)[0],
-    };
-  }
-  handleToggle(tab) {
-    this.setState({activeTab: tab});
-  }
 
   render() {
     const {params, incident} = this.props;
-    const {activeTab} = this.state;
-    const ActiveComponent = TABS[activeTab].component;
 
     return (
       <StyledPageContent>
         <Main>
           <PageContent>
             <StyledNavTabs underlined={true}>
-              {Object.entries(TABS).map(([id, {name}]) => (
-                <li key={id} className={activeTab === id ? 'active' : ''}>
-                  <Link onClick={() => this.handleToggle(id)}>{name}</Link>
-                </li>
-              ))}
+              <li className="active">
+                <Link>{t('Activity')}</Link>
+              </li>
 
               <SeenByTab>
                 {incident && (
@@ -58,7 +41,10 @@ export default class DetailsBody extends React.Component {
                 )}
               </SeenByTab>
             </StyledNavTabs>
-            <ActiveComponent params={params} incident={incident} />
+            <Activity
+              params={params}
+              incidentStatus={incident ? incident.status : null}
+            />
           </PageContent>
         </Main>
         <Sidebar>

src/sentry/static/sentry/app/views/organizationIncidents/details/chart.jsx

@@ -31,12 +31,13 @@ function getNearbyIndex(data, needle) {
   return index !== -1 ? index - 1 : data.length - 1;
 }
 
-export default class Chart extends React.Component {
+export default class Chart extends React.PureComponent {
   static propTypes = {
     data: PropTypes.arrayOf(PropTypes.number),
     detected: PropTypes.string,
     closed: PropTypes.string,
   };
+
   render() {
     const {data, detected, closed} = this.props;
 

src/sentry/static/sentry/app/views/organizationIncidents/details/index.jsx

@@ -85,13 +85,19 @@ class OrganizationIncidentDetails extends React.Component {
       incident: {...state.incident, status: newStatus},
     }));
 
-    updateStatus(api, orgId, incidentId, newStatus).catch(() => {
-      this.setState(state => ({
-        incident: {...state.incident, status},
-      }));
+    updateStatus(api, orgId, incidentId, newStatus)
+      .then(incident => {
+        // Update entire incident object because updating status can cause other parts
+        // of the model to change (e.g close date)
+        this.setState({incident});
+      })
+      .catch(() => {
+        this.setState(state => ({
+          incident: {...state.incident, status},
+        }));
 
-      addErrorMessage(t('An error occurred, your incident status was not changed.'));
-    });
+        addErrorMessage(t('An error occurred, your incident status was not changed.'));
+      });
   };
 
   render() {

tests/js/spec/views/organizationIncidents/details/index.spec.jsx

@@ -1,11 +1,14 @@
 import React from 'react';
-import {mount} from 'enzyme';
 
+import {initializeOrg} from 'app-test/helpers/initializeOrg';
+import {mount} from 'enzyme';
 import IncidentDetails from 'app/views/organizationIncidents/details';
 
 describe('IncidentDetails', function() {
   const mockIncident = TestStubs.Incident();
-  const routerContext = TestStubs.routerContext();
+  const {organization, routerContext} = initializeOrg();
+
+  let activitiesList;
 
   const createWrapper = props =>
     mount(
@@ -25,12 +28,22 @@ describe('IncidentDetails', function() {
       url: '/organizations/org-slug/incidents/456/',
       statusCode: 404,
     });
+    activitiesList = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/incidents/${
+        mockIncident.identifier
+      }/activity/`,
+      body: [TestStubs.IncidentActivity()],
+    });
   });
 
   afterAll(function() {
     MockApiClient.clearMockResponses();
   });
 
+  beforeEach(function() {
+    activitiesList.mockClear();
+  });
+
   it('loads incident', async function() {
     const wrapper = createWrapper();
     expect(wrapper.find('IncidentTitle').text()).toBe('Loading');
@@ -59,11 +72,11 @@ describe('IncidentDetails', function() {
     await tick();
     wrapper.update();
 
-    // Activtiy will additionally have a LoadingError
+    // Activity will also have a LoadingError
     expect(wrapper.find('LoadingError')).toHaveLength(2);
   });
 
-  it('changes status to closed', async function() {
+  it('changes status to closed and fetches new activities', async function() {
     const updateStatus = MockApiClient.addMockResponse({
       url: '/organizations/org-slug/incidents/123/',
       method: 'PUT',
@@ -77,6 +90,8 @@ describe('IncidentDetails', function() {
     await tick();
     wrapper.update();
 
+    expect(activitiesList).toHaveBeenCalledTimes(1);
+
     expect(wrapper.find('Status').text()).toBe('Open');
     wrapper.find('[data-test-id="status-dropdown"] DropdownButton').simulate('click');
     wrapper
@@ -93,6 +108,8 @@ describe('IncidentDetails', function() {
       })
     );
 
+    // Refresh activities list since status changes also creates an activity
+    expect(activitiesList).toHaveBeenCalledTimes(2);
     expect(wrapper.find('Status').text()).toBe('Closed');
   });