Add API to post an add-on review
This commit is contained in:
Mathieu Pillard
Родитель
8a0618495d
Коммит
6fa70318ea
|
|
@ -60,3 +60,18 @@ This endpoint allows you to fetch a review by its id.
|
|||
:>json object user: Object holding information about the user who posted the review.
|
||||
:>json string user.url: The user profile URL.
|
||||
:>json string user.name: The user name.
|
||||
|
||||
----
|
||||
Post
|
||||
----
|
||||
|
||||
.. review-list-addon:
|
||||
|
||||
This endpoint allows you to post a new review for a given add-on and version.
|
||||
|
||||
.. http:post:: /api/v3/addons/addon/(int:id|string:slug|string:guid)/reviews/
|
||||
|
||||
:<json string|null body: The text of the review.
|
||||
:<json string|null: The title of the review.
|
||||
:<json int rating: The rating the user wants to give as part of the review (required).
|
||||
:<json int version: The add-on version id the review applies to.
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import json
|
|||
from olympia import amo
|
||||
from olympia.activity.tests.test_serializers import LogMixin
|
||||
from olympia.amo.tests import (
|
||||
addon_factory, user_factory, TestCase)
|
||||
addon_factory, APITestClient, user_factory, TestCase)
|
||||
from olympia.amo.urlresolvers import reverse
|
||||
from olympia.addons.models import AddonUser
|
||||
from olympia.addons.utils import generate_addon_guid
|
||||
|
|
@ -123,6 +123,8 @@ class ReviewNotesViewSetDetailMixin(LogMixin):
|
|||
|
||||
|
||||
class TestReviewNotesViewSetDetail(ReviewNotesViewSetDetailMixin, TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestReviewNotesViewSetDetail, self).setUp()
|
||||
self.addon = addon_factory(
|
||||
|
|
@ -155,6 +157,8 @@ class TestReviewNotesViewSetDetail(ReviewNotesViewSetDetailMixin, TestCase):
|
|||
|
||||
|
||||
class TestReviewNotesViewSetList(ReviewNotesViewSetDetailMixin, TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestReviewNotesViewSetList, self).setUp()
|
||||
self.addon = addon_factory(
|
||||
|
|
|
|||
|
|
@ -34,5 +34,6 @@ class VersionReviewNotesViewSet(AddonChildMixin, RetrieveModelMixin,
|
|||
|
||||
def check_object_permissions(self, request, obj):
|
||||
"""Check object permissions against the Addon, not the ActivityLog."""
|
||||
# Just loading the add-on object should trigger permission checks.
|
||||
# Just loading the add-on object triggers permission checks, because
|
||||
# the implementation in AddonChildMixin calls AddonViewSet.get_object()
|
||||
self.get_addon_object()
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ from pyquery import PyQuery as pq
|
|||
from waffle.testutils import override_switch
|
||||
|
||||
from olympia import amo
|
||||
from olympia.amo.tests import ESTestCase, TestCase
|
||||
from olympia.amo.tests import APITestClient, ESTestCase, TestCase
|
||||
from olympia.amo.helpers import numberfmt, urlparams
|
||||
from olympia.amo.tests import addon_factory, version_factory
|
||||
from olympia.amo.urlresolvers import reverse
|
||||
|
|
@ -1704,6 +1704,8 @@ class AddonAndVersionViewSetDetailMixin(object):
|
|||
|
||||
|
||||
class TestAddonViewSetDetail(AddonAndVersionViewSetDetailMixin, TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestAddonViewSetDetail, self).setUp()
|
||||
self.addon = addon_factory(
|
||||
|
|
@ -1724,6 +1726,8 @@ class TestAddonViewSetDetail(AddonAndVersionViewSetDetailMixin, TestCase):
|
|||
|
||||
|
||||
class TestVersionViewSetDetail(AddonAndVersionViewSetDetailMixin, TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestVersionViewSetDetail, self).setUp()
|
||||
self.addon = addon_factory(
|
||||
|
|
@ -1821,6 +1825,8 @@ class TestVersionViewSetDetail(AddonAndVersionViewSetDetailMixin, TestCase):
|
|||
|
||||
|
||||
class TestVersionViewSetList(AddonAndVersionViewSetDetailMixin, TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestVersionViewSetList, self).setUp()
|
||||
self.addon = addon_factory(
|
||||
|
|
@ -1950,6 +1956,8 @@ class TestVersionViewSetList(AddonAndVersionViewSetDetailMixin, TestCase):
|
|||
|
||||
|
||||
class TestAddonViewSetFeatureCompatibility(TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestAddonViewSetFeatureCompatibility, self).setUp()
|
||||
self.addon = addon_factory(
|
||||
|
|
@ -1982,6 +1990,8 @@ class TestAddonViewSetFeatureCompatibility(TestCase):
|
|||
|
||||
|
||||
class TestAddonViewSetEulaPolicy(TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestAddonViewSetEulaPolicy, self).setUp()
|
||||
self.addon = addon_factory(
|
||||
|
|
@ -2017,6 +2027,8 @@ class TestAddonViewSetEulaPolicy(TestCase):
|
|||
|
||||
|
||||
class TestAddonSearchView(ESTestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
fixtures = ['base/users']
|
||||
|
||||
def setUp(self):
|
||||
|
|
@ -2337,6 +2349,8 @@ class TestAddonSearchView(ESTestCase):
|
|||
|
||||
|
||||
class TestAddonFeaturedView(TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
self.url = reverse('addon-featured')
|
||||
|
||||
|
|
|
|||
|
|
@ -691,7 +691,7 @@ class AddonVersionViewSet(AddonChildMixin, RetrieveModelMixin,
|
|||
AllowRelatedObjectPermissions('addon', AddonViewSet.permission_classes)
|
||||
]
|
||||
serializer_class = VersionSerializer
|
||||
# Since permission checks are dont on the parent add-on, we rely on
|
||||
# Since permission checks are done on the parent add-on, we rely on
|
||||
# queryset filtering to hide non-valid versions. get_queryset() might
|
||||
# override this if we are asked to see non-valid versions explicitly.
|
||||
queryset = Version.objects.filter(
|
||||
|
|
|
|||
|
|
@ -22,12 +22,15 @@ from django.test.client import Client, RequestFactory
|
|||
from django.test.utils import override_settings
|
||||
from django.conf import urls as django_urls
|
||||
from django.utils import translation
|
||||
from django.utils.encoding import force_bytes
|
||||
from django.utils.importlib import import_module
|
||||
from django.utils.http import urlencode
|
||||
|
||||
import mock
|
||||
import pytest
|
||||
from dateutil.parser import parse as dateutil_parser
|
||||
from rest_framework.views import APIView
|
||||
from rest_framework.test import APIClient
|
||||
from waffle.models import Flag, Sample, Switch
|
||||
|
||||
from olympia import amo
|
||||
|
|
@ -247,6 +250,9 @@ class TestClient(Client):
|
|||
else:
|
||||
raise AttributeError
|
||||
|
||||
|
||||
class APITestClient(APIClient):
|
||||
|
||||
def generate_api_token(self, user, **payload_overrides):
|
||||
"""
|
||||
Creates a jwt token for this user.
|
||||
|
|
@ -274,6 +280,17 @@ class TestClient(Client):
|
|||
"""
|
||||
self.defaults.pop('HTTP_AUTHORIZATION', None)
|
||||
|
||||
def get(self, path, data=None, **extra):
|
||||
# Work around DRF #4458 since we're running an old version that does
|
||||
# not have this fix yet.
|
||||
r = {
|
||||
'QUERY_STRING': urlencode(data or {}, doseq=True),
|
||||
}
|
||||
if not data and '?' in path:
|
||||
r['QUERY_STRING'] = force_bytes(path.split('?')[1])
|
||||
r.update(extra)
|
||||
return self.generic('GET', path, **r)
|
||||
|
||||
|
||||
Mocked_ES = mock.patch('olympia.amo.search.get_es', spec=True)
|
||||
|
||||
|
|
|
|||
|
|
@ -76,6 +76,20 @@ class AllowAddonAuthor(BasePermission):
|
|||
return obj.authors.filter(pk=request.user.pk).exists()
|
||||
|
||||
|
||||
class AllowOwner(BasePermission):
|
||||
"""
|
||||
Permission class to use when you are dealing with a model instance that has
|
||||
a "user" FK pointing to an UserProfile, and you want only the corresponding
|
||||
user to be able to access your instance.
|
||||
"""
|
||||
def has_permission(self, request, view):
|
||||
return request.user.is_authenticated()
|
||||
|
||||
def has_object_permission(self, request, view, obj):
|
||||
return ((obj == request.user) or
|
||||
(getattr(obj, 'user', None) == request.user))
|
||||
|
||||
|
||||
class AllowReviewer(BasePermission):
|
||||
"""Allow addons reviewer access.
|
||||
|
||||
|
|
@ -115,7 +129,20 @@ class AllowReviewerUnlisted(AllowReviewer):
|
|||
return not obj.is_listed and self.has_permission(request, view)
|
||||
|
||||
|
||||
class AllowReadOnlyIfReviewedAndListed(BasePermission):
|
||||
class AllowIfReviewedAndListed(BasePermission):
|
||||
"""
|
||||
Allow access when the object's is_public() method and is_listed property
|
||||
both return True.
|
||||
"""
|
||||
def has_permission(self, request, view):
|
||||
return True
|
||||
|
||||
def has_object_permission(self, request, view, obj):
|
||||
return (obj.is_reviewed() and not obj.disabled_by_user and
|
||||
obj.is_listed and self.has_permission(request, view))
|
||||
|
||||
|
||||
class AllowReadOnlyIfReviewedAndListed(AllowIfReviewedAndListed):
|
||||
"""
|
||||
Allow access when the object's is_public() method and is_listed property
|
||||
both return True and the request HTTP method is GET/OPTIONS/HEAD.
|
||||
|
|
@ -123,10 +150,6 @@ class AllowReadOnlyIfReviewedAndListed(BasePermission):
|
|||
def has_permission(self, request, view):
|
||||
return request.method in SAFE_METHODS
|
||||
|
||||
def has_object_permission(self, request, view, obj):
|
||||
return (obj.is_reviewed() and not obj.disabled_by_user and
|
||||
obj.is_listed and self.has_permission(request, view))
|
||||
|
||||
|
||||
class ByHttpMethod(BasePermission):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ from rest_framework_jwt.settings import api_settings
|
|||
from rest_framework_jwt.views import refresh_jwt_token
|
||||
|
||||
from olympia.amo.helpers import absolutify
|
||||
from olympia.amo.tests import TestCase, WithDynamicEndpoints
|
||||
from olympia.amo.tests import APITestClient, TestCase, WithDynamicEndpoints
|
||||
from olympia.api.authentication import (
|
||||
JSONWebTokenAuthentication, JWTKeyAuthentication)
|
||||
from olympia.api.tests.test_jwt_auth import JWTAuthKeyTester
|
||||
|
|
@ -38,6 +38,7 @@ class JWTKeyAuthTestView(APIView):
|
|||
|
||||
class TestJWTKeyAuthentication(JWTAuthKeyTester):
|
||||
fixtures = ['base/addon_3615']
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestJWTKeyAuthentication, self).setUp()
|
||||
|
|
@ -146,6 +147,7 @@ class TestJWTKeyAuthentication(JWTAuthKeyTester):
|
|||
|
||||
class TestJWTKeyAuthProtectedView(WithDynamicEndpoints, JWTAuthKeyTester):
|
||||
fixtures = ['base/addon_3615']
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestJWTKeyAuthProtectedView, self).setUp()
|
||||
|
|
@ -190,6 +192,7 @@ class TestJWTKeyAuthProtectedView(WithDynamicEndpoints, JWTAuthKeyTester):
|
|||
|
||||
class TestJSONWebTokenAuthentication(TestCase):
|
||||
fixtures = ['base/addon_3615']
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestJSONWebTokenAuthentication, self).setUp()
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ from rest_framework.views import APIView
|
|||
from olympia.access.models import Group, GroupUser
|
||||
from olympia.addons.models import Addon
|
||||
from olympia.api.permissions import (
|
||||
AllowAddonAuthor, AllowNone, AllowReadOnlyIfReviewedAndListed,
|
||||
AllowAddonAuthor, AllowNone, AllowOwner, AllowReadOnlyIfReviewedAndListed,
|
||||
AllowReviewer, AllowReviewerUnlisted, AnyOf, ByHttpMethod, GroupPermission)
|
||||
from olympia.amo.tests import TestCase, WithDynamicEndpoints
|
||||
from olympia.users.models import UserProfile
|
||||
|
|
@ -161,6 +161,44 @@ class TestAllowAddonAuthor(TestCase):
|
|||
self.request, myview, self.addon)
|
||||
|
||||
|
||||
class TestAllowOwner(TestCase):
|
||||
fixtures = ['base/users']
|
||||
|
||||
def setUp(self):
|
||||
self.permission = AllowOwner()
|
||||
self.anonymous = AnonymousUser()
|
||||
self.user = UserProfile.objects.get(pk=999)
|
||||
self.request = RequestFactory().get('/')
|
||||
self.request.user = self.anonymous
|
||||
|
||||
def test_has_permission_anonymous(self):
|
||||
assert not self.permission.has_permission(self.request, 'myview')
|
||||
|
||||
def test_has_permission_user(self):
|
||||
self.request.user = self.user
|
||||
assert self.permission.has_permission(self.request, 'myview')
|
||||
|
||||
def test_has_object_permission_user(self):
|
||||
self.request.user = self.user
|
||||
obj = Mock()
|
||||
obj.user = self.user
|
||||
assert self.permission.has_object_permission(
|
||||
self.request, 'myview', obj)
|
||||
|
||||
def test_has_object_permission_no_user_on_obj(self):
|
||||
self.request.user = self.user
|
||||
obj = Mock()
|
||||
assert not self.permission.has_object_permission(
|
||||
self.request, 'myview', obj)
|
||||
|
||||
def test_has_object_permission_different_user(self):
|
||||
self.request.user = self.user
|
||||
obj = Mock()
|
||||
obj.user = UserProfile.objects.get(pk=20)
|
||||
assert not self.permission.has_object_permission(
|
||||
self.request, 'myview', obj)
|
||||
|
||||
|
||||
class TestAllowReviewer(TestCase):
|
||||
fixtures = ['base/users']
|
||||
|
||||
|
|
|
|||
|
|
@ -8,12 +8,12 @@ from django.core.urlresolvers import reverse
|
|||
from django.test import override_settings
|
||||
|
||||
import mock
|
||||
from rest_framework.test import APIClient
|
||||
from rest_framework_jwt.serializers import VerifyJSONWebTokenSerializer
|
||||
|
||||
from olympia.accounts import verify, views
|
||||
from olympia.accounts.tests.test_views import BaseAuthenticationView
|
||||
from olympia.amo.tests import addon_factory, ESTestCase, TestCase
|
||||
from olympia.amo.tests import (
|
||||
addon_factory, APITestClient, ESTestCase, TestCase)
|
||||
from olympia.users.models import UserProfile
|
||||
|
||||
FXA_CONFIG = {
|
||||
|
|
@ -25,6 +25,7 @@ FXA_CONFIG = {
|
|||
|
||||
|
||||
class TestInternalAddonSearchView(ESTestCase):
|
||||
client_class = APITestClient
|
||||
fixtures = ['base/users']
|
||||
|
||||
def setUp(self):
|
||||
|
|
@ -172,6 +173,7 @@ class TestInternalAddonSearchView(ESTestCase):
|
|||
|
||||
@override_settings(FXA_CONFIG={'internal': FXA_CONFIG})
|
||||
class TestLoginStartView(TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super(TestLoginStartView, self).setUp()
|
||||
|
|
@ -259,6 +261,7 @@ endpoint_overrides = [
|
|||
FXA_CONFIG={'internal': FXA_CONFIG},
|
||||
CORS_ENDPOINT_OVERRIDES=endpoint_overrides)
|
||||
class TestLoginView(BaseAuthenticationView):
|
||||
client_class = APITestClient
|
||||
view_name = 'internal-login'
|
||||
|
||||
def setUp(self):
|
||||
|
|
@ -276,7 +279,7 @@ class TestLoginView(BaseAuthenticationView):
|
|||
return self.client.post(self.url, kwargs)
|
||||
|
||||
def options(self, url, origin):
|
||||
return APIClient(HTTP_ORIGIN=origin).options(url)
|
||||
return self.client_class(HTTP_ORIGIN=origin).options(url)
|
||||
|
||||
def test_no_code_provided(self):
|
||||
response = self.post(code='')
|
||||
|
|
|
|||
|
|
@ -1641,7 +1641,10 @@ REST_FRAMEWORK = {
|
|||
# Use our pagination class by default, which allows clients to request a
|
||||
# different page size.
|
||||
'DEFAULT_PAGINATION_CLASS': (
|
||||
'olympia.api.paginator.CustomPageNumberPagination')
|
||||
'olympia.api.paginator.CustomPageNumberPagination'),
|
||||
|
||||
# Use json by default when using APIClient.
|
||||
'TEST_REQUEST_DEFAULT_FORMAT': 'json',
|
||||
}
|
||||
|
||||
# This is the DSN to the local Sentry service. It might be overridden in
|
||||
|
|
|
|||
|
|
@ -0,0 +1,12 @@
|
|||
from rest_framework.permissions import BasePermission
|
||||
|
||||
from olympia.reviews.helpers import user_can_delete_review
|
||||
|
||||
|
||||
class CanDeleteReviewPermission(BasePermission):
|
||||
"""A DRF permission class wrapping user_can_delete_review()."""
|
||||
def has_permission(self, request, view):
|
||||
return request.user.is_authenticated()
|
||||
|
||||
def has_object_permission(self, request, view, obj):
|
||||
return user_can_delete_review(request, obj)
|
||||
|
|
@ -1,7 +1,9 @@
|
|||
from rest_framework import serializers
|
||||
from rest_framework.relations import PrimaryKeyRelatedField
|
||||
|
||||
from olympia.reviews.models import Review
|
||||
from olympia.users.serializers import BaseUserSerializer
|
||||
from olympia.versions.models import Version
|
||||
|
||||
|
||||
class BaseReviewSerializer(serializers.ModelSerializer):
|
||||
|
|
@ -9,9 +11,13 @@ class BaseReviewSerializer(serializers.ModelSerializer):
|
|||
# translation for each review - it's essentially useless. Because of that
|
||||
# we use a simple CharField in the API, hiding the fact that it's a
|
||||
# TranslatedField underneath.
|
||||
body = serializers.CharField()
|
||||
title = serializers.CharField()
|
||||
user = BaseUserSerializer()
|
||||
body = serializers.CharField(allow_null=True, required=False)
|
||||
title = serializers.CharField(allow_null=True, required=False)
|
||||
user = BaseUserSerializer(read_only=True)
|
||||
|
||||
# Version queryset is unfiltered, the version is checked more thoroughly
|
||||
# in the validate() method.
|
||||
version = PrimaryKeyRelatedField(queryset=Version.unfiltered)
|
||||
|
||||
class Meta:
|
||||
model = Review
|
||||
|
|
@ -24,9 +30,41 @@ class BaseReviewSerializer(serializers.ModelSerializer):
|
|||
data['version'] = unicode(obj.version.version) if obj.version else None
|
||||
return data
|
||||
|
||||
def validate_version(self, version):
|
||||
addon = self.context['view'].get_addon_object()
|
||||
if (version.addon_id != addon.pk or
|
||||
not version.is_public()):
|
||||
raise serializers.ValidationError(
|
||||
'Version does not exist on this add-on or is not public.')
|
||||
return version
|
||||
|
||||
def validate(self, data):
|
||||
data = super(BaseReviewSerializer, self).validate(data)
|
||||
# Get the add-on pk from the URL, no need to pass it as POST data since
|
||||
# the URL is always going to have it.
|
||||
data['addon'] = self.context['view'].get_addon_object()
|
||||
|
||||
# Get the user from the request, don't allow clients to pick one
|
||||
# themselves.
|
||||
data['user'] = self.context['request'].user
|
||||
|
||||
if data['addon'].authors.filter(pk=data['user'].pk).exists():
|
||||
raise serializers.ValidationError(
|
||||
'An add-on author can not leave a review on its own add-on.')
|
||||
|
||||
if Review.objects.filter(
|
||||
addon=data['addon'], user=data['user'],
|
||||
version=data['version']).exists():
|
||||
raise serializers.ValidationError(
|
||||
'The same user can not leave a review on the same version more'
|
||||
' than once.')
|
||||
|
||||
return data
|
||||
|
||||
|
||||
class ReviewSerializer(BaseReviewSerializer):
|
||||
reply = BaseReviewSerializer(read_only=True)
|
||||
rating = serializers.IntegerField(min_value=1, max_value=5)
|
||||
|
||||
class Meta:
|
||||
model = Review
|
||||
|
|
|
|||
|
|
@ -0,0 +1,33 @@
|
|||
from django.contrib.auth.models import AnonymousUser
|
||||
from django.test import RequestFactory
|
||||
|
||||
from mock import mock
|
||||
|
||||
from olympia.amo.tests import TestCase
|
||||
from olympia.reviews.permissions import CanDeleteReviewPermission
|
||||
from olympia.users.models import UserProfile
|
||||
|
||||
|
||||
class TestCanDeleteReviewPermission(TestCase):
|
||||
def setUp(self):
|
||||
self.request = RequestFactory().get('/')
|
||||
self.request.user = AnonymousUser()
|
||||
self.perm = CanDeleteReviewPermission()
|
||||
|
||||
def test_has_permission_anonymous(self):
|
||||
assert not self.perm.has_permission(self.request, None)
|
||||
|
||||
def test_has_permission_authenticated(self):
|
||||
self.request.user = UserProfile()
|
||||
assert self.perm.has_permission(self.request, None)
|
||||
|
||||
@mock.patch('olympia.reviews.permissions.user_can_delete_review')
|
||||
def test_has_object_permission(self, user_can_delete_review_mock):
|
||||
user_can_delete_review_mock.return_value = True
|
||||
assert self.perm.has_object_permission(self.request, None, object())
|
||||
|
||||
@mock.patch('olympia.reviews.permissions.user_can_delete_review')
|
||||
def test_has_object_permission_false(self, user_can_delete_review_mock):
|
||||
user_can_delete_review_mock.return_value = False
|
||||
assert not self.perm.has_object_permission(
|
||||
self.request, None, object())
|
||||
|
|
@ -7,9 +7,12 @@ import mock
|
|||
from pyquery import PyQuery as pq
|
||||
from rest_framework.exceptions import ParseError
|
||||
|
||||
from olympia import amo
|
||||
from olympia.addons.utils import generate_addon_guid
|
||||
from olympia.amo import helpers
|
||||
from olympia.amo.tests import addon_factory, TestCase, MobileTest, user_factory
|
||||
from olympia.amo.tests import (
|
||||
addon_factory, APITestClient, TestCase, MobileTest, version_factory,
|
||||
user_factory)
|
||||
from olympia.access.models import Group, GroupUser
|
||||
from olympia.addons.models import Addon, AddonUser
|
||||
from olympia.devhub.models import ActivityLog
|
||||
|
|
@ -593,6 +596,8 @@ class TestMobileReviews(MobileTest, TestCase):
|
|||
|
||||
|
||||
class TestReviewViewSetGet(TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
self.addon = addon_factory(
|
||||
guid=generate_addon_guid(), name=u'My Addôn', slug='my-addon')
|
||||
|
|
@ -631,13 +636,8 @@ class TestReviewViewSetGet(TestCase):
|
|||
def test_list_unknown_addon(self):
|
||||
self.url = reverse(
|
||||
'addon-review-list', kwargs={'addon_pk': self.addon.pk + 42})
|
||||
# We don't bother checking for the add-on existence, so this should
|
||||
# just result in an empty list.
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == 200
|
||||
data = json.loads(response.content)
|
||||
assert data['count'] == 0
|
||||
assert data['results'] == []
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_list_addon_guid(self):
|
||||
self.url = reverse(
|
||||
|
|
@ -730,3 +730,283 @@ class TestReviewViewSetGet(TestCase):
|
|||
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
class TestReviewViewSetPost(TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
self.addon = addon_factory(
|
||||
guid=generate_addon_guid(), name=u'My Addôn', slug='my-addon')
|
||||
self.url = reverse(
|
||||
'addon-review-list', kwargs={'addon_pk': self.addon.pk})
|
||||
|
||||
def test_post_anonymous(self):
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5})
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_post_no_version(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5})
|
||||
assert response.status_code == 400
|
||||
assert response.data['version'] == [u'This field is required.']
|
||||
|
||||
def test_post_version_string(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5,
|
||||
'version': self.addon.current_version.version})
|
||||
assert response.status_code == 400
|
||||
assert response.data['version'] == [
|
||||
'Incorrect type. Expected pk value, received unicode.']
|
||||
|
||||
def test_post_logged_in(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': u'blahé', 'rating': 5,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 201
|
||||
review = Review.objects.latest('pk')
|
||||
assert review.pk == response.data['id']
|
||||
assert unicode(review.body) == response.data['body'] == u'test bodyé'
|
||||
assert review.rating == response.data['rating'] == 5
|
||||
assert review.user == self.user
|
||||
assert unicode(review.title) == response.data['title'] == u'blahé'
|
||||
assert review.reply_to is None
|
||||
assert review.addon == self.addon
|
||||
assert review.version == self.addon.current_version
|
||||
assert response.data['version'] == review.version.version
|
||||
|
||||
def test_post_rating_float(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 4.5,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['rating'] == ['A valid integer is required.']
|
||||
|
||||
def test_post_rating_too_big(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 6,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['rating'] == [
|
||||
'Ensure this value is less than or equal to 5.']
|
||||
|
||||
def test_post_rating_too_low(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 0,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['rating'] == [
|
||||
'Ensure this value is greater than or equal to 1.']
|
||||
|
||||
def test_post_rating_no_title(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 201
|
||||
review = Review.objects.latest('pk')
|
||||
assert review.pk == response.data['id']
|
||||
assert unicode(review.body) == response.data['body'] == u'test bodyé'
|
||||
assert review.rating == response.data['rating'] == 5
|
||||
assert review.user == self.user
|
||||
assert review.title is None
|
||||
assert response.data['title'] is None
|
||||
assert review.reply_to is None
|
||||
assert review.addon == self.addon
|
||||
assert review.version == self.addon.current_version
|
||||
assert response.data['version'] == review.version.version
|
||||
|
||||
def test_no_body_or_title_just_rating(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': None, 'title': None, 'rating': 5,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 201
|
||||
review = Review.objects.latest('pk')
|
||||
assert review.pk == response.data['id']
|
||||
assert review.body is None
|
||||
assert response.data['body'] is None
|
||||
assert review.rating == response.data['rating'] == 5
|
||||
assert review.user == self.user
|
||||
assert review.title is None
|
||||
assert response.data['title'] is None
|
||||
assert review.reply_to is None
|
||||
assert review.addon == self.addon
|
||||
assert review.version == self.addon.current_version
|
||||
assert response.data['version'] == review.version.version
|
||||
|
||||
def test_omit_body_and_title_completely_just_rating(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'rating': 5, 'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 201
|
||||
review = Review.objects.latest('pk')
|
||||
assert review.pk == response.data['id']
|
||||
assert review.body is None
|
||||
assert response.data['body'] is None
|
||||
assert review.rating == response.data['rating'] == 5
|
||||
assert review.user == self.user
|
||||
assert review.title is None
|
||||
assert response.data['title'] is None
|
||||
assert review.reply_to is None
|
||||
assert review.addon == self.addon
|
||||
assert review.version == self.addon.current_version
|
||||
assert response.data['version'] == review.version.version
|
||||
|
||||
def test_post_rating_rating_required(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['rating'] == ['This field is required.']
|
||||
|
||||
def test_post_no_such_addon_id(self):
|
||||
self.url = reverse(
|
||||
'addon-review-list', kwargs={'addon_pk': self.addon.pk + 42})
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
response = self.client.post(self.url, {
|
||||
'body': 'test body', 'title': None, 'rating': 5,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_post_version_not_linked_to_the_right_addon(self):
|
||||
addon2 = addon_factory()
|
||||
self.url = reverse(
|
||||
'addon-review-list', kwargs={'addon_pk': self.addon.pk})
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
response = self.client.post(self.url, {
|
||||
'body': 'test body', 'title': None, 'rating': 5,
|
||||
'version': addon2.current_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['version'] == [
|
||||
'Version does not exist on this add-on or is not public.']
|
||||
|
||||
def test_post_deleted_addon(self):
|
||||
version_pk = self.addon.current_version.pk
|
||||
self.addon.delete()
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5,
|
||||
'version': version_pk})
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_post_deleted_version(self):
|
||||
old_version_pk = self.addon.current_version.pk
|
||||
old_version = self.addon.current_version
|
||||
new_version = version_factory(addon=self.addon)
|
||||
old_version.delete()
|
||||
# Just in case, make sure the add-on is still public.
|
||||
self.addon.reload()
|
||||
assert self.addon.current_version == new_version
|
||||
assert self.addon.status
|
||||
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5,
|
||||
'version': old_version_pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['version'] == [
|
||||
'Version does not exist on this add-on or is not public.']
|
||||
|
||||
def test_post_disabled_version(self):
|
||||
self.addon.current_version.update(created=self.days_ago(1))
|
||||
new_version = version_factory(addon=self.addon)
|
||||
old_version = self.addon.current_version
|
||||
old_version.files.update(status=amo.STATUS_DISABLED)
|
||||
assert self.addon.current_version == new_version
|
||||
assert self.addon.status == amo.STATUS_PUBLIC
|
||||
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5,
|
||||
'version': old_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['version'] == [
|
||||
'Version does not exist on this add-on or is not public.']
|
||||
|
||||
def test_post_not_public_addon(self):
|
||||
version_pk = self.addon.current_version.pk
|
||||
self.addon.update(status=amo.STATUS_NULL)
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5,
|
||||
'version': version_pk})
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_post_logged_in_but_is_addon_author(self):
|
||||
self.user = user_factory()
|
||||
self.addon.addonuser_set.create(user=self.user)
|
||||
self.client.login_api(self.user)
|
||||
assert not Review.objects.exists()
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'test bodyé', 'title': None, 'rating': 5,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['non_field_errors'] == [
|
||||
'An add-on author can not leave a review on its own add-on.']
|
||||
|
||||
def test_post_twice_different_version(self):
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
Review.objects.create(
|
||||
addon=self.addon, version=self.addon.current_version, rating=1,
|
||||
body='My review', user=self.user)
|
||||
second_version = version_factory(addon=self.addon)
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'My ôther review', 'title': None, 'rating': 2,
|
||||
'version': second_version.pk})
|
||||
assert response.status_code == 201
|
||||
assert Review.objects.count() == 2
|
||||
|
||||
def test_post_twice_same_version(self):
|
||||
# Posting a review more than once for the same version is not allowed.
|
||||
self.user = user_factory()
|
||||
self.client.login_api(self.user)
|
||||
Review.objects.create(
|
||||
addon=self.addon, version=self.addon.current_version, rating=1,
|
||||
body='My review', user=self.user)
|
||||
response = self.client.post(self.url, {
|
||||
'body': u'My ôther review', 'title': None, 'rating': 2,
|
||||
'version': self.addon.current_version.pk})
|
||||
assert response.status_code == 400
|
||||
assert response.data['non_field_errors'] == [
|
||||
'The same user can not leave a review on the same version more'
|
||||
' than once.']
|
||||
|
|
|
|||
|
|
@ -14,8 +14,9 @@ from django.utils.translation import ugettext as _
|
|||
import commonware.log
|
||||
from mobility.decorators import mobile_template
|
||||
from rest_framework.exceptions import ParseError
|
||||
from rest_framework.mixins import ListModelMixin, RetrieveModelMixin
|
||||
from rest_framework.permissions import AllowAny
|
||||
from rest_framework.mixins import (
|
||||
CreateModelMixin, ListModelMixin, RetrieveModelMixin)
|
||||
from rest_framework.permissions import AllowAny, IsAuthenticated
|
||||
from rest_framework.viewsets import GenericViewSet
|
||||
from waffle.decorators import waffle_switch
|
||||
|
||||
|
|
@ -28,11 +29,14 @@ from olympia.amo.utils import render, paginate, send_mail as amo_send_mail
|
|||
from olympia.access import acl
|
||||
from olympia.addons.decorators import addon_view_factory
|
||||
from olympia.addons.models import Addon
|
||||
from olympia.addons.views import AddonViewSet
|
||||
from olympia.api.permissions import ByHttpMethod
|
||||
from olympia.addons.views import AddonChildMixin
|
||||
from olympia.api.permissions import (
|
||||
AllowIfReviewedAndListed, AllowOwner, AnyOf, ByHttpMethod,
|
||||
GroupPermission)
|
||||
|
||||
from .helpers import user_can_delete_review
|
||||
from .models import Review, ReviewFlag, GroupedRating, Spam
|
||||
from .permissions import CanDeleteReviewPermission
|
||||
from .serializers import ReviewSerializer
|
||||
from . import forms
|
||||
|
||||
|
|
@ -321,31 +325,51 @@ def spam(request):
|
|||
dict(buckets=buckets, review_perms=dict(is_admin=True)))
|
||||
|
||||
|
||||
class ReviewViewSet(ListModelMixin, RetrieveModelMixin, GenericViewSet):
|
||||
class ReviewViewSet(AddonChildMixin, CreateModelMixin, ListModelMixin,
|
||||
RetrieveModelMixin, GenericViewSet):
|
||||
permission_classes = [
|
||||
ByHttpMethod({
|
||||
'get': AllowAny,
|
||||
'head': AllowAny,
|
||||
'options': AllowAny, # Needed for CORS.
|
||||
|
||||
# Deletion requires a specific permission check.
|
||||
'delete': CanDeleteReviewPermission,
|
||||
|
||||
# To post a review you just need to be authenticated.
|
||||
'post': IsAuthenticated,
|
||||
|
||||
# To edit a review you need to be the author or be an admin.
|
||||
'patch': AnyOf(AllowOwner, GroupPermission('Addons', 'Edit')),
|
||||
'put': AnyOf(AllowOwner, GroupPermission('Addons', 'Edit')),
|
||||
}),
|
||||
]
|
||||
|
||||
serializer_class = ReviewSerializer
|
||||
queryset = Review.objects.all()
|
||||
|
||||
def get_addon_object(self):
|
||||
# When loading the add-on, pass a specific permission class - the
|
||||
# default from AddonViewSet is too restrictive, we are not modifying
|
||||
# the add-on itself so we don't need all the permission checks it does.
|
||||
return super(ReviewViewSet, self).get_addon_object(
|
||||
permission_classes=[AllowIfReviewedAndListed])
|
||||
|
||||
def check_permissions(self, request):
|
||||
if 'addon_pk' in self.kwargs:
|
||||
# In addition to the regular permission checks that are made, we
|
||||
# need to verify that the add-on exists, is public and listed. Just
|
||||
# loading the addon should be enough to do that, since
|
||||
# AddonChildMixin implementation calls AddonViewSet.get_object().
|
||||
self.get_addon_object()
|
||||
|
||||
# Proceed with the regular permission checks.
|
||||
return super(ReviewViewSet, self).check_permissions(request)
|
||||
|
||||
def filter_queryset(self, qs):
|
||||
if self.action == 'list':
|
||||
# No need to load the actual add-on or user. Just filter the
|
||||
# queryset according to what the addon_pk/user_pk parameter we got
|
||||
# looks like - if we end up with an empty list it's fine.
|
||||
if 'addon_pk' in self.kwargs:
|
||||
addon_identifier = self.kwargs.get('addon_pk')
|
||||
lookup_field = AddonViewSet(
|
||||
request=self.request,
|
||||
kwargs={'pk': self.kwargs['addon_pk']}
|
||||
).get_lookup_field(addon_identifier)
|
||||
qs = qs.filter(
|
||||
**{'addon__%s' % lookup_field: addon_identifier})
|
||||
qs = qs.filter(addon=self.get_addon_object())
|
||||
elif 'account_pk' in self.kwargs:
|
||||
qs = qs.filter(user=self.kwargs.get('account_pk'))
|
||||
else:
|
||||
|
|
@ -365,5 +389,7 @@ class ReviewViewSet(ListModelMixin, RetrieveModelMixin, GenericViewSet):
|
|||
qs = super(ReviewViewSet, self).get_queryset()
|
||||
# The serializer needs user, reply and version, so use
|
||||
# prefetch_related() to avoid extra queries (avoid select_related() as
|
||||
# we need crazy joins already)
|
||||
return qs.prefetch_related('reply', 'user', 'version')
|
||||
# we need crazy joins already). Also avoid loading addon since we don't
|
||||
# need it, we already loaded it for permission checks through the pk
|
||||
# specified in the URL.
|
||||
return qs.defer('addon').prefetch_related('reply', 'user', 'version')
|
||||
|
|
|
|||
|
|
@ -81,7 +81,8 @@ class BaseUploadVersionCase(SigningAPITestCase):
|
|||
|
||||
return getattr(self.client, method.lower())(
|
||||
url, data,
|
||||
HTTP_AUTHORIZATION=self.authorization())
|
||||
HTTP_AUTHORIZATION=self.authorization(),
|
||||
format='multipart')
|
||||
|
||||
def make_admin(self, user):
|
||||
admin_group = Group.objects.create(name='Admin', rules='*:*')
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче