From 90c8201eb92cdd2b1e3a41f9d52ba5145ce0ebde Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Thu, 28 Mar 2019 12:31:36 +0100 Subject: [PATCH] Add reviewer name alias for reviewers to hide their actual name It's used when exposing reviewer name to developers for reviewer actions - regular actions still use the normal name, even when made by a reviewer. --- docs/topics/api/activity.rst | 11 +++-- src/olympia/accounts/serializers.py | 23 ++++++++-- .../accounts/tests/test_serializers.py | 30 ++++++++++-- src/olympia/activity/models.py | 11 +++++ src/olympia/activity/serializers.py | 16 ++++++- .../activity/tests/test_serializers.py | 26 +++++------ src/olympia/activity/tests/test_utils.py | 46 ++++++++++--------- src/olympia/activity/utils.py | 4 +- .../templates/devhub/addons/activity.html | 3 +- .../templates/devhub/versions/list.html | 4 +- src/olympia/devhub/tests/test_views.py | 30 +++++++++++- .../migrations/1082-add-reviewer-name.sql | 1 + src/olympia/reviewers/utils.py | 2 +- src/olympia/users/admin.py | 4 +- src/olympia/users/models.py | 4 ++ static/js/zamboni/devhub.js | 7 ++- 16 files changed, 161 insertions(+), 61 deletions(-) create mode 100644 src/olympia/migrations/1082-add-reviewer-name.sql diff --git a/docs/topics/api/activity.rst b/docs/topics/api/activity.rst index beece0eb06..9697194f32 100644 --- a/docs/topics/api/activity.rst +++ b/docs/topics/api/activity.rst @@ -39,6 +39,11 @@ Review Notes Detail This endpoint allows you to fetch a single review note for a specific version of an add-on. + .. note:: + To allow reviewers to stay anonymous if they wish, the ``user`` object ``name`` can point to + their "reviewer" name depending on the action. In addition all other fields in that object, + despite being present for backwards-compatibility, are set to ``null``. + .. http:get:: /api/v4/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/reviewnotes/(int:id)/ .. _review-notes-version-detail-object: @@ -46,10 +51,10 @@ This endpoint allows you to fetch a single review note for a specific version of :>json int id: The id for a review note. :>json string action: The :ref:`type of review note`. :>json string action_label: The text label of the action. - :>json int user.id: The id of the reviewer or author who left the review note. + :>json int|null user.id: The id of the reviewer or author who left the review note. :>json string user.name: The name of the reviewer or author. - :>json string user.url: The link to the profile page for of the reviewer or author. - :>json string user.username: The username of the reviewer or author. + :>json string|null user.url: The link to the profile page for of the reviewer or author. + :>json string|null user.username: The username of the reviewer or author. :>json string comments: The text content of the review note. :>json string date: The date the review note was created. diff --git a/src/olympia/accounts/serializers.py b/src/olympia/accounts/serializers.py index bfd966afe0..e0a5357a82 100644 --- a/src/olympia/accounts/serializers.py +++ b/src/olympia/accounts/serializers.py @@ -78,19 +78,28 @@ class UserProfileSerializer(PublicUserProfileSerializer): picture_upload = serializers.ImageField(use_url=True, write_only=True) permissions = serializers.SerializerMethodField() fxa_edit_email_url = serializers.SerializerMethodField() + reviewer_name = serializers.CharField( + min_length=2, max_length=50, + validators=[OneOrMorePrintableCharacterAPIValidator()]) class Meta(PublicUserProfileSerializer.Meta): fields = PublicUserProfileSerializer.Meta.fields + ( - 'display_name', 'email', 'deleted', 'last_login', 'picture_upload', - 'last_login_ip', 'read_dev_agreement', 'permissions', - 'fxa_edit_email_url', 'username', + 'deleted', 'display_name', 'email', 'fxa_edit_email_url', + 'last_login', 'last_login_ip', 'permissions', 'picture_upload', + 'read_dev_agreement', 'reviewer_name', 'username' ) writeable_fields = ( 'biography', 'display_name', 'homepage', 'location', 'occupation', - 'picture_upload', + 'picture_upload', 'reviewer_name', ) read_only_fields = tuple(set(fields) - set(writeable_fields)) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if (not self.instance or + not acl.is_user_any_kind_of_reviewer(self.instance)): + self.fields.pop('reviewer_name', None) + def get_fxa_edit_email_url(self, user): base_url = '{}/settings'.format( settings.FXA_CONFIG['default']['content_host'] @@ -111,6 +120,12 @@ class UserProfileSerializer(PublicUserProfileSerializer): ugettext(u'This display name cannot be used.')) return value + def validate_reviewer_name(self, value): + if DeniedName.blocked(value): + raise serializers.ValidationError( + ugettext(u'This reviewer name cannot be used.')) + return value + def validate_homepage(self, value): if settings.DOMAIN.lower() in value.lower(): raise serializers.ValidationError( diff --git a/src/olympia/accounts/tests/test_serializers.py b/src/olympia/accounts/tests/test_serializers.py index aa4e5f5e19..02574ef44d 100644 --- a/src/olympia/accounts/tests/test_serializers.py +++ b/src/olympia/accounts/tests/test_serializers.py @@ -28,7 +28,8 @@ class TestBaseUserSerializer(TestCase): def serialize(self): # Manually reload the user first to clear any cached properties. self.user = UserProfile.objects.get(pk=self.user.pk) - serializer = self.serializer_class(context={'request': self.request}) + serializer = self.serializer_class( + self.user, context={'request': self.request}) return serializer.to_representation(self.user) def test_basic(self): @@ -67,14 +68,19 @@ class TestPublicUserProfileSerializer(TestCase): user_kwargs = { 'username': 'amo', 'biography': 'stuff', 'homepage': 'http://mozilla.org/', - 'location': 'everywhere', 'occupation': 'job'} + 'location': 'everywhere', 'occupation': 'job', + } + user_private_kwargs = { + 'reviewer_name': 'batman', + } def setUp(self): self.request = APIRequestFactory().get('/') - self.user = user_factory(**self.user_kwargs) + self.user = user_factory( + **self.user_kwargs, **self.user_private_kwargs) def serialize(self): - return (self.serializer(context={'request': self.request}) + return (self.serializer(self.user, context={'request': self.request}) .to_representation(self.user)) def test_picture(self): @@ -92,6 +98,8 @@ class TestPublicUserProfileSerializer(TestCase): data = self.serialize() for prop, val in self.user_kwargs.items(): assert data[prop] == six.text_type(val), prop + for prop, val in self.user_private_kwargs.items(): + assert prop not in data return data def test_addons(self): @@ -142,6 +150,11 @@ class TestPublicUserProfileSerializer(TestCase): assert data['has_anonymous_username'] is False assert data['has_anonymous_display_name'] is False + def test_is_reviewer(self): + self.grant_permission(self.user, 'Addons:PostReview') + # private data should still be absent, this is a public serializer + self.test_basic() + class PermissionsTestMixin(object): def test_permissions(self): @@ -194,6 +207,15 @@ class TestUserProfileSerializer(TestPublicUserProfileSerializer, self.now.replace(microsecond=0).isoformat() + 'Z') assert data['read_dev_agreement'] == data['last_login'] + def test_is_reviewer(self): + self.grant_permission(self.user, 'Addons:PostReview') + data = self.serialize() + for prop, val in self.user_kwargs.items(): + assert data[prop] == six.text_type(val), prop + # We can also see private stuff, it's the same user. + for prop, val in self.user_private_kwargs.items(): + assert data[prop] == six.text_type(val), prop + def test_expose_fxa_edit_email_url(self): fxa_host = 'http://example.com' fxa_config = { diff --git a/src/olympia/activity/models.py b/src/olympia/activity/models.py index f15d2d6619..bbfac0cf9d 100644 --- a/src/olympia/activity/models.py +++ b/src/olympia/activity/models.py @@ -479,6 +479,17 @@ class ActivityLog(ModelBase): def __html__(self): return self + @property + def author_name(self): + """Name of the user that triggered the activity. + + If it's a reviewer action that will be shown to developers, the + `reviewer_name` property is used if present, otherwise `name` is + used.""" + if self.action in constants.activity.LOG_REVIEW_QUEUE_DEVELOPER: + return self.user.reviewer_name or self.user.name + return self.user.name + @classmethod def create(cls, action, *args, **kw): """ diff --git a/src/olympia/activity/serializers.py b/src/olympia/activity/serializers.py index 5014a1762f..7f88288595 100644 --- a/src/olympia/activity/serializers.py +++ b/src/olympia/activity/serializers.py @@ -2,7 +2,6 @@ from django.utils.translation import ugettext from rest_framework import serializers -from olympia.accounts.serializers import BaseUserSerializer from olympia.activity.models import ActivityLog @@ -11,7 +10,7 @@ class ActivityLogSerializer(serializers.ModelSerializer): action_label = serializers.SerializerMethodField() comments = serializers.SerializerMethodField() date = serializers.DateTimeField(source='created') - user = BaseUserSerializer() + user = serializers.SerializerMethodField() highlight = serializers.SerializerMethodField() class Meta: @@ -37,3 +36,16 @@ class ActivityLogSerializer(serializers.ModelSerializer): def get_highlight(self, obj): return obj in self.to_highlight + + def get_user(self, obj): + """Return minimal user information using ActivityLog.author_name to + avoid revealing actual name of reviewers for their review actions if + they have set an alias. + + id, username and url are present for backwards-compatibility only.""" + return { + 'id': None, + 'username': None, + 'url': None, + 'name': obj.author_name, + } diff --git a/src/olympia/activity/tests/test_serializers.py b/src/olympia/activity/tests/test_serializers.py index 637a811c3e..b2df27da52 100644 --- a/src/olympia/activity/tests/test_serializers.py +++ b/src/olympia/activity/tests/test_serializers.py @@ -4,7 +4,6 @@ from rest_framework.test import APIRequestFactory from olympia import amo from olympia.activity.models import ActivityLog from olympia.activity.serializers import ActivityLogSerializer -from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.amo.tests import TestCase, addon_factory, user_factory @@ -24,16 +23,16 @@ class LogMixin(object): class TestReviewNotesSerializerOutput(TestCase, LogMixin): def setUp(self): self.request = APIRequestFactory().get('/') - self.user = user_factory() + self.user = user_factory(reviewer_name='fôo') self.addon = addon_factory() self.now = self.days_ago(0) - self.entry = self.log(u'Oh nôes!', amo.LOG.REJECT_VERSION, self.now) + self.entry = self.log(u'Oh nøes!', amo.LOG.REJECT_VERSION, self.now) def serialize(self, context=None): if context is None: context = {} context['request'] = self.request - serializer = ActivityLogSerializer(context=context) + serializer = ActivityLogSerializer(self.entry, context=context) return serializer.to_representation(self.entry) def test_basic(self): @@ -43,33 +42,32 @@ class TestReviewNotesSerializerOutput(TestCase, LogMixin): assert result['date'] == self.now.isoformat() + 'Z' assert result['action'] == 'rejected' assert result['action_label'] == 'Rejected' - assert result['comments'] == u'Oh nôes!' + assert result['comments'] == u'Oh nøes!' + # To allow reviewers to stay anonymous the user object only contains + # the "activity name", which uses the reviewer name alias if present. assert result['user'] == { - 'id': self.user.pk, - 'name': self.user.name, + 'id': None, + 'name': 'fôo', 'url': None, - 'username': self.user.username, + 'username': None, } def test_url_for_yourself(self): - # should include account profile url for your own requests self.request.user = self.user result = self.serialize() - assert result['user']['url'] == absolutify(self.user.get_url_path()) + assert result['user']['url'] is None def test_url_for_developers(self): - # should include account profile url for a developer addon_factory(users=[self.user]) result = self.serialize() - assert result['user']['url'] == absolutify(self.user.get_url_path()) + assert result['user']['url'] is None def test_url_for_admins(self): - # should include account profile url for admins admin = user_factory() self.grant_permission(admin, 'Users:Edit') self.request.user = admin result = self.serialize() - assert result['user']['url'] == absolutify(self.user.get_url_path()) + assert result['user']['url'] is None def test_should_highlight(self): result = self.serialize(context={'to_highlight': [self.entry]}) diff --git a/src/olympia/activity/tests/test_utils.py b/src/olympia/activity/tests/test_utils.py index 564194dd4a..882061db3d 100644 --- a/src/olympia/activity/tests/test_utils.py +++ b/src/olympia/activity/tests/test_utils.py @@ -4,6 +4,7 @@ import json import os from datetime import datetime, timedelta +from email.utils import formataddr from django.conf import settings from django.core import mail @@ -21,7 +22,7 @@ from olympia.activity.utils import ( ACTIVITY_MAIL_GROUP, ActivityEmailEncodingError, ActivityEmailParser, ActivityEmailTokenError, ActivityEmailUUIDError, add_email_to_activity_log, add_email_to_activity_log_wrapper, log_and_notify, - notify_about_activity_log, send_activity_mail) + notify_about_activity_log, NOTIFICATIONS_FROM_EMAIL, send_activity_mail) from olympia.addons.models import Addon, AddonReviewerFlags from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.amo.tests import TestCase, addon_factory, user_factory @@ -238,7 +239,7 @@ class TestLogAndNotify(TestCase): def setUp(self): self.developer = user_factory() self.developer2 = user_factory() - self.reviewer = user_factory() + self.reviewer = user_factory(reviewer_name='Revîewer') self.grant_permission(self.reviewer, 'Addons:Review', 'Addon Reviewers') @@ -280,6 +281,8 @@ class TestLogAndNotify(TestCase): assert days_text in body assert 'reviewing version %s of the add-on %s' % ( self.version.version, self.addon.name) in body + assert self.reviewer.name not in body + assert self.reviewer.reviewer_name in body def _check_email(self, call, url, reason_text): subject = call[0][0] @@ -289,6 +292,7 @@ class TestLogAndNotify(TestCase): assert ('visit %s' % url) in body assert ('receiving this email because %s' % reason_text) in body assert 'If we do not hear from you within' not in body + assert self.reviewer.name not in body @mock.patch('olympia.activity.utils.send_mail') def test_reviewer_request_for_information(self, send_mail_mock): @@ -300,8 +304,8 @@ class TestLogAndNotify(TestCase): amo.LOG.REQUEST_INFORMATION, 'blah', self.reviewer, self.version) assert send_mail_mock.call_count == 2 # Both authors. - sender = '%s ' % ( - self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.reviewer.reviewer_name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] recipients = self._recipients(send_mail_mock) assert len(recipients) == 2 @@ -331,8 +335,8 @@ class TestLogAndNotify(TestCase): amo.LOG.REQUEST_INFORMATION, 'blah', self.reviewer, self.version) assert send_mail_mock.call_count == 2 # Both authors. - sender = '%s ' % ( - self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.reviewer.reviewer_name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] recipients = self._recipients(send_mail_mock) assert len(recipients) == 2 @@ -362,8 +366,8 @@ class TestLogAndNotify(TestCase): amo.LOG.REQUEST_INFORMATION, 'blah', self.reviewer, self.version) assert send_mail_mock.call_count == 2 # Both authors. - sender = '%s ' % ( - self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.reviewer.reviewer_name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] recipients = self._recipients(send_mail_mock) assert len(recipients) == 2 @@ -415,8 +419,8 @@ class TestLogAndNotify(TestCase): assert logs[0].details['comments'] == u'Thïs is á reply' assert send_mail_mock.call_count == 2 # One author, one reviewer. - sender = '%s ' % ( - self.developer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.developer.name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] recipients = self._recipients(send_mail_mock) assert len(recipients) == 2 @@ -456,8 +460,8 @@ class TestLogAndNotify(TestCase): assert logs[0].details['comments'] == u'Thîs ïs a revïewer replyîng' assert send_mail_mock.call_count == 2 # Both authors. - sender = '%s ' % ( - self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.reviewer.reviewer_name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] recipients = self._recipients(send_mail_mock) assert len(recipients) == 2 @@ -489,8 +493,8 @@ class TestLogAndNotify(TestCase): assert not logs[0].details # No details json because no comment. assert send_mail_mock.call_count == 2 # One author, one reviewer. - sender = '%s ' % ( - self.developer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.developer.name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] recipients = self._recipients(send_mail_mock) assert len(recipients) == 2 @@ -518,8 +522,8 @@ class TestLogAndNotify(TestCase): assert len(logs) == 1 recipients = self._recipients(send_mail_mock) - sender = '%s ' % ( - self.developer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.developer.name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] assert len(recipients) == 2 # self.reviewers wasn't on the thread, but gets an email anyway. @@ -545,8 +549,8 @@ class TestLogAndNotify(TestCase): assert len(logs) == 1 recipients = self._recipients(send_mail_mock) - sender = '%s ' % ( - self.developer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr( + (self.developer.name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] developer_subject = send_mail_mock.call_args_list[0][0][0] assert developer_subject == ( @@ -675,7 +679,7 @@ class TestLogAndNotify(TestCase): @mock.patch('olympia.activity.utils.send_mail') def test_from_name_escape(self, send_mail_mock): - self.reviewer.update(display_name='mr "quote" escape') + self.reviewer.update(reviewer_name='mr "quote" escape') # One from the reviewer. self._create(amo.LOG.REJECT_VERSION, self.reviewer) @@ -707,8 +711,8 @@ class TestLogAndNotify(TestCase): assert ActivityLog.objects.count() == 1 # No new activity created. assert send_mail_mock.call_count == 2 # Both authors. - sender = '%s ' % ( - self.reviewer.name, settings.INBOUND_EMAIL_DOMAIN) + sender = formataddr(( + self.reviewer.reviewer_name, NOTIFICATIONS_FROM_EMAIL)) assert sender == send_mail_mock.call_args_list[0][1]['from_email'] recipients = self._recipients(send_mail_mock) assert len(recipients) == 2 diff --git a/src/olympia/activity/utils.py b/src/olympia/activity/utils.py index d1298e22f4..d375d0a3dd 100644 --- a/src/olympia/activity/utils.py +++ b/src/olympia/activity/utils.py @@ -242,7 +242,7 @@ def notify_about_activity_log(addon, version, note, perm_setting=None, author_context_dict = { 'name': addon.name, 'number': version.version, - 'author': note.user.name, + 'author': note.author_name, 'comments': comments, 'url': absolutify(addon.get_dev_url('versions')), 'SITE_URL': settings.SITE_URL, @@ -279,7 +279,7 @@ def notify_about_activity_log(addon, version, note, perm_setting=None, addon.name, version.version) # Build and send the mail for authors. template = template_from_user(note.user, version) - from_email = formataddr((note.user.name, NOTIFICATIONS_FROM_EMAIL)) + from_email = formataddr((note.author_name, NOTIFICATIONS_FROM_EMAIL)) send_activity_mail( subject, template.render(author_context_dict), version, addon_authors, from_email, note.id, perm_setting) diff --git a/src/olympia/devhub/templates/devhub/addons/activity.html b/src/olympia/devhub/templates/devhub/addons/activity.html index f3b55c71df..b244c175e5 100644 --- a/src/olympia/devhub/templates/devhub/addons/activity.html +++ b/src/olympia/devhub/templates/devhub/addons/activity.html @@ -30,7 +30,8 @@ {{ item }}

