From 2ad172db78ee48d6c1525b771f5e99cb9acca12c Mon Sep 17 00:00:00 2001 From: David Cramer Date: Tue, 22 Nov 2016 12:50:08 -0800 Subject: [PATCH] [tags] Improve handling of values - Add export to csv action in UI - Remove uniqueValues reference (not issue specific) - Augment data when user tag is selected - Add email action for users --- CHANGES | 2 + .../api/endpoints/group_tagkey_values.py | 8 +- .../api/serializers/models/eventuser.py | 1 + src/sentry/api/serializers/models/tagvalue.py | 61 +++++---- src/sentry/models/eventuser.py | 19 +++ .../app/components/events/contexts/user.jsx | 28 +++- .../sentry/app/views/groupDetails/header.jsx | 2 +- .../sentry/app/views/groupTagValues.jsx | 50 ++++---- src/sentry/web/frontend/group_tag_export.py | 121 +++++++++++------- src/sentry/web/frontend/mixins/__init__.py | 1 + src/sentry/web/frontend/mixins/csv.py | 48 +++++++ .../api/endpoints/test_group_tagkey_values.py | 40 +++++- 12 files changed, 283 insertions(+), 98 deletions(-) create mode 100644 src/sentry/web/frontend/mixins/__init__.py create mode 100644 src/sentry/web/frontend/mixins/csv.py diff --git a/CHANGES b/CHANGES index 5aed5b1e7435c9..8a8355f419c5c0 100644 --- a/CHANGES +++ b/CHANGES @@ -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 ~~~~~~~~~~~~~~ diff --git a/src/sentry/api/endpoints/group_tagkey_values.py b/src/sentry/api/endpoints/group_tagkey_values.py index 6f778381975428..39c378485352b9 100644 --- a/src/sentry/api/endpoints/group_tagkey_values.py +++ b/src/sentry/api/endpoints/group_tagkey_values.py @@ -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 @@ -68,10 +69,15 @@ def get(self, request, group, key): order_by = '-id' paginator_cls = Paginator + if key == 'user': + serializer_cls = UserTagValueSerializer() + 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), ) diff --git a/src/sentry/api/serializers/models/eventuser.py b/src/sentry/api/serializers/models/eventuser.py index b98f8e2748c9c4..318016181acd83 100644 --- a/src/sentry/api/serializers/models/eventuser.py +++ b/src/sentry/api/serializers/models/eventuser.py @@ -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, diff --git a/src/sentry/api/serializers/models/tagvalue.py b/src/sentry/api/serializers/models/tagvalue.py index f98295f7b86d5e..b64e87020b858f 100644 --- a/src/sentry/api/serializers/models/tagvalue.py +++ b/src/sentry/api/serializers/models/tagvalue.py @@ -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 = {} @@ -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 diff --git a/src/sentry/models/eventuser.py b/src/sentry/models/eventuser.py index 394a5ce64362f6..47afbc54dbbf61 100644 --- a/src/sentry/models/eventuser.py +++ b/src/sentry/models/eventuser.py @@ -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' diff --git a/src/sentry/static/sentry/app/components/events/contexts/user.jsx b/src/sentry/static/sentry/app/components/events/contexts/user.jsx index 1546afea743f1c..4bd8c3dc038511 100644 --- a/src/sentry/static/sentry/app/components/events/contexts/user.jsx +++ b/src/sentry/static/sentry/app/components/events/contexts/user.jsx @@ -1,3 +1,4 @@ +/*eslint react/jsx-key:0*/ import React from 'react'; import _ from 'underscore'; @@ -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', 
{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}
]); // We also attach user supplied data as 'user.data' _.each(user.data, function(value, key) { @@ -30,7 +39,16 @@ const UserContextType = React.createClass({
- + + {builtins.map(([key, value]) => { + return ( + + + + + ); + })} +
{key}{value}
{children && } diff --git a/src/sentry/static/sentry/app/views/groupDetails/header.jsx b/src/sentry/static/sentry/app/views/groupDetails/header.jsx index dafea59c323aa9..0d04b347a94e72 100644 --- a/src/sentry/static/sentry/app/views/groupDetails/header.jsx +++ b/src/sentry/static/sentry/app/views/groupDetails/header.jsx @@ -162,7 +162,7 @@ const GroupHeader = React.createClass({
{t('Users')}
{userCount !== 0 ? - + : diff --git a/src/sentry/static/sentry/app/views/groupTagValues.jsx b/src/sentry/static/sentry/app/views/groupTagValues.jsx index 28b297fe2287ba..e31cb195d52a21 100644 --- a/src/sentry/static/sentry/app/views/groupTagValues.jsx +++ b/src/sentry/static/sentry/app/views/groupTagValues.jsx @@ -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() { @@ -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, @@ -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, @@ -79,6 +78,10 @@ const GroupTagValues = React.createClass({ }); }, + getUserDisplayName(item) { + return item.email || item.username || item.identifier || item.ipAddress || item.value; + }, + render() { if (this.state.loading) { return ; @@ -86,11 +89,10 @@ const GroupTagValues = React.createClass({ return ; } + 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 ( @@ -101,10 +103,20 @@ const GroupTagValues = React.createClass({ - {deviceNameMapper(tagValue.name)} + {tagKey.key === 'user' ? [ + , + {this.getUserDisplayName(tagValue)} + ] : + deviceNameMapper(tagValue.name) + } + {tagValue.email && + + + + } {isUrl(tagValue.value) && @@ -121,22 +133,15 @@ const GroupTagValues = React.createClass({ return (

- {tagKey.name + ' '} - {tn( - '%2$d unique historical value', - '%2$d unique historical values', - tagKey.uniqueValues, - - )} + {tagKey.key == 'user' ? t('Affected Users') : tagKey.name} + {t('Export to CSV')}

-
- {t('Data is based on events seen in the last 7 days.')} -
- + @@ -145,6 +150,7 @@ const GroupTagValues = React.createClass({
%{t('Value')} {t('Last Seen')}
+

{t('Note: Percentage of issue is based on events seen in the last 7 days.')}

); } diff --git a/src/sentry/web/frontend/group_tag_export.py b/src/sentry/web/frontend/group_tag_export.py index 551c4e3cd660fa..66cb41c0b3aa5c 100644 --- a/src/sentry/web/frontend/group_tag_export.py +++ b/src/sentry/web/frontend/group_tag_export.py @@ -1,46 +1,76 @@ from __future__ import absolute_import -import csv -import six - -from django.http import Http404, StreamingHttpResponse -from django.utils.text import slugify +from django.http import Http404 from sentry.models import ( - GroupTagValue, TagKey, TagKeyStatus, Group, get_group_with_redirect + EventUser, GroupTagValue, TagKey, TagKeyStatus, Group, get_group_with_redirect ) from sentry.web.frontend.base import ProjectView +from sentry.web.frontend.mixins.csv import CsvMixin +from sentry.utils.query import RangeQuerySetWrapper + + +def attach_eventuser(project_id): + def wrapped(items): + users = EventUser.for_tags(project_id, [i.value for i in items]) + for item in items: + item._eventuser = users.get(item.value) + return wrapped + + +class GroupTagExportView(ProjectView, CsvMixin): + required_scope = 'event:read' + def get_header(self, key): + if key == 'user': + return self.get_user_header() + return self.get_generic_header() -# Python 2 doesn't support unicode with CSV, but Python 3 does via -# the encoding param -if six.PY3: - def get_row(row): + def get_row(self, item, key): + if key == 'user': + return self.get_user_row(item) + return self.get_generic_row(item) + + def get_generic_header(self): return ( - row.value, - six.text_type(row.times_seen), - row.last_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), - row.first_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + 'value', + 'times_seen', + 'last_seen', + 'first_seen', ) -else: - def get_row(row): + + def get_generic_row(self, item): return ( - row.value.encode('utf-8'), - six.text_type(row.times_seen), - row.last_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), - row.first_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + item.value, + item.times_seen, + item.last_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + item.first_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), ) + def get_user_header(self): + return ( + 'value', + 'id', + 'email', + 'username', + 'ip_address', + 'times_seen', + 'last_seen', + 'first_seen', + ) -# csv.writer doesn't provide a non-file interface -# https://docs.djangoproject.com/en/1.9/howto/outputting-csv/#streaming-large-csv-files -class Echo(object): - def write(self, value): - return value - - -class GroupTagExportView(ProjectView): - required_scope = 'event:read' + def get_user_row(self, item): + euser = item._eventuser + return ( + item.value, + euser.ident if euser else '', + euser.email if euser else '', + euser.username if euser else '', + euser.ip_address if euser else '', + item.times_seen, + item.last_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + item.first_seen.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + ) def get(self, request, organization, project, team, group_id, key): try: @@ -68,23 +98,22 @@ def get(self, request, organization, project, team, group_id, key): except TagKey.DoesNotExist: raise Http404 - queryset = GroupTagValue.objects.filter( - group=group, - key=lookup_key, - ) - - def row_iter(): - yield ('value', 'times_seen', 'last_seen', 'first_seen') - for row in queryset.iterator(): - yield get_row(row) + if key == 'user': + callbacks = [attach_eventuser(project.id)] + else: + callbacks = [] - pseudo_buffer = Echo() - writer = csv.writer(pseudo_buffer) - response = StreamingHttpResponse( - (writer.writerow(r) for r in row_iter()), - content_type='text/csv', + queryset = RangeQuerySetWrapper( + GroupTagValue.objects.filter( + group=group, + key=lookup_key, + ), + callbacks=callbacks, ) - response['Content-Disposition'] = 'attachment; filename="{}-{}.csv"'.format( - group.qualified_short_id or group.id, slugify(key) + + filename = '{}-{}'.format( + group.qualified_short_id or group.id, + key, ) - return response + + return self.to_csv_response(queryset, filename, key=key) diff --git a/src/sentry/web/frontend/mixins/__init__.py b/src/sentry/web/frontend/mixins/__init__.py new file mode 100644 index 00000000000000..c3961685ab8def --- /dev/null +++ b/src/sentry/web/frontend/mixins/__init__.py @@ -0,0 +1 @@ +from __future__ import absolute_import diff --git a/src/sentry/web/frontend/mixins/csv.py b/src/sentry/web/frontend/mixins/csv.py new file mode 100644 index 00000000000000..6f9e7b692ceffb --- /dev/null +++ b/src/sentry/web/frontend/mixins/csv.py @@ -0,0 +1,48 @@ +from __future__ import absolute_import + +import csv +import six + +from django.http import StreamingHttpResponse + +# Python 2 doesn't support unicode with CSV, but Python 3 does via +# the encoding param +if six.PY3: + def encode_row(row): + return row +else: + def encode_row(row): + return [six.text_type(e).encode('utf-8') for e in row] + + +# csv.writer doesn't provide a non-file interface +# https://docs.djangoproject.com/en/1.9/howto/outputting-csv/#streaming-large-csv-files +class Echo(object): + def write(self, value): + return value + + +class CsvMixin(object): + def get_header(self, **kwargs): + return () + + def get_row(self, item, **kwargs): + return () + + def to_csv_response(self, iterable, filename, **kwargs): + def row_iter(): + header = self.get_header(**kwargs) + if header: + yield header + for item in iterable: + yield self.get_row(item, **kwargs) + + pseudo_buffer = Echo() + writer = csv.writer(pseudo_buffer) + response = StreamingHttpResponse( + (writer.writerow(r) for r in row_iter()), + content_type='text/csv', + ) + response['Content-Disposition'] = \ + 'attachment; filename="{}.csv"'.format(filename) + return response diff --git a/tests/sentry/api/endpoints/test_group_tagkey_values.py b/tests/sentry/api/endpoints/test_group_tagkey_values.py index 63c31acca00259..a070800f05000e 100644 --- a/tests/sentry/api/endpoints/test_group_tagkey_values.py +++ b/tests/sentry/api/endpoints/test_group_tagkey_values.py @@ -1,6 +1,6 @@ from __future__ import absolute_import -from sentry.models import GroupTagValue, TagKey, TagValue +from sentry.models import EventUser, GroupTagValue, TagKey, TagValue from sentry.testutils import APITestCase @@ -33,3 +33,41 @@ def test_simple(self): assert len(response.data) == 1 assert response.data[0]['value'] == 'bar' + + def test_user_tag(self): + project = self.create_project() + group = self.create_group(project=project) + euser = EventUser.objects.create( + project_id=project.id, + ident='1', + email='foo@example.com', + username='foo', + ip_address='127.0.0.1', + ) + TagKey.objects.create( + project=project, + key='sentry:user', + ) + TagValue.objects.create( + project=project, + key='sentry:user', + value=euser.tag_value, + ) + GroupTagValue.objects.create( + project=project, + group=group, + key='sentry:user', + value=euser.tag_value, + ) + + self.login_as(user=self.user) + + url = '/api/0/issues/{}/tags/user/values/'.format(group.id) + + response = self.client.get(url) + + assert response.status_code == 200 + assert len(response.data) == 1 + + assert response.data[0]['email'] == 'foo@example.com' + assert response.data[0]['value'] == euser.tag_value