From da9b4c5c1e81cf212def8c54e437f36da7b41e53 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 4 Jun 2019 16:20:28 -0400 Subject: [PATCH 01/13] feat(events-v2) Add a event graph to the event modal Show an event graph for grouped views. The grouped view mode requires showing a single event + grouped data above the single event view. I've implemented a stripped down line graph as we don't need release markers or previous period markers on this graph. Additionally we'll need special click handling that navigates to other events in the series. Refs SEN-716 --- .../sentry/app/actionCreators/events.jsx | 2 + .../utils/eventsRequest.jsx | 6 ++ .../app/views/organizationEventsV2/data.jsx | 38 +++++++- .../organizationEventsV2/eventDetails.jsx | 16 ++-- .../eventModalContent.jsx | 28 +++++- .../app/views/organizationEventsV2/index.jsx | 7 +- .../organizationEventsV2/modalLineGraph.jsx | 86 +++++++++++++++++++ 7 files changed, 168 insertions(+), 15 deletions(-) create mode 100644 src/sentry/static/sentry/app/views/organizationEventsV2/modalLineGraph.jsx diff --git a/src/sentry/static/sentry/app/actionCreators/events.jsx b/src/sentry/static/sentry/app/actionCreators/events.jsx index 643080c02b45..f788a43d7371 100644 --- a/src/sentry/static/sentry/app/actionCreators/events.jsx +++ b/src/sentry/static/sentry/app/actionCreators/events.jsx @@ -31,6 +31,7 @@ export const doEventsRequest = ( limit, query, yAxis, + groupId, } ) => { const shouldDoublePeriod = canIncludePreviousPeriod(includePrevious, period); @@ -40,6 +41,7 @@ export const doEventsRequest = ( environment, query, yAxis, + group: groupId, }; // Doubling period for absolute dates is not accurate unless starting and diff --git a/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx b/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx index 0e5af7be9d57..a6f7b05e6089 100644 --- a/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx +++ b/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx @@ -98,6 +98,11 @@ class EventsRequest extends React.PureComponent { * The yAxis being plotted */ yAxis: PropTypes.string, + + /** + * issue group id or groupids to filter results by. + */ + groupId: PropTypes.string, }; static defaultProps = { @@ -108,6 +113,7 @@ class EventsRequest extends React.PureComponent { limit: 15, getCategory: i => i, query: '', + groupId: '', includePrevious: true, includeTransformedData: true, diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx index 543c809d70db..852e52da4c1f 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx @@ -131,12 +131,42 @@ export const SPECIAL_FIELDS = { ), }, error: { - fields: ['issue_title'], - renderFunc: data => {data.issue_title}, + fields: ['issue_title', 'last_event'], + renderFunc: (data, {organization, location}) => { + const target = { + pathname: `/organizations/${organization.slug}/events/`, + query: { + ...location.query, + eventSlug: `${data['project.name']}:${data.last_event}`, + }, + }; + return ( + + + {data.issue_title} + + + ); + }, }, csp: { - fields: ['issue_title'], - renderFunc: data => {data.issue_title}, + fields: ['issue_title', 'last_event'], + renderFunc: (data, {organization, location}) => { + const target = { + pathname: `/organizations/${organization.slug}/events/`, + query: { + ...location.query, + eventSlug: `${data['project.name']}:${data.last_event}`, + }, + }; + return ( + + + {data.issue_title} + + + ); + }, }, event_count: { title: 'events', diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index 12ad971053dd..b2992cc1cafa 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -2,6 +2,7 @@ import React from 'react'; import styled from 'react-emotion'; import {browserHistory} from 'react-router'; import PropTypes from 'prop-types'; +import SentryTypes from 'app/sentryTypes'; import AsyncComponent from 'app/components/asyncComponent'; import InlineSvg from 'app/components/inlineSvg'; @@ -12,15 +13,17 @@ import EventModalContent from './eventModalContent'; class EventDetails extends AsyncComponent { static propTypes = { - params: PropTypes.object, + organization: SentryTypes.Organization.isRequired, eventSlug: PropTypes.string.isRequired, + location: PropTypes.object.isRequired, + view: PropTypes.object.isRequired, }; getEndpoints() { - const {orgId} = this.props.params; + const {organization} = this.props; const [projectId, eventId] = this.props.eventSlug.toString().split(':'); - return [['event', `/projects/${orgId}/${projectId}/events/${eventId}/`]]; + return [['event', `/projects/${organization.slug}/${projectId}/events/${eventId}/`]]; } onRequestSuccess({data}) { @@ -36,8 +39,9 @@ class EventDetails extends AsyncComponent { handleTabChange = tab => this.setState({activeTab: tab}); renderBody() { - const {orgId} = this.props.params; + const {organization, view, location} = this.props; const [projectId, _] = this.props.eventSlug.split(':'); + return ( @@ -46,7 +50,9 @@ class EventDetails extends AsyncComponent { event={this.state.event} activeTab={this.state.activeTab} projectId={projectId} - orgId={orgId} + organization={organization} + view={view} + location={location} /> ); diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx index 3ae788edb4c3..1e680a10c990 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx @@ -19,6 +19,7 @@ import utils from 'app/utils'; import {getMessage, getTitle} from 'app/utils/events'; import {INTERFACES} from 'app/components/events/eventEntries'; +import ModalLineGraph from './modalLineGraph'; import TagsTable from './tagsTable'; import LinkedIssuePreview from './linkedIssuePreview'; @@ -76,15 +77,29 @@ ActiveTab.propTypes = { * Controlled by the EventDetails View. */ const EventModalContent = props => { - const {event, activeTab, projectId, orgId, onTabChange} = props; - const eventJsonUrl = `/api/0/projects/${orgId}/${projectId}/events/${ + const {event, activeTab, projectId, organization, onTabChange, location, view} = props; + const isGroupedView = !!view.data.groupby; + const eventJsonUrl = `/api/0/projects/${organization.slug}/${projectId}/events/${ event.eventID }/json/`; return ( - + + {isGroupedView && + getDynamicText({ + value: ( + + ), + fixed: 'events chart', + })} + + {event.entries.map(entry => { if (!INTERFACES.hasOwnProperty(entry.type)) { @@ -143,7 +158,9 @@ const EventModalContent = props => { EventModalContent.propTypes = { ...ActiveTab.propTypes, onTabChange: PropTypes.func.isRequired, - orgId: PropTypes.string.isRequired, + organization: SentryTypes.Organization.isRequired, + view: PropTypes.object.isRequired, + location: PropTypes.object.isRequired, }; /** @@ -202,6 +219,9 @@ const ColumnGrid = styled('div')` grid-column-gap: ${space(3)}; `; +const HeaderBox = styled('div')` + grid-column: 1 / 3; +`; const ContentColumn = styled('div')` grid-column: 1 / 2; `; diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx index f191a8d44db6..8ff9b2813653 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx @@ -49,6 +49,7 @@ export default class OrganizationEventsV2 extends React.Component { render() { const {organization, location, router} = this.props; const {eventSlug} = location.query; + const currentView = getCurrentView(location.query.view); return ( @@ -64,16 +65,18 @@ export default class OrganizationEventsV2 extends React.Component { {this.renderTabs()} {eventSlug && ( )} diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/modalLineGraph.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/modalLineGraph.jsx new file mode 100644 index 000000000000..401b6b6b0848 --- /dev/null +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/modalLineGraph.jsx @@ -0,0 +1,86 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import {t} from 'app/locale'; +import SentryTypes from 'app/sentryTypes'; +import {getInterval, useShortInterval} from 'app/components/charts/utils'; +import {getFormattedDate} from 'app/utils/dates'; +import EventsRequest from 'app/views/organizationEvents/utils/eventsRequest'; +import LineChart from 'app/components/charts/lineChart'; +import {Panel} from 'app/components/panels'; +import withApi from 'app/utils/withApi'; +import withGlobalSelection from 'app/utils/withGlobalSelection'; + +const defaultGetCategory = () => t('Events'); + +const ModalLineGraph = props => { + const {api, groupId, organization, location, selection} = props; + + const isUtc = selection.datetime.utc; + const dateFormat = 'lll'; + + const interval = getInterval(selection.datetime, true); + const hasShortInterval = useShortInterval(selection.datetime); + + const xAxisOptions = { + axisLabel: { + formatter: (value, index) => { + const firstItem = index === 0; + const format = hasShortInterval && !firstItem ? 'LT' : dateFormat; + return getFormattedDate(value, format, {local: !isUtc}); + }, + }, + }; + + const tooltip = { + formatAxisLabel: value => { + return getFormattedDate(value, 'lll', {local: !isUtc}); + }, + }; + + return ( + + + {({loading, reloading, timeseriesData}) => ( + + )} + + + ); +}; +ModalLineGraph.propTypes = { + api: PropTypes.object.isRequired, + organization: SentryTypes.Organization.isRequired, + groupId: PropTypes.string.isRequired, + location: PropTypes.object.isRequired, + selection: PropTypes.object.isRequired, +}; + +export default withGlobalSelection(withApi(ModalLineGraph)); From 4a718ee3c02c7d662d360f048b56a1ec4500ab50 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 6 Jun 2019 11:32:08 -0400 Subject: [PATCH 02/13] Split out event details modal integration tests --- tests/js/setup.js | 1 + .../eventDetails.spec.jsx | 173 ++++++++++++++++++ .../views/organizationEventsV2/index.spec.jsx | 72 -------- 3 files changed, 174 insertions(+), 72 deletions(-) create mode 100644 tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx diff --git a/tests/js/setup.js b/tests/js/setup.js index 3ea003af3407..82606d48b5d4 100644 --- a/tests/js/setup.js +++ b/tests/js/setup.js @@ -65,6 +65,7 @@ jest.mock('react-router', () => { Route: ReactRouter.Route, withRouter: ReactRouter.withRouter, browserHistory: { + goBack: jest.fn(), push: jest.fn(), replace: jest.fn(), listen: jest.fn(() => {}), diff --git a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx new file mode 100644 index 000000000000..c8cdfa87f7a4 --- /dev/null +++ b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx @@ -0,0 +1,173 @@ +import React from 'react'; +import {mount} from 'enzyme'; +import {initializeOrg} from 'app-test/helpers/initializeOrg'; +import {browserHistory} from 'react-router'; + +import EventDetails from 'app/views/organizationEventsV2/eventDetails'; +import {ALL_VIEWS} from 'app/views/organizationEventsV2/data'; + +describe('OrganizationEventsV2 > EventDetails', function() { + const allEventsView = ALL_VIEWS.find(view => view.id === 'all'); + const errorsView = ALL_VIEWS.find(view => view.id === 'errors'); + + beforeEach(function() { + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/events/', + body: [ + { + id: 'deadbeef', + title: 'Oh no something bad', + 'project.name': 'project-slug', + timestamp: '2019-05-23T22:12:48+00:00', + }, + ], + }); + MockApiClient.addMockResponse({ + url: '/projects/org-slug/project-slug/events/deadbeef/', + method: 'GET', + body: { + id: '1234', + size: 1200, + eventID: 'deadbeef', + groupID: '123', + title: 'Oh no something bad', + message: 'It was not good', + dateCreated: '2019-05-23T22:12:48+00:00', + entries: [ + { + type: 'message', + message: 'bad stuff', + data: {}, + }, + ], + tags: [{key: 'browser', value: 'Firefox'}], + }, + }); + MockApiClient.addMockResponse({ + url: '/issues/123/', + method: 'GET', + body: TestStubs.Group({id: '123'}), + }); + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/events-stats/', + method: 'GET', + body: { + data: [[1234561700, [1]], [1234561800, [1]]], + }, + }); + }); + + it('renders', function() { + const wrapper = mount( + , + TestStubs.routerContext() + ); + const content = wrapper.find('EventHeader'); + expect(content.text()).toContain('Oh no something bad'); + }); + + it('renders a chart in grouped view', function() { + const wrapper = mount( + , + TestStubs.routerContext() + ); + const graph = wrapper.find('ModalLineGraph'); + expect(graph).toHaveLength(1); + }); + + it('goes back when close button is clicked', function() { + const wrapper = mount( + , + TestStubs.routerContext() + ); + const button = wrapper.find('CloseButton'); + button.simulate('click'); + expect(browserHistory.goBack).toHaveBeenCalled(); + }); + + it('navigates when tag values are clicked', async function() { + const {organization, routerContext} = initializeOrg({ + organization: TestStubs.Organization({projects: [TestStubs.Project()]}), + router: { + location: { + pathname: '/organizations/org-slug/events/', + query: { + eventSlug: 'project-slug:deadbeef', + }, + }, + }, + }); + const wrapper = mount( + , + routerContext + ); + await tick(); + await wrapper.update(); + + // Get the first link as we wrap react-router's link + const tagLink = wrapper.find('EventDetails TagsTable TagValue Link').first(); + + // Should remove eventSlug and append new tag value causing + // the view to re-render + expect(tagLink.props().to).toEqual({ + pathname: '/organizations/org-slug/events/', + query: {query: 'browser:"Firefox"'}, + }); + }); + + it('appends tag value to existing query when clicked', async function() { + const {organization, routerContext} = initializeOrg({ + organization: TestStubs.Organization({projects: [TestStubs.Project()]}), + router: { + location: { + pathname: '/organizations/org-slug/events/', + query: { + query: 'Dumpster', + eventSlug: 'project-slug:deadbeef', + }, + }, + }, + }); + const wrapper = mount( + , + routerContext + ); + await tick(); + await wrapper.update(); + + // Get the first link as we wrap react-router's link + const tagLink = wrapper.find('EventDetails TagsTable TagValue Link').first(); + + // Should remove eventSlug and append new tag value causing + // the view to re-render + expect(tagLink.props().to).toEqual({ + pathname: '/organizations/org-slug/events/', + query: {query: 'Dumpster browser:"Firefox"'}, + }); + }); +}); diff --git a/tests/js/spec/views/organizationEventsV2/index.spec.jsx b/tests/js/spec/views/organizationEventsV2/index.spec.jsx index f1680b672e7e..6fa61b56437f 100644 --- a/tests/js/spec/views/organizationEventsV2/index.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/index.spec.jsx @@ -1,6 +1,5 @@ import React from 'react'; import {mount} from 'enzyme'; -import {initializeOrg} from 'app-test/helpers/initializeOrg'; import OrganizationEventsV2 from 'app/views/organizationEventsV2'; @@ -95,75 +94,4 @@ describe('OrganizationEventsV2', function() { const modal = wrapper.find('EventDetails'); expect(modal).toHaveLength(1); }); - - it('navigates when tag values are clicked', async function() { - const {organization, routerContext} = initializeOrg({ - organization: TestStubs.Organization({projects: [TestStubs.Project()]}), - router: { - location: { - pathname: '/organizations/org-slug/events/', - query: { - eventSlug: 'project-slug:deadbeef', - }, - }, - }, - }); - const wrapper = mount( - , - routerContext - ); - await tick(); - await wrapper.update(); - - // Get the first link as we wrap react-router's link - const tagLink = wrapper.find('EventDetails TagsTable TagValue Link').first(); - - // Should remove eventSlug and append new tag value causing - // the view to re-render - expect(tagLink.props().to).toEqual({ - pathname: '/organizations/org-slug/events/', - query: {query: 'browser:"Firefox"'}, - }); - }); - - it('appends tag value to existing query when clicked', async function() { - const {organization, routerContext} = initializeOrg({ - organization: TestStubs.Organization({projects: [TestStubs.Project()]}), - router: { - location: { - pathname: '/organizations/org-slug/events/', - query: { - query: 'Dumpster', - eventSlug: 'project-slug:deadbeef', - }, - }, - }, - }); - const wrapper = mount( - , - routerContext - ); - await tick(); - await wrapper.update(); - - // Get the first link as we wrap react-router's link - const tagLink = wrapper.find('EventDetails TagsTable TagValue Link').first(); - - // Should remove eventSlug and append new tag value causing - // the view to re-render - expect(tagLink.props().to).toEqual({ - pathname: '/organizations/org-slug/events/', - query: {query: 'Dumpster browser:"Firefox"'}, - }); - }); }); From 6ad7312814c0eee8afaae2602db5df5b7200a627 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 6 Jun 2019 13:54:23 -0400 Subject: [PATCH 03/13] Update proptype and remove redundant property. --- .../sentry/app/views/organizationEvents/utils/eventsRequest.jsx | 2 +- .../sentry/app/views/organizationEventsV2/eventDetails.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx b/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx index a6f7b05e6089..584a6c55fe71 100644 --- a/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx +++ b/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx @@ -102,7 +102,7 @@ class EventsRequest extends React.PureComponent { /** * issue group id or groupids to filter results by. */ - groupId: PropTypes.string, + groupId: PropTypes.oneOfType([PropTypes.string, PropTypes.array]), }; static defaultProps = { diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index b2992cc1cafa..5a9a06efb511 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -40,7 +40,7 @@ class EventDetails extends AsyncComponent { renderBody() { const {organization, view, location} = this.props; - const [projectId, _] = this.props.eventSlug.split(':'); + const [projectId] = this.props.eventSlug.split(':'); return ( From f72a994148a7db37a50bfd768ccf9d4cf5fe5588 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 6 Jun 2019 14:53:53 -0400 Subject: [PATCH 04/13] Fix overflow --- .../app/views/organizationEventsV2/eventModalContent.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx index 1e680a10c990..fd4f09f0c05f 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx @@ -214,9 +214,9 @@ const MetadataContainer = styled('div')` const ColumnGrid = styled('div')` display: grid; - grid-template-columns: 70% 1fr; + grid-template-columns: 70% 28%; grid-template-rows: auto; - grid-column-gap: ${space(3)}; + grid-column-gap: 2%; `; const HeaderBox = styled('div')` From 0a4129e99678342706b9b04374f9c39a9b8fcd7d Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 7 Jun 2019 10:52:37 -0400 Subject: [PATCH 05/13] Use new endpoints in grouped modal Use the new /latest endpoint in the grouped modal view. This pushes the complexity of finding the most recent event into the API where it should be. --- src/sentry/api/urls.py | 2 +- .../app/views/organizationEventsV2/data.jsx | 8 +-- .../organizationEventsV2/eventDetails.jsx | 50 +++++++++++++++---- .../app/views/organizationEventsV2/index.jsx | 6 ++- .../eventDetails.spec.jsx | 36 +++++++++++-- .../views/organizationEventsV2/index.spec.jsx | 2 +- 6 files changed, 84 insertions(+), 20 deletions(-) diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 81f7e62efdf3..baaa1a8f5764 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -607,7 +607,7 @@ name='sentry-api-0-organization-events' ), url( - r'^organizations/(?P[^\/]+)/events/(?P[^\/]+):(?P(?:\d+|[A-Fa-f0-9]{32}))/$', + r'^organizations/(?P[^\/]+)/events/(?P[^\/]+)/(?P(?:\d+|[A-Fa-f0-9]{32}))/$', OrganizationEventDetailsEndpoint.as_view(), name='sentry-api-0-organization-event-details' ), diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx index 852e52da4c1f..e71cf8773379 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx @@ -131,13 +131,13 @@ export const SPECIAL_FIELDS = { ), }, error: { - fields: ['issue_title', 'last_event'], + fields: ['issue_title', 'issue.id'], renderFunc: (data, {organization, location}) => { const target = { pathname: `/organizations/${organization.slug}/events/`, query: { ...location.query, - eventSlug: `${data['project.name']}:${data.last_event}`, + groupId: data['issue.id'], }, }; return ( @@ -150,13 +150,13 @@ export const SPECIAL_FIELDS = { }, }, csp: { - fields: ['issue_title', 'last_event'], + fields: ['issue_title', 'issue.id'], renderFunc: (data, {organization, location}) => { const target = { pathname: `/organizations/${organization.slug}/events/`, query: { ...location.query, - eventSlug: `${data['project.name']}:${data.last_event}`, + groupId: data['issue.id'], }, }; return ( diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index 5a9a06efb511..ea37c832c7c4 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -10,20 +10,48 @@ import withApi from 'app/utils/withApi'; import space from 'app/styles/space'; import EventModalContent from './eventModalContent'; +import {getQuery} from './utils'; class EventDetails extends AsyncComponent { static propTypes = { organization: SentryTypes.Organization.isRequired, - eventSlug: PropTypes.string.isRequired, + eventSlug: PropTypes.string, + groupId: PropTypes.string, location: PropTypes.object.isRequired, view: PropTypes.object.isRequired, }; getEndpoints() { - const {organization} = this.props; - const [projectId, eventId] = this.props.eventSlug.toString().split(':'); - - return [['event', `/projects/${organization.slug}/${projectId}/events/${eventId}/`]]; + const {organization, eventSlug, groupId, view, location} = this.props; + + // If we're getting an issue/group use the latest endpoint. + // We pass the current query/view state to the API so we get an + // event that matches the current list filters. + if (groupId) { + const query = getQuery(view, location); + if (query.query) { + query.query += `issue.id:${groupId}`; + } else { + query.query = `issue.id:${groupId}`; + } + + return [ + [ + 'event', + `/organizations/${organization.slug}/events/latest/`, + { + query, + }, + ], + ]; + } + + // Get a specific event. This could be coming from + // a paginated group or standalone event. + const [projectId, eventId] = eventSlug.toString().split(':'); + return [ + ['event', `/organizations/${organization.slug}/events/${projectId}/${eventId}/`], + ]; } onRequestSuccess({data}) { @@ -39,16 +67,20 @@ class EventDetails extends AsyncComponent { handleTabChange = tab => this.setState({activeTab: tab}); renderBody() { - const {organization, view, location} = this.props; - const [projectId] = this.props.eventSlug.split(':'); + const {organization, view, location, eventSlug} = this.props; + const {event, activeTab} = this.state; + let projectId = event.projectSlug; + if (!projectId && eventSlug) { + [projectId] = eventSlug.split(':'); + } return ( @@ -70,11 +71,12 @@ export default class OrganizationEventsV2 extends React.Component { router={router} /> - {eventSlug && ( + {showModal && ( diff --git a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx index c8cdfa87f7a4..8714b6032257 100644 --- a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx @@ -23,11 +23,12 @@ describe('OrganizationEventsV2 > EventDetails', function() { ], }); MockApiClient.addMockResponse({ - url: '/projects/org-slug/project-slug/events/deadbeef/', + url: '/organizations/org-slug/events/project-slug/deadbeef/', method: 'GET', body: { id: '1234', size: 1200, + projectSlug: 'org-slug', eventID: 'deadbeef', groupID: '123', title: 'Oh no something bad', @@ -55,6 +56,29 @@ describe('OrganizationEventsV2 > EventDetails', function() { data: [[1234561700, [1]], [1234561800, [1]]], }, }); + + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/events/latest/', + method: 'GET', + body: { + id: '1234', + size: 1200, + projectSlug: 'org-slug', + eventID: 'deadbeef', + groupID: '123', + title: 'Oh no something bad', + message: 'It was not good', + dateCreated: '2019-05-23T22:12:48+00:00', + entries: [ + { + type: 'message', + message: 'bad stuff', + data: {}, + }, + ], + tags: [{key: 'browser', value: 'Firefox'}], + }, + }); }); it('renders', function() { @@ -69,18 +93,24 @@ describe('OrganizationEventsV2 > EventDetails', function() { ); const content = wrapper.find('EventHeader'); expect(content.text()).toContain('Oh no something bad'); + + const graph = wrapper.find('ModalLineGraph'); + expect(graph).toHaveLength(0); }); it('renders a chart in grouped view', function() { const wrapper = mount( , TestStubs.routerContext() ); + const content = wrapper.find('EventHeader'); + expect(content.text()).toContain('Oh no something bad'); + const graph = wrapper.find('ModalLineGraph'); expect(graph).toHaveLength(1); }); diff --git a/tests/js/spec/views/organizationEventsV2/index.spec.jsx b/tests/js/spec/views/organizationEventsV2/index.spec.jsx index 6fa61b56437f..0506dfd75d40 100644 --- a/tests/js/spec/views/organizationEventsV2/index.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/index.spec.jsx @@ -17,7 +17,7 @@ describe('OrganizationEventsV2', function() { ], }); MockApiClient.addMockResponse({ - url: '/projects/org-slug/project-slug/events/deadbeef/', + url: '/organizations/org-slug/events/project-slug/deadbeef/', method: 'GET', body: { id: '1234', From fc48f323569d39ec22a0384f62322adad3d37345 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 7 Jun 2019 14:11:59 -0400 Subject: [PATCH 06/13] Fix NaN displaying --- .../static/sentry/app/views/organizationEventsV2/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx index 007c5cfad91d..49908df66aa4 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx @@ -50,7 +50,7 @@ export default class OrganizationEventsV2 extends React.Component { const {organization, location, router} = this.props; const {eventSlug, groupId} = location.query; const currentView = getCurrentView(location.query.view); - const showModal = groupId + eventSlug; + const showModal = groupId || eventSlug; return ( From c2776f350e1147d502eb5f0255446357dfd72032 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 7 Jun 2019 16:25:56 -0400 Subject: [PATCH 07/13] Add better proptype validator. --- .../app/views/organizationEvents/utils/eventsRequest.jsx | 2 +- .../app/views/organizationEventsV2/eventDetails.jsx | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx b/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx index 584a6c55fe71..7bade2ffa895 100644 --- a/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx +++ b/src/sentry/static/sentry/app/views/organizationEvents/utils/eventsRequest.jsx @@ -109,11 +109,11 @@ class EventsRequest extends React.PureComponent { period: null, start: null, end: null, + groupId: null, interval: '1d', limit: 15, getCategory: i => i, query: '', - groupId: '', includePrevious: true, includeTransformedData: true, diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index ea37c832c7c4..09968b7c902a 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -15,7 +15,13 @@ import {getQuery} from './utils'; class EventDetails extends AsyncComponent { static propTypes = { organization: SentryTypes.Organization.isRequired, - eventSlug: PropTypes.string, + eventSlug: function(props, propName, componentName) { + const value = props[propName]; + if (value && !/^[^:]+:[a-f0-9]+$/.test(value)) { + return new Error(`Invalid value for ${propName} provided to ${componentName}.`); + } + return null; + }, groupId: PropTypes.string, location: PropTypes.object.isRequired, view: PropTypes.object.isRequired, From ae1bfe7c0df13aa4926636d5bca2e1475b1379c7 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 10 Jun 2019 15:13:41 -0400 Subject: [PATCH 08/13] Revert changes to projectSlug. Lyn prefers to keep the URL parameter `:` separated. --- src/sentry/api/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index baaa1a8f5764..81f7e62efdf3 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -607,7 +607,7 @@ name='sentry-api-0-organization-events' ), url( - r'^organizations/(?P[^\/]+)/events/(?P[^\/]+)/(?P(?:\d+|[A-Fa-f0-9]{32}))/$', + r'^organizations/(?P[^\/]+)/events/(?P[^\/]+):(?P(?:\d+|[A-Fa-f0-9]{32}))/$', OrganizationEventDetailsEndpoint.as_view(), name='sentry-api-0-organization-event-details' ), From 8218c01f4137a8e7de387b5054c13715f894d8d2 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 10 Jun 2019 15:14:25 -0400 Subject: [PATCH 09/13] Switch to groupSlug with the latest/oldest/eventid By creating a combined query parameter we have fewer state bits we need to manage together. Instead we can focus on the valid/invalid value states. --- .../app/views/organizationEventsV2/data.jsx | 8 +-- .../organizationEventsV2/eventDetails.jsx | 71 +++++++++++-------- .../app/views/organizationEventsV2/index.jsx | 6 +- .../eventDetails.spec.jsx | 6 +- .../views/organizationEventsV2/index.spec.jsx | 18 ++++- 5 files changed, 70 insertions(+), 39 deletions(-) diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx index e71cf8773379..32857d61aac0 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx @@ -131,13 +131,13 @@ export const SPECIAL_FIELDS = { ), }, error: { - fields: ['issue_title', 'issue.id'], + fields: ['issue_title', 'project.name', 'issue.id'], renderFunc: (data, {organization, location}) => { const target = { pathname: `/organizations/${organization.slug}/events/`, query: { ...location.query, - groupId: data['issue.id'], + groupSlug: `${data['project.name']}:${data['issue.id']}:latest`, }, }; return ( @@ -150,13 +150,13 @@ export const SPECIAL_FIELDS = { }, }, csp: { - fields: ['issue_title', 'issue.id'], + fields: ['issue_title', 'project.name', 'issue.id'], renderFunc: (data, {organization, location}) => { const target = { pathname: `/organizations/${organization.slug}/events/`, query: { ...location.query, - groupId: data['issue.id'], + groupSlug: `${data['project.name']}:${data['issue.id']}:latest`, }, }; return ( diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index 09968b7c902a..72ab435328b7 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -12,51 +12,58 @@ import space from 'app/styles/space'; import EventModalContent from './eventModalContent'; import {getQuery} from './utils'; +const slugValidator = function(props, propName, componentName) { + const value = props[propName]; + if (value && !/^(?:[^:]+:)?(?:[^:]+):(?:[a-f0-9]+|latest|oldest)$/.test(value)) { + return new Error(`Invalid value for ${propName} provided to ${componentName}.`); + } + return null; +}; + class EventDetails extends AsyncComponent { static propTypes = { organization: SentryTypes.Organization.isRequired, - eventSlug: function(props, propName, componentName) { - const value = props[propName]; - if (value && !/^[^:]+:[a-f0-9]+$/.test(value)) { - return new Error(`Invalid value for ${propName} provided to ${componentName}.`); - } - return null; - }, - groupId: PropTypes.string, + eventSlug: slugValidator, + groupSlug: slugValidator, location: PropTypes.object.isRequired, view: PropTypes.object.isRequired, }; getEndpoints() { - const {organization, eventSlug, groupId, view, location} = this.props; + const {organization, eventSlug, groupSlug, view, location} = this.props; + const query = getQuery(view, location); // If we're getting an issue/group use the latest endpoint. // We pass the current query/view state to the API so we get an // event that matches the current list filters. - if (groupId) { - const query = getQuery(view, location); + if (groupSlug) { + const [projectId, groupId, eventId] = groupSlug.toString().split(':'); + + let url = `/organizations/${organization.slug}/events/`; + // latest / oldest have dedicated endpoints + if (['latest', 'oldest'].includes(eventId)) { + url += 'latest/'; + } else { + url += `${projectId}:${eventId}/`; + } if (query.query) { - query.query += `issue.id:${groupId}`; + query.query += ` issue.id:${groupId}`; } else { query.query = `issue.id:${groupId}`; } - return [ - [ - 'event', - `/organizations/${organization.slug}/events/latest/`, - { - query, - }, - ], - ]; + return [['event', url, {query}]]; } // Get a specific event. This could be coming from // a paginated group or standalone event. const [projectId, eventId] = eventSlug.toString().split(':'); return [ - ['event', `/organizations/${organization.slug}/events/${projectId}/${eventId}/`], + [ + 'event', + `/organizations/${organization.slug}/events/${projectId}:${eventId}/`, + {query}, + ], ]; } @@ -72,13 +79,21 @@ class EventDetails extends AsyncComponent { handleTabChange = tab => this.setState({activeTab: tab}); + get projectId() { + if (this.props.eventSlug) { + const [projectId] = this.props.eventSlug.split(':'); + return projectId; + } + if (this.props.groupSlug) { + const [projectId] = this.props.groupSlug.split(':'); + return projectId; + } + throw new Error('Could not determine projectId'); + } + renderBody() { - const {organization, view, location, eventSlug} = this.props; + const {organization, view, location} = this.props; const {event, activeTab} = this.state; - let projectId = event.projectSlug; - if (!projectId && eventSlug) { - [projectId] = eventSlug.split(':'); - } return ( @@ -87,7 +102,7 @@ class EventDetails extends AsyncComponent { onTabChange={this.handleTabChange} event={event} activeTab={activeTab} - projectId={projectId} + projectId={this.projectId} organization={organization} view={view} location={location} diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx index 49908df66aa4..2ee973adf791 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx @@ -48,9 +48,9 @@ export default class OrganizationEventsV2 extends React.Component { render() { const {organization, location, router} = this.props; - const {eventSlug, groupId} = location.query; + const {eventSlug, groupSlug} = location.query; const currentView = getCurrentView(location.query.view); - const showModal = groupId || eventSlug; + const showModal = groupSlug || eventSlug; return ( @@ -76,7 +76,7 @@ export default class OrganizationEventsV2 extends React.Component { organization={organization} params={this.props.params} eventSlug={eventSlug} - groupId={groupId} + groupSlug={groupSlug} view={currentView} location={location} /> diff --git a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx index 8714b6032257..0bfc9988ff07 100644 --- a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx @@ -23,7 +23,7 @@ describe('OrganizationEventsV2 > EventDetails', function() { ], }); MockApiClient.addMockResponse({ - url: '/organizations/org-slug/events/project-slug/deadbeef/', + url: '/organizations/org-slug/events/project-slug:deadbeef/', method: 'GET', body: { id: '1234', @@ -102,8 +102,8 @@ describe('OrganizationEventsV2 > EventDetails', function() { const wrapper = mount( , TestStubs.routerContext() diff --git a/tests/js/spec/views/organizationEventsV2/index.spec.jsx b/tests/js/spec/views/organizationEventsV2/index.spec.jsx index 0506dfd75d40..6c0cdc390d71 100644 --- a/tests/js/spec/views/organizationEventsV2/index.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/index.spec.jsx @@ -17,7 +17,7 @@ describe('OrganizationEventsV2', function() { ], }); MockApiClient.addMockResponse({ - url: '/organizations/org-slug/events/project-slug/deadbeef/', + url: '/organizations/org-slug/events/project-slug:deadbeef/', method: 'GET', body: { id: '1234', @@ -94,4 +94,20 @@ describe('OrganizationEventsV2', function() { const modal = wrapper.find('EventDetails'); expect(modal).toHaveLength(1); }); + + it('opens a modal when groupSlug is present', async function() { + const organization = TestStubs.Organization({projects: [TestStubs.Project()]}); + const wrapper = mount( + , + TestStubs.routerContext() + ); + + const modal = wrapper.find('EventDetails'); + expect(modal).toHaveLength(1); + }); }); From 57b5b71ff01a70fda87e4b0db05d6e749ad2af6d Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 7 Jun 2019 16:45:09 -0400 Subject: [PATCH 10/13] feat(events-v2) Add pagination controls to event modal In grouped views we want to show pagination buttons to navigate between the events that match the current search filters including the query string. By creating URLs that use eventSlug and the current search criteria we'll end up at the right events. Refs SEN-715 --- .../static/sentry/app/icons/icon-next.svg | 1 + .../static/sentry/app/icons/icon-prev.svg | 1 + .../organizationEventsV2/eventDetails.jsx | 12 ++- .../eventModalContent.jsx | 2 + .../organizationEventsV2/modalPagination.jsx | 97 +++++++++++++++++++ .../eventDetails.spec.jsx | 26 ++++- 6 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 src/sentry/static/sentry/app/icons/icon-next.svg create mode 100644 src/sentry/static/sentry/app/icons/icon-prev.svg create mode 100644 src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx diff --git a/src/sentry/static/sentry/app/icons/icon-next.svg b/src/sentry/static/sentry/app/icons/icon-next.svg new file mode 100644 index 000000000000..878c589ca201 --- /dev/null +++ b/src/sentry/static/sentry/app/icons/icon-next.svg @@ -0,0 +1 @@ + diff --git a/src/sentry/static/sentry/app/icons/icon-prev.svg b/src/sentry/static/sentry/app/icons/icon-prev.svg new file mode 100644 index 000000000000..5fe53a4bd2c3 --- /dev/null +++ b/src/sentry/static/sentry/app/icons/icon-prev.svg @@ -0,0 +1 @@ + diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index 72ab435328b7..49ade88113bb 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -2,8 +2,9 @@ import React from 'react'; import styled from 'react-emotion'; import {browserHistory} from 'react-router'; import PropTypes from 'prop-types'; -import SentryTypes from 'app/sentryTypes'; +import {omit} from 'lodash'; +import SentryTypes from 'app/sentryTypes'; import AsyncComponent from 'app/components/asyncComponent'; import InlineSvg from 'app/components/inlineSvg'; import withApi from 'app/utils/withApi'; @@ -74,7 +75,14 @@ class EventDetails extends AsyncComponent { handleClose = event => { event.preventDefault(); - browserHistory.goBack(); + const {location} = this.props; + // Remove modal related query parameters. + const query = omit(location.query, ['groupSlug', 'eventSlug']); + + browserHistory.push({ + pathname: location.pathname, + query, + }); }; handleTabChange = tab => this.setState({activeTab: tab}); diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx index fd4f09f0c05f..90f7f86fb21e 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx @@ -19,6 +19,7 @@ import utils from 'app/utils'; import {getMessage, getTitle} from 'app/utils/events'; import {INTERFACES} from 'app/components/events/eventEntries'; +import ModalPagination from './modalPagination'; import ModalLineGraph from './modalLineGraph'; import TagsTable from './tagsTable'; import LinkedIssuePreview from './linkedIssuePreview'; @@ -87,6 +88,7 @@ const EventModalContent = props => { + {isGroupedView && } {isGroupedView && getDynamicText({ value: ( diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx new file mode 100644 index 000000000000..e00f634dcf1b --- /dev/null +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx @@ -0,0 +1,97 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import styled from 'react-emotion'; +import isPropValid from '@emotion/is-prop-valid'; +import {omit} from 'lodash'; + +import {t} from 'app/locale'; +import Link from 'app/components/links/link'; +import SentryTypes from 'app/sentryTypes'; +import InlineSvg from 'app/components/inlineSvg'; +import space from 'app/styles/space'; + +const ModalPagination = props => { + const {location, event} = props; + + // Remove the groupId and eventSlug keys as we need to create new ones + const query = omit(location.query, ['groupSlug', 'eventSlug']); + const previousEventUrl = event.previousEventID + ? { + pathname: location.pathname, + query: { + ...query, + groupSlug: `${event.projectSlug}:${event.groupID}:${event.previousEventID}`, + }, + } + : null; + const nextEventUrl = event.nextEventID + ? { + pathname: location.pathname, + query: { + ...query, + groupSlug: `${event.projectSlug}:${event.groupID}:${event.nextEventID}`, + }, + } + : null; + const newestUrl = { + pathname: location.pathname, + query: { + ...query, + groupSlug: `${event.projectSlug}:${event.groupID}:latest`, + }, + }; + const oldestUrl = { + pathname: location.pathname, + query: { + ...query, + groupSlug: `${event.projectSlug}:${event.groupID}:oldest`, + }, + }; + + return ( + + + + + + + {t('Older Event')} + + + {t('Newer Event')} + + + + + + + ); +}; +ModalPagination.propTypes = { + location: PropTypes.object.isRequired, + event: SentryTypes.Event.isRequired, +}; + +const StyledLink = styled(Link, {shouldForwardProp: isPropValid})` + color: ${p => (p.disabled ? p.theme.disabled : p.theme.gray3)}; + font-size: ${p => p.fontSizeMedium}; + padding: ${space(0.5)} ${space(1.5)}; + ${p => (p.isLast ? '' : `border-right: 1px solid ${p.theme.borderDark};`)} + ${p => (p.disabled ? 'pointer-events: none;' : '')} +`; + +const Wrapper = styled('div')` + display: flex; +`; + +const Container = styled('div')` + display: flex; + background: ${p => p.theme.offWhite}; + border: 1px solid ${p => p.theme.borderDark}; + border-radius: ${p => p.theme.borderRadius}; + margin-bottom: ${space(3)}; + box-shadow: 3px 3px 0 ${p => p.theme.offWhite}, 3px 3px 0 1px ${p => p.theme.borderDark}, + 7px 7px ${p => p.theme.offWhite}, 7px 7px 0 1px ${p => p.theme.borderDark}; +`; + +export default ModalPagination; diff --git a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx index 0bfc9988ff07..85cc32f534e8 100644 --- a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx @@ -115,19 +115,39 @@ describe('OrganizationEventsV2 > EventDetails', function() { expect(graph).toHaveLength(1); }); - it('goes back when close button is clicked', function() { + it('renders pagination buttons in grouped view', function() { + const wrapper = mount( + , + TestStubs.routerContext() + ); + const content = wrapper.find('ModalPagination'); + expect(content).toHaveLength(1); + }); + + it('changes history when close button is clicked', function() { const wrapper = mount( , TestStubs.routerContext() ); const button = wrapper.find('CloseButton'); button.simulate('click'); - expect(browserHistory.goBack).toHaveBeenCalled(); + expect(browserHistory.push).toHaveBeenCalledWith({ + pathname: '/organizations/org-slug/events/', + query: {}, + }); }); it('navigates when tag values are clicked', async function() { From 36f6b865d8b06f19eb9b0de2b5920a17b81f770a Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 10 Jun 2019 15:53:33 -0400 Subject: [PATCH 11/13] Add another test --- .../eventDetails.spec.jsx | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx index 85cc32f534e8..a87b6db1d810 100644 --- a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx @@ -129,7 +129,7 @@ describe('OrganizationEventsV2 > EventDetails', function() { expect(content).toHaveLength(1); }); - it('changes history when close button is clicked', function() { + it('removes eventSlug when close button is clicked', function() { const wrapper = mount( EventDetails', function() { }); }); + it('removes groupSlug when close button is clicked', function() { + const wrapper = mount( + , + TestStubs.routerContext() + ); + const button = wrapper.find('CloseButton'); + button.simulate('click'); + expect(browserHistory.push).toHaveBeenCalledWith({ + pathname: '/organizations/org-slug/events/', + query: {}, + }); + }); + it('navigates when tag values are clicked', async function() { const {organization, routerContext} = initializeOrg({ organization: TestStubs.Organization({projects: [TestStubs.Project()]}), From 4a8f02727279ae95d0f086eb2fc6dcd65b8c310c Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 11 Jun 2019 13:57:45 -0400 Subject: [PATCH 12/13] Update src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx Co-Authored-By: Billy Vong --- .../sentry/app/views/organizationEventsV2/modalPagination.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx index e00f634dcf1b..7a38fe198aaa 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx @@ -13,7 +13,7 @@ import space from 'app/styles/space'; const ModalPagination = props => { const {location, event} = props; - // Remove the groupId and eventSlug keys as we need to create new ones + // Remove the groupSlug and eventSlug keys as we need to create new ones const query = omit(location.query, ['groupSlug', 'eventSlug']); const previousEventUrl = event.previousEventID ? { From 3ac70ae4ebfc46ba43366962b6bc2b8a845e5b3e Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 11 Jun 2019 15:04:19 -0400 Subject: [PATCH 13/13] Apply feedback from review --- .../app/views/organizationEventsV2/eventDetails.jsx | 5 +++++ .../app/views/organizationEventsV2/modalPagination.jsx | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index 49ade88113bb..20093a16941c 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -15,6 +15,11 @@ import {getQuery} from './utils'; const slugValidator = function(props, propName, componentName) { const value = props[propName]; + // Accept slugs that look like: + // * project-slug:123:latest + // * project-slug:123:oldest + // * project-slug:123:deadbeef + // * project-slug:deadbeef if (value && !/^(?:[^:]+:)?(?:[^:]+):(?:[a-f0-9]+|latest|oldest)$/.test(value)) { return new Error(`Invalid value for ${propName} provided to ${componentName}.`); } diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx index 7a38fe198aaa..77971b120856 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx @@ -50,8 +50,8 @@ const ModalPagination = props => { return ( - - + + @@ -60,10 +60,10 @@ const ModalPagination = props => { {t('Newer Event')} - + - + ); }; @@ -84,7 +84,7 @@ const Wrapper = styled('div')` display: flex; `; -const Container = styled('div')` +const ShadowBox = styled('div')` display: flex; background: ${p => p.theme.offWhite}; border: 1px solid ${p => p.theme.borderDark};