Implement new `canned_response` relationship in draft comments. (#12156)
* Implement new `canned_response` relationship in draft comments. This also updates the api changelog along the way with a few things I forgot to add in the past. Fixes #11320 Fixes #11807 * Fix flake8 * Add missing migration
This commit is contained in:
Christopher Grebs
GitHub
Родитель
b63dff900d
Коммит
f5653ba68a
|
|
@ -321,6 +321,7 @@ 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,v5}/github api entirely. They have been marked as experimental. https://github.com/mozilla/addons-server/issues/10411
|
||||
* 2019-02-21: added new /api/v4/reviewers/addon/(addon_id)/versions/ endpoint. https://github.com/mozilla/addons-server/issues/10432
|
||||
* 2019-03-14: added new /reviewers/compare/ endpoint. https://github.com/mozilla/addons-server/issues/10323
|
||||
* 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
|
||||
* 2019-05-09: added ``is_recommended`` to addons API. https://github.com/mozilla/addons-server/issues/11278
|
||||
|
|
@ -329,6 +330,7 @@ v4 API changelog
|
|||
* 2019-05-23: changed the addons search API default sort when no query string is passed - now ``sort=recommended,downloads``.
|
||||
Also made ``recommended`` sort available generally to the addons search API. https://github.com/mozilla/addons-server/issues/11432
|
||||
* 2019-06-27: removed ``sort`` parameter from addon autocomplete API. https://github.com/mozilla/addons-server/issues/11664
|
||||
* 2019-07-18: completely changed the 2019-05-16 added draft-comment related APIs. See `#11380`_, `#11379`_, `#11378`_ and `#11374`_
|
||||
* 2019-07-25: added /hero/ endpoint to expose recommended addons and other content to frontend to allow customizable promos https://github.com/mozilla/addons-server/issues/11842.
|
||||
* 2019-08-01: added alias ``edition=MozillaOnline`` for ``edition=china`` in /discovery/ endpoint.
|
||||
* 2019-08-08: add support for externally hosted addons to /hero/ endpoints. https://github.com/mozilla/addons-server/issues/11882
|
||||
|
|
@ -337,6 +339,13 @@ v4 API changelog
|
|||
* 2019-08-15: dropped support for LWT specific statuses.
|
||||
* 2019-08-15: added promo modules to secondary hero shelves. https://github.com/mozilla/addons-server/issues/11780
|
||||
* 2019-08-15: removed /addons/compat-override/ from v4 and above. Still exists in /v3/ but will always return an empty response. https://github.com/mozilla/addons-server/issues/12063
|
||||
* 2019-08-22: added ``canned_response`` property to draft comment api. https://github.com/mozilla/addons-server/issues/11807
|
||||
|
||||
|
||||
.. _`#11380`: https://github.com/mozilla/addons-server/issues/11380/
|
||||
.. _`#11379`: https://github.com/mozilla/addons-server/issues/11379/
|
||||
.. _`#11378`: https://github.com/mozilla/addons-server/issues/11378/
|
||||
.. _`#11374`: https://github.com/mozilla/addons-server/issues/11374/
|
||||
|
||||
|
||||
----------------
|
||||
|
|
|
|||
|
|
@ -291,6 +291,8 @@ This endpoint allows you to retrieve a list of canned responses.
|
|||
|
||||
.. http:get:: /api/v4/reviewers/canned-responses/
|
||||
|
||||
.. _reviewers-canned-response-detail:
|
||||
|
||||
Retrieve canned responses
|
||||
|
||||
.. note::
|
||||
|
|
@ -340,7 +342,7 @@ These endpoints allow you to draft comments that can be submitted through the re
|
|||
:>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.
|
||||
|
||||
:>json object canned_response: Object holding the :ref:`canned response <reviewers-canned-response-detail>` if set.
|
||||
|
||||
.. http:post:: /api/v4/reviewers/addon/(int:addon_id)/versions/(int:version_id)/draft_comments/
|
||||
|
||||
|
|
@ -349,6 +351,7 @@ These endpoints allow you to draft comments that can be submitted through the re
|
|||
:<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.
|
||||
:<json int draft_comment.id: The id of the draft comment (optional).
|
||||
: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).
|
||||
|
|
@ -369,5 +372,6 @@ These endpoints allow you to draft comments that can be submitted through the re
|
|||
:<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.
|
||||
:<json int draft_comment.id: The id of the draft comment (optional).
|
||||
:statuscode 200: The comment has been updated.
|
||||
:statuscode 400: An error occurred, check the `error` value in the JSON.
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ from olympia.amo.models import ManagerBase, ModelBase
|
|||
from olympia.bandwagon.models import Collection
|
||||
from olympia.files.models import File
|
||||
from olympia.ratings.models import Rating
|
||||
from olympia.reviewers.models import CannedResponse
|
||||
from olympia.tags.models import Tag
|
||||
from olympia.users.models import UserProfile
|
||||
from olympia.users.templatetags.jinja_helpers import user_link
|
||||
|
|
@ -171,7 +172,10 @@ class DraftComment(ModelBase):
|
|||
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()
|
||||
canned_response = models.ForeignKey(
|
||||
CannedResponse, null=True, default=None,
|
||||
on_delete=models.SET_DEFAULT)
|
||||
comment = models.TextField(blank=True)
|
||||
|
||||
class Meta:
|
||||
db_table = 'log_activity_comment_draft'
|
||||
|
|
|
|||
|
|
@ -6,12 +6,14 @@ from pyquery import PyQuery as pq
|
|||
|
||||
from olympia import amo, core
|
||||
from olympia.activity.models import (
|
||||
MAX_TOKEN_USE_COUNT, ActivityLog, ActivityLogToken, AddonLog)
|
||||
MAX_TOKEN_USE_COUNT, ActivityLog, ActivityLogToken, AddonLog,
|
||||
DraftComment)
|
||||
from olympia.addons.models import Addon, AddonUser
|
||||
from olympia.amo.tests import (
|
||||
TestCase, addon_factory, user_factory, version_factory)
|
||||
from olympia.bandwagon.models import Collection
|
||||
from olympia.ratings.models import Rating
|
||||
from olympia.reviewers.models import CannedResponse
|
||||
from olympia.tags.models import Tag
|
||||
from olympia.users.models import UserProfile
|
||||
from olympia.versions.models import Version
|
||||
|
|
@ -413,3 +415,39 @@ class TestActivityLogCount(TestCase):
|
|||
ActivityLog.create(amo.LOG.EDIT_VERSION, Addon.objects.get())
|
||||
assert len(ActivityLog.objects.admin_events()) == 0
|
||||
assert len(ActivityLog.objects.for_developer()) == 1
|
||||
|
||||
|
||||
class TestDraftComment(TestCase):
|
||||
|
||||
def test_default_requirements(self):
|
||||
addon = addon_factory()
|
||||
user = user_factory()
|
||||
# user and version are the absolute minimum required to
|
||||
# create a DraftComment
|
||||
comment = DraftComment.objects.create(
|
||||
user=user, version=addon.current_version)
|
||||
|
||||
assert comment.user == user
|
||||
assert comment.version == addon.current_version
|
||||
assert comment.filename is None
|
||||
assert comment.lineno is None
|
||||
assert comment.canned_response is None
|
||||
assert comment.comment == ''
|
||||
|
||||
def test_canned_response_on_delete(self):
|
||||
addon = addon_factory()
|
||||
user = user_factory()
|
||||
|
||||
canned_response = CannedResponse.objects.create(
|
||||
name=u'Terms of services',
|
||||
response=u'test',
|
||||
category=amo.CANNED_RESPONSE_CATEGORY_OTHER,
|
||||
type=amo.CANNED_RESPONSE_TYPE_ADDON)
|
||||
|
||||
DraftComment.objects.create(
|
||||
user=user, version=addon.current_version,
|
||||
canned_response=canned_response)
|
||||
|
||||
canned_response.delete()
|
||||
|
||||
assert DraftComment.objects.get().canned_response is None
|
||||
|
|
|
|||
|
|
@ -0,0 +1,7 @@
|
|||
ALTER TABLE `log_activity_comment_draft`
|
||||
ADD COLUMN `canned_response_id` integer UNSIGNED DEFAULT NULL;
|
||||
|
||||
ALTER TABLE `log_activity_comment_draft`
|
||||
ADD CONSTRAINT `log_activity_comment_canned_response_id_6a9271d5_fk_cannedres`
|
||||
FOREIGN KEY (`canned_response_id`)
|
||||
REFERENCES `cannedresponses` (`id`);
|
||||
|
|
@ -12,6 +12,7 @@ from rest_framework.exceptions import NotFound
|
|||
from django.utils.functional import cached_property
|
||||
from django.utils.encoding import force_text
|
||||
from django.utils.timezone import FixedOffset
|
||||
from django.utils.translation import ugettext
|
||||
|
||||
from olympia import amo
|
||||
from olympia.activity.models import DraftComment
|
||||
|
|
@ -340,23 +341,6 @@ class AddonCompareVersionSerializer(AddonBrowseVersionSerializer):
|
|||
pass
|
||||
|
||||
|
||||
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 = (
|
||||
'id', 'filename', 'lineno', 'comment',
|
||||
'version', 'user'
|
||||
)
|
||||
|
||||
|
||||
class CannedResponseSerializer(serializers.ModelSerializer):
|
||||
# Title is actually more fitting than the internal "name"
|
||||
title = serializers.CharField(source='name')
|
||||
|
|
@ -368,3 +352,33 @@ class CannedResponseSerializer(serializers.ModelSerializer):
|
|||
|
||||
def get_category(self, obj):
|
||||
return amo.CANNED_RESPONSE_CATEGORY_CHOICES[obj.category]
|
||||
|
||||
|
||||
class DraftCommentSerializer(serializers.ModelSerializer):
|
||||
user = SplitField(
|
||||
serializers.PrimaryKeyRelatedField(queryset=UserProfile.objects.all()),
|
||||
BaseUserSerializer())
|
||||
version = SplitField(
|
||||
serializers.PrimaryKeyRelatedField(
|
||||
queryset=Version.unfiltered.all()),
|
||||
VersionSerializer())
|
||||
canned_response = SplitField(
|
||||
serializers.PrimaryKeyRelatedField(
|
||||
queryset=CannedResponse.objects.all(),
|
||||
required=False),
|
||||
CannedResponseSerializer())
|
||||
|
||||
class Meta:
|
||||
model = DraftComment
|
||||
fields = (
|
||||
'id', 'filename', 'lineno', 'comment',
|
||||
'version', 'user', 'canned_response'
|
||||
)
|
||||
|
||||
def validate(self, data):
|
||||
if data.get('comment') and data.get('canned_response'):
|
||||
raise serializers.ValidationError(
|
||||
{'comment': ugettext(
|
||||
'You can\'t submit a comment if `canned_response` is '
|
||||
'defined.')})
|
||||
return data
|
||||
|
|
|
|||
|
|
@ -53,6 +53,7 @@ from olympia.reviewers.models import (
|
|||
Whiteboard)
|
||||
from olympia.reviewers.utils import ContentReviewTable
|
||||
from olympia.reviewers.views import _queue
|
||||
from olympia.reviewers.serializers import CannedResponseSerializer
|
||||
from olympia.users.models import UserProfile
|
||||
from olympia.versions.models import ApplicationsVersions, AppVersion
|
||||
from olympia.versions.tasks import extract_version_to_git
|
||||
|
|
@ -5743,7 +5744,21 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
},
|
||||
]
|
||||
|
||||
def test_draft_comment_create_and_retrieve(self):
|
||||
|
||||
class TestDraftCommentViewSet(TestCase):
|
||||
client_class = APITestClient
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
|
||||
self.addon = addon_factory(
|
||||
name=u'My Addôn', slug='my-addon',
|
||||
file_kw={'filename': 'webextension_no_id.xpi'})
|
||||
|
||||
self.version = self.addon.current_version
|
||||
self.version.refresh_from_db()
|
||||
|
||||
def test_create_and_retrieve(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
|
@ -5779,6 +5794,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
'filename': 'manifest.json',
|
||||
'lineno': 20,
|
||||
'comment': 'Some really fancy comment',
|
||||
'canned_response': None,
|
||||
'version': json.loads(json.dumps(
|
||||
VersionSerializer(self.version).data,
|
||||
cls=amo.utils.AMOJSONEncoder)),
|
||||
|
|
@ -5788,7 +5804,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
cls=amo.utils.AMOJSONEncoder))
|
||||
}
|
||||
|
||||
def test_draft_comment_create_retrieve_and_update(self):
|
||||
def test_create_retrieve_and_update(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
|
@ -5855,7 +5871,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
assert response.json()['lineno'] == 16
|
||||
assert response.json()['filename'] == 'new_manifest.json'
|
||||
|
||||
def test_draft_comment_lineno_filename_optional(self):
|
||||
def test_draft_optional_fields(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
|
@ -5885,8 +5901,9 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
assert response.json()['comment'] == 'Some really fancy comment'
|
||||
assert response.json()['lineno'] is None
|
||||
assert response.json()['filename'] is None
|
||||
assert response.json()['canned_response'] is None
|
||||
|
||||
def test_draft_comment_delete(self):
|
||||
def test_delete(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
|
@ -5906,7 +5923,87 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
|
||||
assert DraftComment.objects.first() is None
|
||||
|
||||
def test_draft_comment_delete_not_comment_owner(self):
|
||||
def test_canned_response_and_comment_not_together(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
||||
canned_response = CannedResponse.objects.create(
|
||||
name=u'Terms of services',
|
||||
response=u'doesn\'t regard our terms of services',
|
||||
category=amo.CANNED_RESPONSE_CATEGORY_OTHER,
|
||||
type=amo.CANNED_RESPONSE_TYPE_ADDON)
|
||||
|
||||
data = {
|
||||
'comment': 'Some really fancy comment',
|
||||
'canned_response': canned_response.pk,
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
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)
|
||||
assert response.status_code == 400
|
||||
assert (
|
||||
str(response.data['comment'][0]) ==
|
||||
"You can't submit a comment if `canned_response` is defined.")
|
||||
|
||||
def test_canned_response(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
||||
canned_response = CannedResponse.objects.create(
|
||||
name=u'Terms of services',
|
||||
response=u'doesn\'t regard our terms of services',
|
||||
category=amo.CANNED_RESPONSE_CATEGORY_OTHER,
|
||||
type=amo.CANNED_RESPONSE_TYPE_ADDON)
|
||||
|
||||
data = {
|
||||
'canned_response': canned_response.pk,
|
||||
'lineno': 20,
|
||||
'filename': 'manifest.json',
|
||||
}
|
||||
|
||||
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
|
||||
assert DraftComment.objects.count() == 1
|
||||
|
||||
response = self.client.get(url)
|
||||
|
||||
request = APIRequestFactory().get('/')
|
||||
request.user = user
|
||||
|
||||
assert response.json()['count'] == 1
|
||||
assert response.json()['results'][0] == {
|
||||
'id': comment_id,
|
||||
'filename': 'manifest.json',
|
||||
'lineno': 20,
|
||||
'comment': '',
|
||||
'canned_response': json.loads(json.dumps(
|
||||
CannedResponseSerializer(canned_response).data,
|
||||
cls=amo.utils.AMOJSONEncoder)),
|
||||
'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_delete_not_comment_owner(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
|
||||
|
|
@ -5931,7 +6028,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
response = self.client.delete(url)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_disabled_version_user_but_not_author(self):
|
||||
def test_disabled_version_user_but_not_author(self):
|
||||
user = user_factory(username='simpleuser')
|
||||
self.client.login_api(user)
|
||||
self.version.files.update(status=amo.STATUS_DISABLED)
|
||||
|
|
@ -5950,7 +6047,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
response = self.client.post(url, data)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_draft_comment_deleted_version_reviewer(self):
|
||||
def test_deleted_version_reviewer(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
|
@ -5970,7 +6067,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
response = self.client.post(url, data)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_deleted_version_author(self):
|
||||
def test_deleted_version_author(self):
|
||||
user = user_factory(username='author')
|
||||
AddonUser.objects.create(user=user, addon=self.addon)
|
||||
self.client.login_api(user)
|
||||
|
|
@ -5990,7 +6087,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
response = self.client.post(url, data)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_deleted_version_user_but_not_author(self):
|
||||
def test_deleted_version_user_but_not_author(self):
|
||||
user = user_factory(username='simpleuser')
|
||||
self.client.login_api(user)
|
||||
self.version.delete()
|
||||
|
|
@ -6009,7 +6106,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
response = self.client.post(url, data)
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_draft_comment_unlisted_version_reviewer(self):
|
||||
def test_unlisted_version_reviewer(self):
|
||||
user = user_factory(username='reviewer')
|
||||
self.grant_permission(user, 'Addons:Review')
|
||||
self.client.login_api(user)
|
||||
|
|
@ -6029,7 +6126,7 @@ class TestReviewAddonVersionViewSetList(TestCase):
|
|||
response = self.client.post(url, data)
|
||||
assert response.status_code == 403
|
||||
|
||||
def test_draft_comment_unlisted_version_user_but_not_author(self):
|
||||
def test_unlisted_version_user_but_not_author(self):
|
||||
user = user_factory(username='simpleuser')
|
||||
self.client.login_api(user)
|
||||
self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче