From eed1df3082606fa7c73aaefc3adaea6460fe74c9 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 2 Aug 2019 12:14:13 -0400 Subject: [PATCH] feat(eventsv2) Remove transactionSlug and groupSlug The upcoming work to adopt discover style queries means that we can no longer have special slugs. We can also use `argMax()` in snuba to get to the latest event when viewing an aggregated view. This removes the special slug value handling, and relies entirely on eventSlug. Discover2 will require that all modal states need to be reachable from just the eventSlug and query state. This change will break pagination in the short term but I'll be working on new endpoints that will allow that behavior to be restored in the short term. Refs SEN-875 --- src/sentry/api/bases/organization_events.py | 5 + .../app/views/organizationEventsV2/data.jsx | 14 +-- .../organizationEventsV2/eventDetails.jsx | 108 ++++-------------- .../app/views/organizationEventsV2/index.jsx | 7 +- .../organizationEventsV2/modalPagination.jsx | 34 +++--- .../eventDetails.spec.jsx | 95 +-------------- .../views/organizationEventsV2/index.spec.jsx | 16 --- 7 files changed, 53 insertions(+), 226 deletions(-) diff --git a/src/sentry/api/bases/organization_events.py b/src/sentry/api/bases/organization_events.py index a7346366c149..0f54759c9106 100644 --- a/src/sentry/api/bases/organization_events.py +++ b/src/sentry/api/bases/organization_events.py @@ -26,6 +26,11 @@ 'user_count': { 'aggregations': [['uniq', 'user', 'user_count']], }, + 'latest_event': { + 'fields': [ + ['argMax', ['id', 'timestamp'], 'latest_event'], + ], + }, } diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx index 027b4c895e42..75162bb704d3 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx @@ -15,7 +15,7 @@ import space from 'app/styles/space'; import {QueryLink} from './styles'; -export const MODAL_QUERY_KEYS = ['eventSlug', 'groupSlug', 'transactionSlug']; +export const MODAL_QUERY_KEYS = ['eventSlug']; export const PIN_ICON = `image://${pinIcon}`; export const ALL_VIEWS = deepFreeze([ @@ -94,14 +94,14 @@ export const ALL_VIEWS = deepFreeze([ */ export const SPECIAL_FIELDS = { transaction: { - fields: ['project.name', 'transaction'], + fields: ['project.name', 'transaction', 'latest_event'], sortField: 'transaction', renderFunc: (data, {organization, location}) => { const target = { pathname: `/organizations/${organization.slug}/events/`, query: { ...location.query, - transactionSlug: `${data['project.name']}:${data.transaction}:latest`, + eventSlug: `${data['project.name']}:${data.latest_event}`, }, }; return ( @@ -207,14 +207,14 @@ export const SPECIAL_FIELDS = { ), }, error: { - fields: ['issue_title', 'project.name', 'issue.id'], + fields: ['issue_title', 'project.name', 'latest_event'], sortField: 'issue_title', renderFunc: (data, {organization, location}) => { const target = { pathname: `/organizations/${organization.slug}/events/`, query: { ...location.query, - groupSlug: `${data['project.name']}:${data['issue.id']}:latest`, + eventSlug: `${data['project.name']}:${data.latest_event}`, }, }; return ( @@ -227,14 +227,14 @@ export const SPECIAL_FIELDS = { }, }, csp: { - fields: ['issue_title', 'project.name', 'issue.id'], + fields: ['issue_title', 'project.name', 'latest_event'], sortField: 'issue_title', renderFunc: (data, {organization, location}) => { const target = { pathname: `/organizations/${organization.slug}/events/`, query: { ...location.query, - groupSlug: `${data['project.name']}:${data['issue.id']}:latest`, + eventSlug: `${data['project.name']}:${data.latest_event}`, }, }; return ( diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx index b17625d0f9c1..1e5ad379772b 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx @@ -17,23 +17,10 @@ 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:latest + // * project-slug:deadbeef:oldest // * project-slug:deadbeef - if (value && !/^(?:[^:]+:)?(?:[^:]+):(?:[a-f0-9]+|latest|oldest)$/.test(value)) { - return new Error(`Invalid value for ${propName} provided to ${componentName}.`); - } - return null; -}; - -const transactionValidator = function(props, propName, componentName) { - const value = props[propName]; - // Accept slugs that look like: - // * project-slug:/some/pathname:latest - // * project-slug:a_bare_string:oldest - // * project-slug:/some/otherpath:deadbeef - if (value && !/^(?:[^:]+):(?:[^:]+):(?:[^:]+|latest|oldest)$/.test(value)) { + if (value && !/^(?:[^:]+):(?:[a-f0-9]+)(?:\:latest|oldest)?$/.test(value)) { return new Error(`Invalid value for ${propName} provided to ${componentName}.`); } return null; @@ -56,86 +43,36 @@ class EventDetails extends AsyncComponent { static propTypes = { organization: SentryTypes.Organization.isRequired, eventSlug: slugValidator, - groupSlug: slugValidator, - transactionSlug: transactionValidator, location: PropTypes.object.isRequired, view: PropTypes.object.isRequired, }; getEndpoints() { - const { - organization, - eventSlug, - groupSlug, - transactionSlug, - view, - location, - } = this.props; + const {organization, eventSlug, 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 (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 += `${eventId}/`; - } else { - url += `${projectId}:${eventId}/`; - } - if (query.query) { - query.query += ` issue.id:${groupId}`; - } else { - query.query = `issue.id:${groupId}`; - } - - return [['event', url, {query}]]; - } - - // If we're looking at a transaction aggregate we need to do a search - // by transaction to find the latest event for the transaction. - if (transactionSlug) { - const [projectId, transactionName, eventId] = transactionSlug.toString().split(':'); - - let url = `/organizations/${organization.slug}/events/`; - // latest / oldest have dedicated endpoints - if (['latest', 'oldest'].includes(eventId)) { - url += `${eventId}/`; - } else { - url += `${projectId}:${eventId}/`; - } - if (query.query) { - query.query += ` transaction:${transactionName}`; - } else { - query.query = `transaction:${transactionName}`; - } - - return [['event', url, {query}]]; + // Check the eventid for the latest/oldest keyword and use that to choose + // the endpoint as oldest/latest have special endpoints. + const [projectId, eventId, keyword] = eventSlug.toString().split(':'); + + let url = `/organizations/${organization.slug}/events/`; + // TODO the latest/oldest links are currently broken as they require a + // new endpoint that works with the upcoming discover2 queries. + if (['latest', 'oldest'].includes(keyword)) { + url += `${keyword}/`; + } else { + url += `${projectId}:${eventId}/`; } - if (eventSlug) { - // 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}/`, - {query}, - ], - ]; - } - - throw new Error('No valid datasource property found.'); + // Get a specific event. This could be coming from + // a paginated group or standalone event. + return [['event', url, {query}]]; } onDismiss = () => { const {location} = this.props; // Remove modal related query parameters. - const query = omit(location.query, ['transactionSlug', 'groupSlug', 'eventSlug']); + const query = omit(location.query, ['eventSlug']); browserHistory.push({ pathname: location.pathname, @@ -144,12 +81,7 @@ class EventDetails extends AsyncComponent { }; get projectId() { - const source = - this.props.eventSlug || this.props.groupSlug || this.props.transactionSlug; - if (!source) { - throw new Error('Could not determine projectId'); - } - return source.split(':')[0]; + return this.props.eventSlug.split(':')[0]; } renderBody() { diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx index 4e6e0427459a..367785fafb3c 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx @@ -53,9 +53,8 @@ export default class OrganizationEventsV2 extends React.Component { render() { const {organization, location, router} = this.props; - const {eventSlug, groupSlug, transactionSlug} = location.query; + const {eventSlug} = location.query; const currentView = getCurrentView(location.query.view); - const showModal = transactionSlug || groupSlug || eventSlug; return ( @@ -76,13 +75,11 @@ export default class OrganizationEventsV2 extends React.Component { router={router} /> - {showModal && ( + {eventSlug && ( diff --git a/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx index 5db934e0f636..842be378e745 100644 --- a/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx +++ b/src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx @@ -15,23 +15,17 @@ import {MODAL_QUERY_KEYS} from './data'; /** * Generate a mapping of link names => link targets for pagination */ -function buildTargets(event, location, view) { +function buildTargets(event, location) { // Remove slug related keys as we need to create new ones const baseQuery = omit(location.query, MODAL_QUERY_KEYS); - let queryKey, aggregateValue; - if (view.id === 'transactions') { - queryKey = 'transactionSlug'; - aggregateValue = event.location; - } else { - queryKey = 'groupSlug'; - aggregateValue = event.groupID; - } const urlMap = { previous: event.previousEventID, next: event.nextEventID, - latest: 'latest', - oldest: 'oldest', + // TODO(mark) Make latest, oldest work once we have new endpoints. + // `${event.eventID}:latest`, + latest: null, + oldest: null, }; const links = {}; @@ -44,7 +38,7 @@ function buildTargets(event, location, view) { pathname: location.pathname, query: { ...baseQuery, - [queryKey]: `${event.projectSlug}:${aggregateValue}:${value}`, + eventSlug: `${event.projectSlug}:${value}`, }, }; } @@ -54,13 +48,16 @@ function buildTargets(event, location, view) { } const ModalPagination = props => { - const {event, location, view} = props; - const links = buildTargets(event, location, view); + const {event, location} = props; + const links = buildTargets(event, location); return ( - + { > {t('Newer Event')} - + @@ -87,7 +88,6 @@ const ModalPagination = props => { ModalPagination.propTypes = { location: PropTypes.object.isRequired, event: SentryTypes.Event.isRequired, - view: PropTypes.object.isRequired, }; const StyledLink = styled(Link, {shouldForwardProp: isPropValid})` diff --git a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx index 7124356f1f39..c9fa5ba92288 100644 --- a/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/eventDetails.spec.jsx @@ -9,7 +9,6 @@ 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'); - const transactionView = ALL_VIEWS.find(view => view.id === 'transactions'); beforeEach(function() { MockApiClient.addMockResponse({ @@ -153,8 +152,8 @@ describe('OrganizationEventsV2 > EventDetails', function() { const wrapper = mount( , TestStubs.routerContext() @@ -166,54 +165,6 @@ describe('OrganizationEventsV2 > EventDetails', function() { expect(graph).toHaveLength(1); }); - it('renders pagination buttons in grouped view', function() { - const wrapper = mount( - , - TestStubs.routerContext() - ); - - const content = wrapper.find('ModalPagination'); - expect(content).toHaveLength(1); - - const prevLink = content.find('StyledLink[data-test-id="older-event"]').first(); - const target = prevLink.props().to; - expect(target.query.groupSlug).toEqual('project-slug:123:beefbeef'); - expect(target.query.transactionSlug).toBeUndefined(); - expect(target.query.eventSlug).toBeUndefined(); - - const nextLink = content.find('StyledLink[data-test-id="newer-event"]').first(); - expect(nextLink.props().to).toBeNull(); - }); - - it('renders pagination buttons in transaction view', function() { - const wrapper = mount( - , - TestStubs.routerContext() - ); - - const content = wrapper.find('ModalPagination'); - expect(content).toHaveLength(1); - - const prevLink = content.find('StyledLink[data-test-id="older-event"]').first(); - const target = prevLink.props().to; - expect(target.query.transactionSlug).toEqual('project-slug:/users/login:beefbeef'); - expect(target.query.groupSlug).toBeUndefined(); - expect(target.query.eventSlug).toBeUndefined(); - - const nextLink = content.find('StyledLink[data-test-id="newer-event"]').first(); - expect(nextLink.props().to).toBeNull(); - }); - 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('DismissButton'); - button.simulate('click'); - expect(browserHistory.push).toHaveBeenCalledWith({ - pathname: '/organizations/org-slug/events/', - query: {}, - }); - }); - - it('removes transactionSlug when close button is clicked', function() { - const wrapper = mount( - , - TestStubs.routerContext() - ); - const button = wrapper.find('DismissButton'); - 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()]}), diff --git a/tests/js/spec/views/organizationEventsV2/index.spec.jsx b/tests/js/spec/views/organizationEventsV2/index.spec.jsx index 173b75d6c996..1c594939d55e 100644 --- a/tests/js/spec/views/organizationEventsV2/index.spec.jsx +++ b/tests/js/spec/views/organizationEventsV2/index.spec.jsx @@ -128,20 +128,4 @@ 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); - }); });