diff --git a/src/sentry/static/sentry/app/views/organizationIncidents/details/activity/index.jsx b/src/sentry/static/sentry/app/views/organizationIncidents/details/activity/index.jsx index a05bc783bcf2d8..ffed98eae9fdac 100644 --- a/src/sentry/static/sentry/app/views/organizationIncidents/details/activity/index.jsx +++ b/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; diff --git a/src/sentry/static/sentry/app/views/organizationIncidents/details/body.jsx b/src/sentry/static/sentry/app/views/organizationIncidents/details/body.jsx index a1860a7b51316d..7c26557ca2b92c 100644 --- a/src/sentry/static/sentry/app/views/organizationIncidents/details/body.jsx +++ b/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 (
- {Object.entries(TABS).map(([id, {name}]) => ( -
  • - this.handleToggle(id)}>{name} -
    • - ))} +
  • + {t('Activity')} +
    • {incident && ( @@ -58,7 +41,10 @@ export default class DetailsBody extends React.Component { )}     - +
    diff --git a/src/sentry/static/sentry/app/views/organizationIncidents/details/chart.jsx b/src/sentry/static/sentry/app/views/organizationIncidents/details/chart.jsx index ff5fada2be62f7..1a43d73a841a96 100644 --- a/src/sentry/static/sentry/app/views/organizationIncidents/details/chart.jsx +++ b/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; diff --git a/src/sentry/static/sentry/app/views/organizationIncidents/details/index.jsx b/src/sentry/static/sentry/app/views/organizationIncidents/details/index.jsx index 3fde3661df5b2a..44b4c4e56512e9 100644 --- a/src/sentry/static/sentry/app/views/organizationIncidents/details/index.jsx +++ b/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() { diff --git a/tests/js/spec/views/organizationIncidents/details/index.spec.jsx b/tests/js/spec/views/organizationIncidents/details/index.spec.jsx index d7d990da00c8c0..086f0c41a39335 100644 --- a/tests/js/spec/views/organizationIncidents/details/index.spec.jsx +++ b/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'); });