Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Version 8.11 (Unreleased)
- Cleaner install UI when creating a new project.
- Added support for recording symbols separately in frames independent of the function.
- Reduce noisy Postgres logs from inserting duplicate onboarding rows.
- Added export action to group tag details.
- Improved display of user tags.

Schema Changes
~~~~~~~~~~~~~~
Expand Down
8 changes: 7 additions & 1 deletion src/sentry/api/endpoints/group_tagkey_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.api.paginator import DateTimePaginator, OffsetPaginator, Paginator
from sentry.api.serializers import serialize
from sentry.api.serializers.models.tagvalue import UserTagValueSerializer
from sentry.models import GroupTagValue, TagKey, TagKeyStatus, Group
from sentry.utils.apidocs import scenario

Expand Down Expand Up @@ -68,10 +69,15 @@ def get(self, request, group, key):
order_by = '-id'
paginator_cls = Paginator

if key == 'user':
serializer_cls = UserTagValueSerializer()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but I had to look up the API for this since the name didn't match what this actually was and couldn't tell if it was meant to be an instance of a class or a class itself with the variable being named _cls.

Copy link
Member Author

Choose a reason for hiding this comment

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

its the class of serializer, not the literal class type

else:
serializer_cls = None

return self.paginate(
request=request,
queryset=queryset,
order_by=order_by,
paginator_cls=paginator_cls,
on_results=lambda x: serialize(x, request.user),
on_results=lambda x: serialize(x, request.user, serializer_cls),
)
1 change: 1 addition & 0 deletions src/sentry/api/serializers/models/eventuser.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class EventUserSerializer(Serializer):
def serialize(self, obj, attrs, user):
return {
'id': six.text_type(obj.id),
'tagValue': obj.tag_value,
'identifier': obj.ident,
'username': obj.username,
'email': obj.email,
Expand Down
61 changes: 39 additions & 22 deletions src/sentry/api/serializers/models/tagvalue.py
Original file line number Diff line number Diff line change
@@ -1,41 +1,28 @@
from __future__ import absolute_import

import operator
import six

from django.db.models import Q
from six.moves import reduce

from sentry.api.serializers import Serializer, register
from sentry.api.serializers import Serializer, register, serialize
from sentry.models import EventUser, TagKey, TagValue


def parse_user_tag(value):
lookup, value = value.split(':', 1)
if lookup == 'id':
lookup = 'ident'
elif lookup == 'ip':
lookup = 'ip_address'
return {lookup: value}


@register(TagValue)
class TagValueSerializer(Serializer):
def get_attrs(self, item_list, user):
user_lookups = [
Q(**parse_user_tag(i.value))
user_tags = [
i.value
for i in item_list
if i.key == 'sentry:user'
]

tag_labels = {}
if user_lookups:
if user_tags:
tag_labels.update({
('sentry:user', euser.tag_value): euser.get_label()
for euser in EventUser.objects.filter(
reduce(operator.or_, user_lookups),
project=item_list[0].project,
)
('sentry:user', k): v.get_label()
for k, v in six.iteritems(EventUser.for_tags(
project_id=item_list[0].project_id,
values=user_tags,
))
})

result = {}
Expand Down Expand Up @@ -67,3 +54,33 @@ def serialize(self, obj, attrs, user):
'id': six.text_type(obj.id),
'name': obj.value,
}


class UserTagValueSerializer(Serializer):
def get_attrs(self, item_list, user):
users = EventUser.for_tags(
project_id=item_list[0].project_id,
values=[t.value for t in item_list],
)

result = {}
for item in item_list:
result[item] = {
'user': users.get(item.value),
}
return result

def serialize(self, obj, attrs, user):
if not attrs['user']:
result = {
'id': None,
}
else:
result = serialize(attrs['user'], user)
result.update({
'value': obj.value,
'count': obj.times_seen,
'lastSeen': obj.last_seen,
'firstSeen': obj.first_seen,
})
return result
19 changes: 19 additions & 0 deletions src/sentry/models/eventuser.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,25 @@ class Meta:
def attr_from_keyword(cls, keyword):
return KEYWORD_MAP[keyword]

@classmethod
def for_tags(cls, project_id, values):
"""
Finds matching EventUser objects from a list of tag values.

Return a dictionary of {tag_value: event_user}.
"""
hashes = [
md5_text(v.split(':', 1)[-1]).hexdigest()
for v in values
]
return {
e.tag_value: e
for e in cls.objects.filter(
project=project_id,
hash__in=hashes,
)
}

def save(self, *args, **kwargs):
assert self.ident or self.username or self.email or self.ip_address, \
'No identifying value found for user'
Expand Down
28 changes: 23 additions & 5 deletions src/sentry/static/sentry/app/components/events/contexts/user.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/*eslint react/jsx-key:0*/
import React from 'react';
import _ from 'underscore';

Expand All @@ -15,10 +16,18 @@ const UserContextType = React.createClass({
let children = [];

// Handle our native attributes special
user.id && builtins.push(['ID', user.id]);
user.email && builtins.push(['Email', user.email]);
user.username && builtins.push(['Username', user.username]);
user.ip_address && builtins.push(['IP Address', user.ip_address]);
user.id && builtins.push(['ID', <pre>{user.id}</pre>]);
user.email && builtins.push([
'Email',
<pre>
{user.email}
<a href={`mailto:${user.email}`} className="external-icon">
<em className="icon-envelope" />
</a>
</pre>
]);
user.username && builtins.push(['Username', <pre>{user.username}</pre>]);
user.ip_address && builtins.push(['IP Address', <pre>{user.ip_address}</pre>]);

// We also attach user supplied data as 'user.data'
_.each(user.data, function(value, key) {
Expand All @@ -30,7 +39,16 @@ const UserContextType = React.createClass({
<div className="pull-left">
<Avatar user={user} size={96} gravatar={false} />
</div>
<KeyValueList data={builtins} isContextData={false} />
<table className="key-value table">
Copy link
Contributor

Choose a reason for hiding this comment

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

So, I see why you removed KeyValueList – because it wouldn't render your custom Email JSX above properly.

I feel a better solution is to modify KeyValueList so that if value is a React component, it should just spit out that component as-is (whereas primitives go through the existing path through deviceNameMapper).

Copy link
Member Author

Choose a reason for hiding this comment

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

that sounds like a lot of work

Copy link
Contributor

Choose a reason for hiding this comment

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

I can do it in a follow-up.

{builtins.map(([key, value]) => {
return (
<tr key={key}>
<td className="key" key="0">{key}</td>
<td className="value" key="1">{value}</td>
</tr>
);
})}
</table>
{children &&
<KeyValueList data={children} isContextData={true} />
}
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/static/sentry/app/views/groupDetails/header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ const GroupHeader = React.createClass({
<div className="count align-right">
<h6 className="nav-header">{t('Users')}</h6>
{userCount !== 0 ?
<Link to={`/${orgId}/${projectId}/issues/${groupId}/tags/user/`}>
<Link to={`/${orgId}/${projectId}/issues/${groupId}/users/`}>
<Count className="count" value={userCount} />
</Link>
:
Expand Down
50 changes: 28 additions & 22 deletions src/sentry/static/sentry/app/views/groupTagValues.jsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
/*eslint react/jsx-key:0*/
import React from 'react';
import {Link} from 'react-router';
import jQuery from 'jquery';
import ApiMixin from '../mixins/apiMixin';
import Count from '../components/count';
import GroupState from '../mixins/groupState';
import Avatar from '../components/avatar';
import LoadingError from '../components/loadingError';
import LoadingIndicator from '../components/loadingIndicator';
import Pagination from '../components/pagination';
import TimeSince from '../components/timeSince';
import {isUrl, percent, deviceNameMapper} from '../utils';
import {t, tn} from '../locale';
import {t} from '../locale';

const GroupTagValues = React.createClass({
mixins: [
ApiMixin,
GroupState
ApiMixin
],

getInitialState() {
Expand Down Expand Up @@ -47,7 +46,7 @@ const GroupTagValues = React.createClass({
error: false
});

this.api.request('/issues/' + this.getGroup().id + '/tags/' + params.tagKey + '/', {
this.api.request(`/issues/${params.groupId}/tags/${params.tagKey}/`, {
success: (data) => {
this.setState({
tagKey: data,
Expand All @@ -62,7 +61,7 @@ const GroupTagValues = React.createClass({
}
});

this.api.request('/issues/' + this.getGroup().id + '/tags/' + params.tagKey + '/values/?' + querystring, {
this.api.request(`/issues/${params.groupId}/tags/${params.tagKey}/values/?${querystring}`, {
success: (data, _, jqXHR) => {
this.setState({
tagValueList: data,
Expand All @@ -79,18 +78,21 @@ const GroupTagValues = React.createClass({
});
},

getUserDisplayName(item) {
return item.email || item.username || item.identifier || item.ipAddress || item.value;
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just:

return item.email || item.username || item.identifier || item.ipAddress || item.value;


render() {
if (this.state.loading) {
return <LoadingIndicator />;
} else if (this.state.error) {
return <LoadingError onRetry={this.fetchData} />;
}

let {orgId, projectId} = this.props.params;
let tagKey = this.state.tagKey;
let children = this.state.tagValueList.map((tagValue, tagValueIdx) => {
let pct = percent(tagValue.count, tagKey.totalValues).toFixed(2);
let orgId = this.getOrganization().slug;
let projectId = this.getProject().slug;
return (
<tr key={tagValueIdx}>
<td className="bar-cell">
Expand All @@ -101,10 +103,20 @@ const GroupTagValues = React.createClass({
<Link
to={{
pathname: `/${orgId}/${projectId}/`,
query: {query: tagKey.key + ':' + '"' + tagValue.value + '"'}
query: {query: `${tagKey.key}:"${tagValue.value}"`}
}}>
{deviceNameMapper(tagValue.name)}
{tagKey.key === 'user' ? [
<Avatar user={tagValue} size={16} className="avatar" />,
<span style={{marginLeft: 10}}>{this.getUserDisplayName(tagValue)}</span>
] :
deviceNameMapper(tagValue.name)
}
</Link>
{tagValue.email &&
<a href={`mailto:${tagValue.email}`} className="external-icon">
<em className="icon-envelope" />
</a>
}
{isUrl(tagValue.value) &&
<a href={tagValue.value} className="external-icon">
<em className="icon-open" />
Expand All @@ -121,22 +133,15 @@ const GroupTagValues = React.createClass({
return (
<div>
<h3>
{tagKey.name + ' '}
<small>{tn(
'%2$d unique historical value',
'%2$d unique historical values',
tagKey.uniqueValues,
<Count value={tagKey.uniqueValues} />
)}</small>
{tagKey.key == 'user' ? t('Affected Users') : tagKey.name}
<a href="export/" className="btn btn-default btn-sm"
style={{marginLeft: 10}}>{t('Export to CSV')}</a>
</h3>
<div className="alert alert-info alert-block">
{t('Data is based on events seen in the last 7 days.')}
</div>
<table className="table table-striped">
<thead>
<tr>
<th style={{width: 30}}>%</th>
<th>{t('Value')}</th>
<th></th>
<th style={{width: 200}}>{t('Last Seen')}</th>
</tr>
</thead>
Expand All @@ -145,6 +150,7 @@ const GroupTagValues = React.createClass({
</tbody>
</table>
<Pagination pageLinks={this.state.pageLinks}/>
<p><small>{t('Note: Percentage of issue is based on events seen in the last 7 days.')}</small></p>
</div>
);
}
Expand Down
Loading