diff --git a/docs/topics/api/activity.rst b/docs/topics/api/activity.rst index 9697194f32..2c4c5d92ec 100644 --- a/docs/topics/api/activity.rst +++ b/docs/topics/api/activity.rst @@ -40,9 +40,9 @@ 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``. + 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)/ @@ -51,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`. :>json string action_label: The text label of the action. - :>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|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/activity/serializers.py b/src/olympia/activity/serializers.py index 7f88288595..e2c9f4e26d 100644 --- a/src/olympia/activity/serializers.py +++ b/src/olympia/activity/serializers.py @@ -3,6 +3,7 @@ from django.utils.translation import ugettext from rest_framework import serializers from olympia.activity.models import ActivityLog +from olympia.api.utils import is_gate_active class ActivityLogSerializer(serializers.ModelSerializer): @@ -42,10 +43,16 @@ class ActivityLogSerializer(serializers.ModelSerializer): 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, + 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 diff --git a/src/olympia/activity/tests/test_serializers.py b/src/olympia/activity/tests/test_serializers.py index b2df27da52..153b415706 100644 --- a/src/olympia/activity/tests/test_serializers.py +++ b/src/olympia/activity/tests/test_serializers.py @@ -44,30 +44,39 @@ class TestReviewNotesSerializerOutput(TestCase, LogMixin): assert result['action_label'] == 'Rejected' 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. + # the author name, which can use the reviewer name alias if present + # depending on the action. assert result['user'] == { - 'id': None, - 'name': 'fôo', - 'url': None, - 'username': None, + 'name': self.user.reviewer_name, } - def test_url_for_yourself(self): - self.request.user = self.user + def test_basic_v3(self): + self.request.version = 'v3' result = self.serialize() - assert result['user']['url'] is None - def test_url_for_developers(self): - addon_factory(users=[self.user]) - result = self.serialize() - assert result['user']['url'] is None + 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): - 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'] is None + assert result['user'] == { + 'name': self.user.name, + } def test_should_highlight(self): result = self.serialize(context={'to_highlight': [self.entry]}) diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index f23ac48af5..b4045cbeb9 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -1654,6 +1654,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',