Skip to content

feat(eventsv2) Remove transactionSlug and groupSlug #14257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 6, 2019
Merged

feat(eventsv2) Remove transactionSlug and groupSlug #14257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/sentry/api/bases/organization_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@
'user_count': {
'aggregations': [['uniq', 'user', 'user_count']],
},
'latest_event': {
'fields': [
['argMax', ['id', 'timestamp'], 'latest_event'],
],
},
Copy link
Member

@dashed dashed Aug 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory Heads up, the docs above the SPECIAL_FIELDS variable may need to be updated.

Copy link
Member Author

@markstory markstory Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is not long for this world. I'll get this updated in my next set of changes.

Copy link
Member

@dashed dashed Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💀

}


Expand Down
14 changes: 7 additions & 7 deletions src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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 (
Expand All @@ -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 (
Expand Down
108 changes: 20 additions & 88 deletions src/sentry/static/sentry/app/views/organizationEventsV2/eventDetails.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Member

@lynnagara lynnagara Aug 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the project slug part of that right? Can we have : in project slug? Is every other character there really valid? If you wanted to, could also enforce that event ID has a length of 32

Copy link
Member Author

@markstory markstory Aug 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Project slugs can't have : in them. The slug generation done in project.save() removes the : from the slug.

Copy link
Member

@dashed dashed Aug 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory to double check, are the colons are removed on the django side?

Copy link
Member Author

@markstory markstory Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still one : left between the projectid and eventid. We need to retain that one as eventids are only unique within a project.

return new Error(`Invalid value for ${propName} provided to ${componentName}.`);
}
return null;
Expand All @@ -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,
Expand All @@ -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() {
Expand Down
7 changes: 2 additions & 5 deletions src/sentry/static/sentry/app/views/organizationEventsV2/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<DocumentTitle title={`Events - ${organization.slug} - Sentry`}>
Expand All @@ -76,13 +75,11 @@ export default class OrganizationEventsV2 extends React.Component {
router={router}
/>
</NoProjectMessage>
{showModal && (
{eventSlug && (
<EventDetails
organization={organization}
params={this.props.params}
eventSlug={eventSlug}
groupSlug={groupSlug}
transactionSlug={transactionSlug}
view={currentView}
location={location}
/>
Expand Down
34 changes: 17 additions & 17 deletions src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand All @@ -44,7 +38,7 @@ function buildTargets(event, location, view) {
pathname: location.pathname,
query: {
...baseQuery,
[queryKey]: `${event.projectSlug}:${aggregateValue}:${value}`,
eventSlug: `${event.projectSlug}:${value}`,
},
};
}
Expand All @@ -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 (
<Wrapper>
<ShadowBox>
<StyledLink to={links.oldest} disabled={links.previous === null}>
<StyledLink
to={links.oldest}
disabled={links.previous === null || links.oldest === null}
>
<InlineSvg src="icon-prev" size="14px" />
</StyledLink>
<StyledLink
Expand All @@ -77,7 +74,11 @@ const ModalPagination = props => {
>
{t('Newer Event')}
</StyledLink>
<StyledLink to={links.latest} disabled={links.next === null} isLast>
<StyledLink
to={links.latest}
disabled={links.next === null || links.latest === null}
isLast
>
<InlineSvg src="icon-next" size="14px" />
</StyledLink>
</ShadowBox>
Expand All @@ -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})`
Expand Down
Loading