- {% trans user=item.user|user_link, ago=item.created|timesince, + {% trans user=item.author_name, + ago=item.created|timesince, iso=item.created|isotime, pretty=item.created|datetime %} diff --git a/src/olympia/devhub/templates/devhub/versions/list.html b/src/olympia/devhub/templates/devhub/versions/list.html index 6975394a01..4fbc91ac84 100644 --- a/src/olympia/devhub/templates/devhub/versions/list.html +++ b/src/olympia/devhub/templates/devhub/versions/list.html @@ -121,8 +121,8 @@

{{ _('Load older...') }}

diff --git a/src/olympia/devhub/tests/test_views.py b/src/olympia/devhub/tests/test_views.py index 6e6891ba84..5bf7aa9dc2 100644 --- a/src/olympia/devhub/tests/test_views.py +++ b/src/olympia/devhub/tests/test_views.py @@ -647,6 +647,8 @@ class TestActivityFeed(TestCase): assert self.client.login(email='del@icio.us') self.addon = Addon.objects.get(id=3615) self.version = self.addon.versions.first() + self.action_user = UserProfile.objects.get( + email='reviewer@mozilla.com') def test_feed_for_all(self): response = self.client.get(reverse('devhub.feed_all')) @@ -675,7 +677,7 @@ class TestActivityFeed(TestCase): assert response.status_code == 302 def add_log(self, action=amo.LOG.ADD_RATING): - core.set_user(UserProfile.objects.get(email='del@icio.us')) + core.set_user(self.action_user) ActivityLog.create(action, self.addon, self.version) def add_hidden_log(self, action=amo.LOG.COMMENT_VERSION): @@ -728,6 +730,32 @@ class TestActivityFeed(TestCase): doc = pq(res.content) assert len(doc('#recent-activity .item')) == 1 + def test_reviewer_name_is_used_for_reviewer_actions(self): + self.action_user.update(display_name='HîdeMe', reviewer_name='ShöwMe') + self.add_log(action=amo.LOG.APPROVE_VERSION) + response = self.client.get( + reverse('devhub.feed', args=[self.addon.slug])) + doc = pq(response.content) + assert len(doc('#recent-activity .item')) == 1 + + content = force_text(response.content) + assert self.action_user.reviewer_name in content + assert self.action_user.name not in content + + def test_regular_name_is_used_for_non_reviewer_actions(self): + # Fields are inverted compared to the test above. + self.action_user.update(reviewer_name='HîdeMe', display_name='ShöwMe') + self.add_log(action=amo.LOG.ADD_RATING) # not a reviewer action. + response = self.client.get( + reverse('devhub.feed', args=[self.addon.slug])) + doc = pq(response.content) + assert len(doc('#recent-activity .item')) == 1 + + content = force_text(response.content) + # Assertions are inverted compared to the test above. + assert self.action_user.reviewer_name not in content + assert self.action_user.name in content + class TestAPIAgreement(TestCase): fixtures = ['base/addon_3615', 'base/addon_5579', 'base/users'] diff --git a/src/olympia/migrations/1082-add-reviewer-name.sql b/src/olympia/migrations/1082-add-reviewer-name.sql new file mode 100644 index 0000000000..8499bbb34f --- /dev/null +++ b/src/olympia/migrations/1082-add-reviewer-name.sql @@ -0,0 +1 @@ +ALTER TABLE `users` ADD COLUMN `reviewer_name` varchar(50); diff --git a/src/olympia/reviewers/utils.py b/src/olympia/reviewers/utils.py index 4b77f681d8..521ddbfbf4 100644 --- a/src/olympia/reviewers/utils.py +++ b/src/olympia/reviewers/utils.py @@ -570,7 +570,7 @@ class ReviewBase(object): dev_ver_url = self.addon.get_dev_url('versions') return {'name': addon.name, 'number': self.version.version if self.version else '', - 'reviewer': self.user.name, + 'reviewer': self.user.reviewer_name or self.user.name, 'addon_url': absolutify(addon_url), 'dev_versions_url': absolutify(dev_ver_url), 'review_url': absolutify(reverse('reviewers.review', diff --git a/src/olympia/users/admin.py b/src/olympia/users/admin.py index eb242b3afb..1c27df22ab 100644 --- a/src/olympia/users/admin.py +++ b/src/olympia/users/admin.py @@ -48,8 +48,8 @@ class UserAdmin(admin.ModelAdmin): fieldsets = ( (None, { 'fields': ('id', 'email', 'fxa_id', 'username', 'display_name', - 'biography', 'homepage', 'location', 'occupation', - 'picture_img'), + 'reviewer_name', 'biography', 'homepage', 'location', + 'occupation', 'picture_img'), }), ('Flags', { 'fields': ('display_collections', 'deleted', 'is_public'), diff --git a/src/olympia/users/models.py b/src/olympia/users/models.py index cd3b89cc32..cf468d1684 100644 --- a/src/olympia/users/models.py +++ b/src/olympia/users/models.py @@ -166,6 +166,10 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser): # newsletter basket_token = models.CharField(blank=True, default='', max_length=128) + reviewer_name = models.CharField( + max_length=50, default='', null=True, blank=True, + validators=[validators.MinLengthValidator(2)]) + class Meta: db_table = 'users' diff --git a/static/js/zamboni/devhub.js b/static/js/zamboni/devhub.js index 8b3fd789fb..de72493962 100644 --- a/static/js/zamboni/devhub.js +++ b/static/js/zamboni/devhub.js @@ -563,11 +563,10 @@ function initVersions() { if (note["highlight"] == true) { clone.addClass("new"); } - clone.find('span.action')[0].textContent = note["action_label"]; - var user = clone.find('a:contains("$user_name")'); + clone.find('.action')[0].textContent = note["action_label"]; + var user = clone.find('.user_name'); user[0].textContent = note["user"]["name"]; - user.attr('href', note["user"]["url"]); - var date = clone.find('time.timeago'); + var date = clone.find('.timeago'); date[0].textContent = note["date"]; date.attr('datetime', note["date"]); date.attr('title', note["date"]);