Revamp draft comments API. (#11808)
* Completely revamp draft comments API. Revamp how draft comments work, store multiple comments per version and paginate them. This also implement PATCH to update a comment, updates DELETE, GET and POST endpoints properly and updates the documentation accordingly. Fixes #11380 Fixes #11379 Fixes #11378 Fixes #11374 * Fix statuscode in docs * Fix docs, use regular VersionSerializer * Allow filename and lineno to be optional. * Add note about lineno should be specific to the version, not the parent. * Fix test to use regular VersionSerializer * Fix test to test that the 'id' is set properly now * Add more details to pagination, add detail endpoint for comments. * Use user_factory
This commit is contained in:
Родитель
020e8c3b41
Коммит
a34a0b5630
|
@ -112,6 +112,8 @@ The response when returning ``HTTP 503 Service Unavailable`` in case of read-onl
|
|||
|
||||
In case we are not in read-only mode everything should be back working as normal.
|
||||
|
||||
.. _api-overview-pagination:
|
||||
|
||||
~~~~~~~~~~
|
||||
Pagination
|
||||
~~~~~~~~~~
|
||||
|
|
|
@ -97,6 +97,10 @@ This endpoint allows you to list versions that can be used either for :ref:`brow
|
|||
unlisted add-ons. Additionally the current user can also be the owner
|
||||
of the add-on.
|
||||
|
||||
This endpoint is not paginated as normal, and instead will return all
|
||||
results without obeying regular pagination parameters.
|
||||
|
||||
|
||||
If the user doesn't have ``AddonsReviewUnlisted`` permissions only listed versions are shown. Otherwise it can contain mixed listed and unlisted versions.
|
||||
|
||||
.. http:get:: /api/v4/reviewers/addon/(int:addon_id)/versions/
|
||||
|
@ -289,6 +293,11 @@ This endpoint allows you to retrieve a list of canned responses.
|
|||
|
||||
Retrieve canned responses
|
||||
|
||||
.. note::
|
||||
Because this endpoint is not returning too much data it is not
|
||||
paginated as normal, and instead will return all results without
|
||||
obeying regular pagination parameters.
|
||||
|
||||
:>json int id: The canned response id.
|
||||
:>json string title: The title of the canned response.
|
||||
:>json string response: The text that will be filled in as the response.
|
||||
|
@ -301,26 +310,64 @@ Drafting Comments
|
|||
|
||||
These endpoints allow you to draft comments that can be submitted through the regular reviewer pages.
|
||||
|
||||
Please note, that once a review is submitted the drafted comments are being cleared as well.
|
||||
|
||||
.. note::
|
||||
Requires authentication and the current user to have ``ReviewerTools:View``
|
||||
permission for listed add-ons as well as ``Addons:ReviewUnlisted`` for
|
||||
unlisted add-ons. Additionally the current user can also be the owner
|
||||
of the add-on.
|
||||
|
||||
|
||||
.. http:get:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/
|
||||
|
||||
Retrieve an exising draft.
|
||||
Retrieve existing draft comments for a specific version. See :ref:`pagination <api-overview-pagination>` for more details.
|
||||
|
||||
:>json string comments: The comment that is being drafted as part of a review.
|
||||
:>json int count: The number of comments for this version.
|
||||
:>json string next: The URL of the next page of results.
|
||||
:>json string previous: The URL of the previous page of results.
|
||||
:>json array results: An array of :ref:`comments <reviewers-draft-comment-detail-object>`.
|
||||
|
||||
.. http:patch:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/
|
||||
|
||||
Create or update a draft.
|
||||
.. http:get:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/(int:comment_id)/
|
||||
|
||||
:<json string comments: The comment that is being drafted as part of a review.
|
||||
.. _reviewers-draft-comment-detail-object:
|
||||
|
||||
.. http:delete:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/
|
||||
:>json int id: The id of the draft comment object.
|
||||
:>json string comment: The comment that is being drafted as part of a review. Specific to a line in a file.
|
||||
:>json string filename: The filename a specific comment is related to.
|
||||
:>json int lineno: The line number a specific comment is related to. Please make sure that in case of comments for git diffs, that the `lineno` used here belongs to the file in the version that belongs to `version_id` and not it's parent.
|
||||
:>json object version: Object holding the :ref:`version <version-detail-object>`.
|
||||
:>json int user.id: The id for an author.
|
||||
:>json string user.name: The name for an author.
|
||||
:>json string user.url: The link to the profile page for an author.
|
||||
:>json string user.username: The username for an author.
|
||||
|
||||
Delete a drafted comment.
|
||||
|
||||
.. http:post:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/
|
||||
|
||||
Create a draft comment for a specific version.
|
||||
|
||||
:<json string comment: The comment that is being drafted as part of a review.
|
||||
:<json string filename: The filename this comment is related to (optional).
|
||||
:<json int lineno: The line number this comment is related to (optional). Please make sure that in case of comments for git diffs, that the `lineno` used here belongs to the file in the version that belongs to `version_id` and not it's parent.
|
||||
:statuscode 201: New comment has been created.
|
||||
:statuscode 400: An error occurred, check the `error` value in the JSON.
|
||||
:statuscode 403: The user doesn't have the permission to create a comment. This might happen (among other cases) when someone without permissions for unlisted versions tries to add a comment for an unlisted version (which shouldn't happen as the user doesn't see unlisted versions, but it's blocked here too).
|
||||
|
||||
|
||||
.. http:delete:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/(int:comment_id)/
|
||||
|
||||
Delete a draft comment.
|
||||
|
||||
:statuscode 204: The comment has been deleted successfully.
|
||||
:statuscode 404: The user doesn't have the permission to delete. This might happen when someone tries to delete a comment created by another reviewer or author.
|
||||
|
||||
|
||||
.. http:patch:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/(int:comment_id)
|
||||
|
||||
Update a comment, it's filename or line number.
|
||||
|
||||
:<json string comment: The comment that is being drafted as part of a review.
|
||||
:<json string filename: The filename this comment is related to.
|
||||
:<json int lineno: The line number this comment is related to. Please make sure that in case of comments for git diffs, that the `lineno` used here belongs to the file in the version that belongs to `version_id` and not it's parent.
|
||||
:statuscode 200: The comment has been updated.
|
||||
:statuscode 400: An error occurred, check the `error` value in the JSON.
|
||||
|
|
|
@ -169,9 +169,11 @@ class DraftComment(ModelBase):
|
|||
This is being used by the commenting API by the code-manager.
|
||||
"""
|
||||
id = PositiveAutoField(primary_key=True)
|
||||
comments = models.TextField()
|
||||
version = models.ForeignKey(Version, on_delete=models.CASCADE)
|
||||
user = models.ForeignKey(UserProfile, on_delete=models.CASCADE)
|
||||
filename = models.CharField(max_length=255, null=True, blank=True)
|
||||
lineno = models.PositiveIntegerField(null=True)
|
||||
comment = models.TextField()
|
||||
|
||||
class Meta:
|
||||
db_table = 'log_activity_comment_draft'
|
||||
|
|
|
@ -1483,6 +1483,7 @@ PS_BIN = '/bin/ps'
|
|||
|
||||
# The maximum file size that is shown inside the file viewer.
|
||||
FILE_VIEWER_SIZE_LIMIT = 1048576
|
||||
|
||||
# The maximum file size that you can have inside a zip file.
|
||||
FILE_UNZIP_SIZE_LIMIT = 104857600
|
||||
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
ALTER TABLE `log_activity_comment_draft`
|
||||
ADD COLUMN `filename` varchar(255) NULL,
|
||||
ADD COLUMN `lineno` integer UNSIGNED NULL,
|
||||
CHANGE `comments` `comment` longtext NOT NULL;
|
|
@ -5,7 +5,8 @@ from rest_framework_nested.routers import NestedSimpleRouter
|
|||
|
||||
from .views import (
|
||||
AddonReviewerViewSet, ReviewAddonVersionViewSet,
|
||||
ReviewAddonVersionCompareViewSet, CannedResponseViewSet)
|
||||
ReviewAddonVersionCompareViewSet, CannedResponseViewSet,
|
||||
ReviewAddonVersionDraftCommentViewSet)
|
||||
|
||||
|
||||
addons = SimpleRouter()
|
||||
|
@ -20,10 +21,16 @@ compare.register(
|
|||
r'compare_to', ReviewAddonVersionCompareViewSet,
|
||||
basename='reviewers-versions-compare')
|
||||
|
||||
draft_comments = NestedSimpleRouter(versions, r'versions', lookup='version')
|
||||
draft_comments.register(
|
||||
r'draft_comments', ReviewAddonVersionDraftCommentViewSet,
|
||||
basename='reviewers-versions-draft-comment')
|
||||
|
||||
urlpatterns = [
|
||||
url(r'', include(addons.urls)),
|
||||
url(r'', include(versions.urls)),
|
||||
url(r'', include(compare.urls)),
|
||||
url(r'', include(draft_comments.urls)),
|
||||
url(r'^canned-responses/$', CannedResponseViewSet.as_view(),
|
||||
name='reviewers-canned-response-list'),
|
||||
]
|
||||
|
|
|
@ -376,9 +376,6 @@ class ReviewForm(forms.Form):
|
|||
self.fields['action'].choices = [
|
||||
(k, v['label']) for k, v in self.helper.actions.items()]
|
||||
|
||||
if self.initial.get('comments_draft'):
|
||||
self.fields['comments'].initial = self.initial['comments_draft']
|
||||
|
||||
@property
|
||||
def unreviewed_files(self):
|
||||
return (self.helper.version.unreviewed_files
|
||||
|
|
|
@ -15,11 +15,14 @@ from django.utils.timezone import FixedOffset
|
|||
|
||||
from olympia import amo
|
||||
from olympia.activity.models import DraftComment
|
||||
from olympia.accounts.serializers import BaseUserSerializer
|
||||
from olympia.amo.urlresolvers import reverse
|
||||
from olympia.amo.templatetags.jinja_helpers import absolutify
|
||||
from olympia.addons.serializers import (
|
||||
VersionSerializer, FileSerializer, SimpleAddonSerializer)
|
||||
from olympia.addons.models import AddonReviewerFlags
|
||||
from olympia.api.fields import SplitField
|
||||
from olympia.users.models import UserProfile
|
||||
from olympia.files.utils import get_sha256
|
||||
from olympia.files.models import File
|
||||
from olympia.reviewers.models import CannedResponse
|
||||
|
@ -332,10 +335,20 @@ class AddonCompareVersionSerializer(AddonBrowseVersionSerializer):
|
|||
|
||||
|
||||
class DraftCommentSerializer(serializers.ModelSerializer):
|
||||
user = SplitField(
|
||||
serializers.PrimaryKeyRelatedField(queryset=UserProfile.objects.all()),
|
||||
BaseUserSerializer())
|
||||
version = SplitField(
|
||||
serializers.PrimaryKeyRelatedField(
|
||||
queryset=Version.unfiltered.all()),
|
||||
VersionSerializer())
|
||||
|
||||
class Meta:
|
||||
model = DraftComment
|
||||
fields = ('comments',)
|
||||
fields = (
|
||||
'id', 'filename', 'lineno', 'comment',
|
||||
'version', 'user'
|
||||
)
|
||||
|
||||
|
||||
class CannedResponseSerializer(serializers.ModelSerializer):
|
||||
|
|
|
@ -17,6 +17,8 @@ from django.db import connection, reset_queries
|
|||
from django.test.client import RequestFactory
|
||||
from django.test.utils import override_settings
|
||||
|
||||
from rest_framework.test import APIRequestFactory
|
||||
|
||||
import pytest
|
||||
import six
|
||||
|
||||
|
@ -30,9 +32,11 @@ from olympia import amo, core, ratings
|
|||
from olympia.abuse.models import AbuseReport
|
||||
from olympia.access.models import Group, GroupUser
|
||||
from olympia.accounts.views import API_TOKEN_COOKIE
|
||||
from olympia.accounts.serializers import BaseUserSerializer
|
||||
from olympia.activity.models import ActivityLog, DraftComment
|
||||
from olympia.addons.models import (
|
||||
Addon, AddonApprovalsCounter, AddonReviewerFlags, AddonUser)
|
||||
from olympia.addons.serializers import VersionSerializer
|
||||
from olympia.amo.storage_utils import copy_stored_file
|
||||
from olympia.amo.templatetags.jinja_helpers import (
|
||||
absolutify, format_date, format_datetime)
|
||||
|
@ -4610,38 +4614,6 @@ class TestReview(ReviewBase):
|
|||
args=[self.addon.current_version.id])
|
||||
)
|
||||
|
||||
@patch('olympia.reviewers.utils.sign_file')
|
||||
def test_draft_comments_prefilled_for_reply(self, mock_sign_file):
|
||||
DraftComment.objects.create(
|
||||
comments='Fancy pancy review',
|
||||
version=self.addon.current_version,
|
||||
user=self.reviewer)
|
||||
|
||||
response = self.client.get(self.url)
|
||||
doc = pq(response.content)
|
||||
assert doc('#id_comments').text() == 'Fancy pancy review'
|
||||
|
||||
@patch('olympia.reviewers.utils.sign_file')
|
||||
def test_draft_comments_cleared_after_submission(self, mock_sign_file):
|
||||
DraftComment.objects.create(
|
||||
comments='Fancy pancy review',
|
||||
version=self.addon.current_version,
|
||||
user=self.reviewer)
|
||||
|
||||
response = self.client.post(self.url, {
|
||||
'action': 'reply',
|
||||
'comments': 'foobar'
|
||||
})
|
||||
|
||||
assert response.status_code == 302
|
||||
assert len(mail.outbox) == 1
|
||||
|
||||
# Draft comments don't take precedence over manual comments
|
||||
assert 'foobar' in mail.outbox[0].body
|
||||
self.assertTemplateUsed(response, 'activity/emails/from_reviewer.txt')
|
||||
|
||||
assert DraftComment.objects.count() == 0
|
||||
|
||||
|
||||
@override_flag('code-manager', active=True)
|
||||
class TestCodeManagerLinks(ReviewBase):
|
||||
|
@ -5754,48 +5726,162 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
},
|
||||
]
|
||||
|
||||
def test_draft_comment_patch_not_allowed(self):
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.patch(url, {})
|
||||
assert response.status_code == 405
|
||||
|
||||
def test_draft_comment(self):
|
||||
user = UserProfile.objects.create(username='reviewer')
|
||||
def test_draft_comment_create_and_retrieve(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
||||
data = {
|
||||
'comments': 'Some really fancy comment',
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.put(url, data)
|
||||
assert response.status_code == 200
|
||||
response = self.client.post(url, data)
|
||||
comment_id = response.json()['id']
|
||||
assert response.status_code == 201
|
||||
|
||||
draft_comment = DraftComment.objects.get()
|
||||
assert draft_comment.version == self.version
|
||||
assert draft_comment.comments == 'Some really fancy comment'
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 201
|
||||
|
||||
def test_draft_comment_delete(self):
|
||||
user = UserProfile.objects.create(username='reviewer')
|
||||
assert DraftComment.objects.count() == 2
|
||||
|
||||
response = self.client.get(url)
|
||||
|
||||
request = APIRequestFactory().get('/')
|
||||
request.user = user
|
||||
|
||||
assert response.json()['count'] == 2
|
||||
assert response.json()['results'][0] == {
|
||||
'id': comment_id,
|
||||
'filename': 'manifest.json',
|
||||
'lineno': 20,
|
||||
'comment': 'Some really fancy comment',
|
||||
'version': json.loads(json.dumps(
|
||||
VersionSerializer(self.version).data,
|
||||
cls=amo.utils.AMOJSONEncoder)),
|
||||
'user': json.loads(json.dumps(
|
||||
BaseUserSerializer(
|
||||
user, context={'request': request}).data,
|
||||
cls=amo.utils.AMOJSONEncoder))
|
||||
}
|
||||
|
||||
def test_draft_comment_create_retrieve_and_update(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
||||
DraftComment.objects.create(
|
||||
version=self.version, comments='test',
|
||||
user=user)
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 201
|
||||
|
||||
comment = DraftComment.objects.first()
|
||||
|
||||
response = self.client.get(url)
|
||||
|
||||
assert response.json()['count'] == 1
|
||||
assert (
|
||||
response.json()['results'][0]['comment'] ==
|
||||
'Some really fancy comment')
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-detail', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'version_pk': self.version.pk,
|
||||
'pk': comment.pk
|
||||
})
|
||||
|
||||
response = self.client.patch(url, {
|
||||
'comment': 'Updated comment!'
|
||||
})
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
response = self.client.get(url)
|
||||
|
||||
assert response.json()['comment'] == 'Updated comment!'
|
||||
assert response.json()['lineno'] == 20
|
||||
|
||||
response = self.client.patch(url, {
|
||||
'lineno': 18
|
||||
})
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
response = self.client.get(url)
|
||||
|
||||
assert response.json()['lineno'] == 18
|
||||
|
||||
# Patch two fields at the same time
|
||||
response = self.client.patch(url, {
|
||||
'lineno': 16,
|
||||
'filename': 'new_manifest.json'
|
||||
})
|
||||
|
||||
assert response.status_code == 200
|
||||
response = self.client.get(url)
|
||||
|
||||
assert response.json()['lineno'] == 16
|
||||
assert response.json()['filename'] == 'new_manifest.json'
|
||||
|
||||
def test_draft_comment_lineno_filename_optional(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.post(url, data)
|
||||
comment_id = response.json()['id']
|
||||
|
||||
assert response.status_code == 201
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-detail', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'version_pk': self.version.pk,
|
||||
'pk': comment_id
|
||||
})
|
||||
|
||||
response = self.client.get(url)
|
||||
|
||||
assert response.json()['comment'] == 'Some really fancy comment'
|
||||
assert response.json()['lineno'] is None
|
||||
assert response.json()['filename'] is None
|
||||
|
||||
def test_draft_comment_delete(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
||||
comment = DraftComment.objects.create(
|
||||
version=self.version, comment='test', user=user,
|
||||
lineno=0, filename='manifest.json')
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-detail', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'version_pk': self.version.pk,
|
||||
'pk': comment.pk
|
||||
})
|
||||
|
||||
response = self.client.delete(url)
|
||||
|
@ -5804,109 +5890,146 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
assert DraftComment.objects.first() is None
|
||||
|
||||
def test_draft_comment_delete_not_comment_owner(self):
|
||||
user = UserProfile.objects.create(username='reviewer')
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
|
||||
DraftComment.objects.create(
|
||||
version=self.version, comments='test',
|
||||
user=user)
|
||||
comment = DraftComment.objects.create(
|
||||
version=self.version, comment='test', user=user,
|
||||
lineno=0, filename='manifest.json')
|
||||
|
||||
# Let's login as someone else who is also a reviewer
|
||||
other_reviewer = UserProfile.objects.create(username='reviewer2')
|
||||
other_reviewer = user_factory(username='reviewer2')
|
||||
|
||||
# Let's give the user admin permissions which doesn't help
|
||||
self.grant_permission(other_reviewer, '*:*')
|
||||
|
||||
self.client.login_api(other_reviewer)
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
url = reverse_ns('reviewers-versions-draft-comment-detail', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk,
|
||||
'pk': comment.pk
|
||||
})
|
||||
|
||||
response = self.client.delete(url)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_disabled_version_user_but_not_author(self):
|
||||
user = UserProfile.objects.create(username='simpleuser')
|
||||
user = user_factory(username='simpleuser')
|
||||
self.client.login_api(user)
|
||||
self.version.files.update(status=amo.STATUS_DISABLED)
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.put(url, {'comments': 'test'})
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_draft_comment_deleted_version_reviewer(self):
|
||||
user = UserProfile.objects.create(username='reviewer')
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
self.version.delete()
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.put(url, {'comments': 'test'})
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_deleted_version_author(self):
|
||||
user = UserProfile.objects.create(username='author')
|
||||
user = user_factory(username='author')
|
||||
AddonUser.objects.create(user=user, addon=self.addon)
|
||||
self.client.login_api(user)
|
||||
self.version.delete()
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.put(url, {'comments': 'test'})
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_deleted_version_user_but_not_author(self):
|
||||
user = UserProfile.objects.create(username='simpleuser')
|
||||
user = user_factory(username='simpleuser')
|
||||
self.client.login_api(user)
|
||||
self.version.delete()
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.put(url, {'comments': 'test'})
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_unlisted_version_reviewer(self):
|
||||
user = UserProfile.objects.create(username='reviewer')
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.put(url, {'comments': 'test'})
|
||||
assert response.status_code == 404
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_draft_comment_unlisted_version_user_but_not_author(self):
|
||||
user = UserProfile.objects.create(username='simpleuser')
|
||||
user = user_factory(username='simpleuser')
|
||||
self.client.login_api(user)
|
||||
self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment', kwargs={
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
url = reverse_ns('reviewers-versions-draft-comment-list', kwargs={
|
||||
'addon_pk': self.addon.pk,
|
||||
'pk': self.version.pk
|
||||
'version_pk': self.version.pk
|
||||
})
|
||||
|
||||
response = self.client.put(url, {'comments': 'test'})
|
||||
assert response.status_code == 404
|
||||
response = self.client.post(url, data)
|
||||
assert response.status_code == 403
|
||||
|
||||
|
||||
class TestReviewAddonVersionCompareViewSet(
|
||||
|
|
|
@ -24,7 +24,9 @@ from rest_framework.decorators import action
|
|||
from rest_framework.response import Response
|
||||
from rest_framework.viewsets import GenericViewSet
|
||||
from rest_framework.generics import ListAPIView
|
||||
from rest_framework.mixins import ListModelMixin, RetrieveModelMixin
|
||||
from rest_framework.mixins import (
|
||||
ListModelMixin, RetrieveModelMixin, CreateModelMixin, DestroyModelMixin,
|
||||
UpdateModelMixin)
|
||||
|
||||
import olympia.core.logger
|
||||
|
||||
|
@ -44,8 +46,7 @@ from olympia.amo.urlresolvers import reverse
|
|||
from olympia.amo.utils import paginate, render
|
||||
from olympia.api.permissions import (
|
||||
AllowAnyKindOfReviewer, GroupPermission,
|
||||
AllowAddonAuthor, AllowReviewer, AllowReviewerUnlisted, AnyOf,
|
||||
ByHttpMethod)
|
||||
AllowAddonAuthor, AllowReviewer, AllowReviewerUnlisted, AnyOf)
|
||||
from olympia.constants.reviewers import REVIEWS_PER_PAGE, REVIEWS_PER_PAGE_MAX
|
||||
from olympia.devhub import tasks as devhub_tasks
|
||||
from olympia.discovery.models import DiscoveryItem
|
||||
|
@ -789,14 +790,6 @@ def review(request, addon, channel=None):
|
|||
'info_request': addon.pending_info_request,
|
||||
}
|
||||
|
||||
try:
|
||||
comments_draft = version.draftcomment_set.get(user=request.user)
|
||||
except (DraftComment.DoesNotExist, AttributeError):
|
||||
comments_draft = None
|
||||
|
||||
form_initial['comments_draft'] = (
|
||||
comments_draft.comments if comments_draft else '')
|
||||
|
||||
form_helper = ReviewHelper(
|
||||
request=request, addon=addon, version=version,
|
||||
content_review_only=content_review_only)
|
||||
|
@ -837,9 +830,6 @@ def review(request, addon, channel=None):
|
|||
if request.method == 'POST' and form.is_valid():
|
||||
form.helper.process()
|
||||
|
||||
if comments_draft:
|
||||
comments_draft.delete()
|
||||
|
||||
amo.messages.success(
|
||||
request, ugettext('Review successfully processed.'))
|
||||
clear_reviewing_cache(addon.id)
|
||||
|
@ -1329,7 +1319,7 @@ class ReviewAddonVersionMixin(object):
|
|||
|
||||
kwargs.setdefault(
|
||||
self.lookup_field,
|
||||
self.kwargs[self.lookup_url_kwarg or self.lookup_field])
|
||||
self.kwargs.get(self.lookup_url_kwarg or self.lookup_field))
|
||||
|
||||
obj = get_object_or_404(qset, **kwargs)
|
||||
|
||||
|
@ -1392,40 +1382,72 @@ class ReviewAddonVersionViewSet(ReviewAddonVersionMixin, ListModelMixin,
|
|||
)
|
||||
return Response(serializer.data)
|
||||
|
||||
draft_comment_permissions = AnyOf(
|
||||
|
||||
class ReviewAddonVersionDraftCommentViewSet(
|
||||
RetrieveModelMixin, ListModelMixin, CreateModelMixin,
|
||||
DestroyModelMixin, UpdateModelMixin, GenericViewSet):
|
||||
|
||||
permission_classes = [AnyOf(
|
||||
AllowReviewer, AllowReviewerUnlisted, AllowAddonAuthor,
|
||||
)
|
||||
)]
|
||||
|
||||
@action(
|
||||
detail=True,
|
||||
methods=['get', 'put', 'delete'],
|
||||
permission_classes=[ByHttpMethod({
|
||||
'get': draft_comment_permissions,
|
||||
'put': draft_comment_permissions,
|
||||
'delete': draft_comment_permissions})
|
||||
])
|
||||
def draft_comment(self, request, **kwargs):
|
||||
version = self.get_object()
|
||||
queryset = DraftComment.objects.all()
|
||||
serializer_class = DraftCommentSerializer
|
||||
|
||||
if request.method == 'GET':
|
||||
instance = get_object_or_404(
|
||||
DraftComment, version=version, user=request.user)
|
||||
serializer = DraftCommentSerializer(instance, data=request.data)
|
||||
return Response(serializer.data)
|
||||
elif request.method == 'DELETE':
|
||||
instance = get_object_or_404(
|
||||
DraftComment, version=version, user=request.user)
|
||||
def check_object_permissions(self, request, obj):
|
||||
"""Check permissions against the parent add-on object."""
|
||||
return super().check_object_permissions(request, obj.version.addon)
|
||||
|
||||
instance.delete()
|
||||
return Response(status=status.HTTP_204_NO_CONTENT)
|
||||
elif request.method == 'PUT':
|
||||
instance, _ = DraftComment.objects.get_or_create(
|
||||
version=version, user=request.user)
|
||||
serializer = DraftCommentSerializer(instance, data=request.data)
|
||||
serializer.is_valid(raise_exception=True)
|
||||
serializer.save()
|
||||
return Response(serializer.data)
|
||||
return Response(status=http.HTTP_405_METHOD_NOT_ALLOWED)
|
||||
def _verify_object_permissions(self, object_to_verify, version):
|
||||
"""Verify permissions.
|
||||
|
||||
This method works for `Version` and `DraftComment` objects.
|
||||
"""
|
||||
# If the instance is marked as deleted and the client is not allowed to
|
||||
# see deleted instances, we want to return a 404, behaving as if it
|
||||
# does not exist.
|
||||
if version.deleted and not (
|
||||
GroupPermission(amo.permissions.ADDONS_VIEW_DELETED).
|
||||
has_object_permission(self.request, self, version.addon)):
|
||||
raise http.Http404
|
||||
|
||||
# Now we can checking permissions
|
||||
super().check_object_permissions(self.request, version.addon)
|
||||
|
||||
def get_object(self, **kwargs):
|
||||
qset = self.filter_queryset(self.get_queryset())
|
||||
|
||||
kwargs.setdefault(
|
||||
self.lookup_field,
|
||||
self.kwargs.get(self.lookup_url_kwarg or self.lookup_field))
|
||||
|
||||
obj = get_object_or_404(qset, **kwargs)
|
||||
self._verify_object_permissions(obj, obj.version)
|
||||
return obj
|
||||
|
||||
def get_version_object(self):
|
||||
version = get_object_or_404(
|
||||
Version.objects.get_queryset().only_translations(),
|
||||
pk=self.kwargs['version_pk'])
|
||||
self._verify_object_permissions(version, version)
|
||||
return version
|
||||
|
||||
def get_extra_comment_data(self):
|
||||
return {
|
||||
'version': self.get_version_object().pk,
|
||||
'user': self.request.user.pk
|
||||
}
|
||||
|
||||
def filter_queryset(self, qset):
|
||||
qset = super().filter_queryset(qset)
|
||||
return qset.filter(**self.get_extra_comment_data())
|
||||
|
||||
def get_serializer_context(self):
|
||||
context = super().get_serializer_context()
|
||||
# Patch in `version` and `user` as those are required by the serializer
|
||||
# and not provided by the API client as part of the POST data.
|
||||
self.request.data.update(self.get_extra_comment_data())
|
||||
return context
|
||||
|
||||
|
||||
class ReviewAddonVersionCompareViewSet(ReviewAddonVersionMixin,
|
||||
|
|
Загрузка…
Ссылка в новой задаче