Merge pull request #11088 from diox/reviewer-alias
Add reviewer name alias for reviewers to hide their actual name
This commit is contained in:
Mathieu Pillard
GitHub
Коммит
49ffbfb213
|
|
@ -59,6 +59,7 @@ If you have `Users:Edit` permission you will see these extra fields for all user
|
|||
|
||||
:>json boolean deleted: Is the account deleted.
|
||||
:>json string|null display_name: The name chosen by the user.
|
||||
:>json string|null reviewer_name: The reviewer name chosen by the user, if set. Only available for users with any kind of reviewer permission.
|
||||
:>json string email: Email address used by the user to login and create this account.
|
||||
:>json string fxa_edit_email_url: The configured URL for editing the user's email on FxA.
|
||||
:>json string last_login: The date of the last successful log in to the website.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
only contains the name of the reviewer or author. That name may, for
|
||||
some actions, be an alias and not the usual name of the user.
|
||||
|
||||
.. 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,7 @@ 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<review-note-action>`.
|
||||
:>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 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 comments: The text content of the review note.
|
||||
:>json string date: The date the review note was created.
|
||||
|
||||
|
|
|
|||
|
|
@ -294,3 +294,4 @@ v4 API changelog
|
|||
* 2019-01-10: added ``release_notes`` and ``license`` (except ``license.text``) to search API results ``current_version`` objects.
|
||||
* 2019-01-11: added new /reviewers/browse/ endpoint. https://github.com/mozilla/addons-server/issues/10322
|
||||
* 2019-01-16: Removed /api/{v3,v4,v4dev}/github api entirely. They have been marked as experimental. https://github.com/mozilla/addons-server/issues/10411
|
||||
* 2019-04-11: Removed ``id``, ``username`` and ``url`` from the ``user`` object in the activity review notes APIs. https://github.com/mozilla/addons-server/issues/11002
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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 = {
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@ from django.utils.translation import ugettext
|
|||
|
||||
from rest_framework import serializers
|
||||
|
||||
from olympia.accounts.serializers import BaseUserSerializer
|
||||
from olympia.activity.models import ActivityLog
|
||||
from olympia.api.utils import is_gate_active
|
||||
|
||||
|
||||
class ActivityLogSerializer(serializers.ModelSerializer):
|
||||
|
|
@ -11,7 +11,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 +37,22 @@ 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 in v3 API
|
||||
only."""
|
||||
data = {
|
||||
'name': obj.author_name,
|
||||
}
|
||||
request = self.context.get('request')
|
||||
if request and is_gate_active(request, 'activity-user-shim'):
|
||||
data.update({
|
||||
'id': None,
|
||||
'username': None,
|
||||
'url': None
|
||||
})
|
||||
return data
|
||||
|
|
|
|||
|
|
@ -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,41 @@ 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 author name, which can use the reviewer name alias if present
|
||||
# depending on the action.
|
||||
assert result['user'] == {
|
||||
'id': self.user.pk,
|
||||
'name': self.user.name,
|
||||
'url': None,
|
||||
'username': self.user.username,
|
||||
'name': self.user.reviewer_name,
|
||||
}
|
||||
|
||||
def test_url_for_yourself(self):
|
||||
# should include account profile url for your own requests
|
||||
self.request.user = self.user
|
||||
def test_basic_v3(self):
|
||||
self.request.version = 'v3'
|
||||
result = self.serialize()
|
||||
assert result['user']['url'] == absolutify(self.user.get_url_path())
|
||||
|
||||
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['id'] == self.entry.pk
|
||||
assert result['date'] == self.now.isoformat() + 'Z'
|
||||
assert result['action'] == 'rejected'
|
||||
assert result['action_label'] == 'Rejected'
|
||||
assert result['comments'] == u'Oh nøes!'
|
||||
# For backwards-compatibility in API v3 the id, url and username are
|
||||
# present but empty - we still don't want to reveal the actual reviewer
|
||||
# info.
|
||||
assert result['user'] == {
|
||||
'id': None,
|
||||
'url': None,
|
||||
'username': None,
|
||||
'name': self.user.reviewer_name,
|
||||
}
|
||||
|
||||
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
|
||||
def test_basic_somehow_not_a_reviewer_action(self):
|
||||
"""Like test_basic(), but somehow the action is not a reviewer action
|
||||
and therefore shouldn't use the reviewer_name."""
|
||||
self.entry.update(action=amo.LOG.ADD_RATING.id)
|
||||
result = self.serialize()
|
||||
assert result['user']['url'] == absolutify(self.user.get_url_path())
|
||||
assert result['user'] == {
|
||||
'name': self.user.name,
|
||||
}
|
||||
|
||||
def test_should_highlight(self):
|
||||
result = self.serialize(context={'to_highlight': [self.entry]})
|
||||
|
|
|
|||
|
|
@ -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 <notifications@%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 <notifications@%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 <notifications@%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 <notifications@%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 <notifications@%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 <notifications@%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 <notifications@%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 <notifications@%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 <notifications@%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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -30,7 +30,8 @@
|
|||
{{ item }}
|
||||
</p>
|
||||
<p class="timestamp">
|
||||
{% 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 %}
|
||||
<time datetime="{{ iso }}" title="{{ pretty }}">{{ ago }}</time>
|
||||
|
|
|
|||
|
|
@ -121,8 +121,8 @@
|
|||
<p><a href="#" class="review-history-loadmore" data-div="#{{ version.id }}-review-history">{{ _('Load older...') }}</a></p>
|
||||
</div>
|
||||
<div class="review-entry-empty hidden">
|
||||
<p><span class="action">$action_label</span> {{ _('by') }}
|
||||
<a href="$user_profile">$user_name</a> <time class="timeago" datetime=$date>$date</time></p>
|
||||
<p><strong class="action">$action_label</strong> {{ _('by') }}
|
||||
<em class="user_name">$user_name</em> <time class="timeago" datetime=$date>$date</time></p>
|
||||
<pre>$comments</pre>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -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']
|
||||
|
|
|
|||
|
|
@ -1655,6 +1655,7 @@ DRF_API_GATES = {
|
|||
'del-accounts-fxa-edit-email-url',
|
||||
'del-version-license-is-custom',
|
||||
'del-ratings-flags',
|
||||
'activity-user-shim',
|
||||
),
|
||||
'v4': (
|
||||
'l10n_flat_input_output',
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
ALTER TABLE `users` ADD COLUMN `reviewer_name` varchar(50);
|
||||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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'),
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
||||
|
|
|
|||
|
|
@ -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"]);
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче