[PATCH v2 2/5] tests: add tests for patch comments detail endpoint

Raxel Gutierrez raxel at google.com
Thu Jul 29 04:17:15 AEST 2021


Add tests for the new api/.../comments/<comment_id> endpoint that takes
GET, PATCH, and PUT requests. The tests cover retrieval and update
requests and handle calls from the various API versions. Also, they
handle permissions for update requests on the new `addressed` field and
invalid update values for the `addressed` field.

Add `addressed` field to create_patch_comment helper in api tests
utils.py.

Signed-off-by: Raxel Gutierrez <raxel at google.com>
---
 patchwork/tests/api/test_comment.py | 199 +++++++++++++++++++++++++---
 patchwork/tests/utils.py            |   1 +
 2 files changed, 183 insertions(+), 17 deletions(-)

diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
index 59450d8..f43d1c7 100644
--- a/patchwork/tests/api/test_comment.py
+++ b/patchwork/tests/api/test_comment.py
@@ -9,11 +9,16 @@ from django.conf import settings
 from django.urls import NoReverseMatch
 from django.urls import reverse
 
+from patchwork.models import PatchComment
 from patchwork.tests.api import utils
 from patchwork.tests.utils import create_cover
 from patchwork.tests.utils import create_cover_comment
 from patchwork.tests.utils import create_patch
 from patchwork.tests.utils import create_patch_comment
+from patchwork.tests.utils import create_maintainer
+from patchwork.tests.utils import create_project
+from patchwork.tests.utils import create_person
+from patchwork.tests.utils import create_user
 from patchwork.tests.utils import SAMPLE_CONTENT
 
 if settings.ENABLE_REST_API:
@@ -86,34 +91,40 @@ class TestCoverComments(utils.APITestCase):
 @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
 class TestPatchComments(utils.APITestCase):
     @staticmethod
-    def api_url(patch, version=None):
-        kwargs = {}
+    def api_url(patch, version=None, item=None):
+        kwargs = {'patch_id': patch.id}
         if version:
             kwargs['version'] = version
-        kwargs['patch_id'] = patch.id
+        if item is None:
+            return reverse('api-patch-comment-list', kwargs=kwargs)
+        kwargs['comment_id'] = item.id
+        return reverse('api-patch-comment-detail', kwargs=kwargs)
 
-        return reverse('api-patch-comment-list', kwargs=kwargs)
+    def setUp(self):
+        super(TestPatchComments, self).setUp()
+        self.project = create_project()
+        self.user = create_maintainer(self.project)
+        self.patch = create_patch(project=self.project)
 
     def assertSerialized(self, comment_obj, comment_json):
         self.assertEqual(comment_obj.id, comment_json['id'])
         self.assertEqual(comment_obj.submitter.id,
                          comment_json['submitter']['id'])
+        self.assertEqual(comment_obj.addressed, comment_json['addressed'])
         self.assertIn(SAMPLE_CONTENT, comment_json['content'])
 
     def test_list_empty(self):
         """List patch comments when none are present."""
-        patch = create_patch()
-        resp = self.client.get(self.api_url(patch))
+        resp = self.client.get(self.api_url(self.patch))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(0, len(resp.data))
 
     @utils.store_samples('patch-comment-list')
     def test_list(self):
         """List patch comments."""
-        patch = create_patch()
-        comment = create_patch_comment(patch=patch)
+        comment = create_patch_comment(patch=self.patch)
 
-        resp = self.client.get(self.api_url(patch))
+        resp = self.client.get(self.api_url(self.patch))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
         self.assertSerialized(comment, resp.data[0])
@@ -121,26 +132,180 @@ class TestPatchComments(utils.APITestCase):
 
     def test_list_version_1_1(self):
         """List patch comments using API v1.1."""
-        patch = create_patch()
-        comment = create_patch_comment(patch=patch)
+        comment = create_patch_comment(patch=self.patch)
 
-        resp = self.client.get(self.api_url(patch, version='1.1'))
+        resp = self.client.get(self.api_url(self.patch, version='1.1'))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
         self.assertSerialized(comment, resp.data[0])
         self.assertNotIn('list_archive_url', resp.data[0])
 
     def test_list_version_1_0(self):
-        """List patch comments using API v1.0."""
-        patch = create_patch()
-        create_patch_comment(patch=patch)
+        """List patch comments using API v1.0.
 
-        # check we can't access comments using the old version of the API
+        Ensure we can't access comments using the old version of the API.
+        """
         with self.assertRaises(NoReverseMatch):
-            self.client.get(self.api_url(patch, version='1.0'))
+            self.client.get(self.api_url(self.patch, version='1.0'))
 
     def test_list_invalid_patch(self):
         """Ensure we get a 404 for a non-existent patch."""
         resp = self.client.get(
             reverse('api-patch-comment-list', kwargs={'patch_id': '99999'}))
         self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+
+    @utils.store_samples('patch-comment-detail')
+    def test_detail(self):
+        """Show a patch comment."""
+        comment = create_patch_comment(patch=self.patch)
+
+        resp = self.client.get(self.api_url(self.patch, item=comment))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertSerialized(comment, resp.data)
+
+    def test_detail_version_1_3(self):
+        """Show a patch comment using API v1.3."""
+        comment = create_patch_comment(patch=self.patch)
+
+        resp = self.client.get(
+            self.api_url(self.patch, version='1.3', item=comment))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertSerialized(comment, resp.data)
+
+    def test_detail_version_1_2(self):
+        """Show a patch comment using API v1.2."""
+        comment = create_patch_comment(patch=self.patch)
+
+        with self.assertRaises(NoReverseMatch):
+            self.client.get(
+                self.api_url(self.patch, version='1.2', item=comment))
+
+    def test_detail_version_1_1(self):
+        """Show a patch comment using API v1.1."""
+        comment = create_patch_comment(patch=self.patch)
+
+        with self.assertRaises(NoReverseMatch):
+            self.client.get(
+                self.api_url(self.patch, version='1.1', item=comment))
+
+    def test_detail_version_1_0(self):
+        """Show a patch comment using API v1.0."""
+        comment = create_patch_comment(patch=self.patch)
+
+        with self.assertRaises(NoReverseMatch):
+            self.client.get(
+                self.api_url(self.patch, version='1.0', item=comment))
+
+    @utils.store_samples('patch-comment-detail-error-not-found')
+    def test_detail_invalid_patch(self):
+        """Ensure we handle non-existent patches."""
+        comment = create_patch_comment()
+        resp = self.client.get(
+            reverse('api-patch-comment-detail', kwargs={
+                'patch_id': '99999',
+                'comment_id': comment.id}
+            ),
+        )
+        self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+
+    def _test_update(self, person, **kwargs):
+        submitter = kwargs.get('submitter', person)
+        patch = kwargs.get('patch', self.patch)
+        comment = create_patch_comment(submitter=submitter, patch=patch)
+
+        if kwargs.get('authenticate', True):
+            self.client.force_authenticate(user=person.user)
+        return self.client.patch(
+            self.api_url(patch, item=comment),
+            {'addressed': kwargs.get('addressed', True)},
+            validate_request=kwargs.get('validate_request', True)
+        )
+
+    @utils.store_samples('patch-comment-detail-update-authorized')
+    def test_update_authorized(self):
+        """Update an existing patch comment as an authorized user.
+
+        To be authorized users must meet at least one of the following:
+        - project maintainer, patch submitter, patch delegate, or
+          patch comment submitter
+
+        Ensure updates can only be performed by authorized users.
+        """
+        # Update as maintainer
+        person = create_person(user=self.user)
+        resp = self._test_update(person=person)
+        self.assertEqual(1, PatchComment.objects.all().count())
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertTrue(resp.data['addressed'])
+
+        # Update as patch submitter
+        person = create_person(name='patch-submitter', user=create_user())
+        patch = create_patch(submitter=person)
+        resp = self._test_update(person=person, patch=patch)
+        self.assertEqual(2, PatchComment.objects.all().count())
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertTrue(resp.data['addressed'])
+
+        # Update as patch delegate
+        person = create_person(name='patch-delegate', user=create_user())
+        patch = create_patch(delegate=person.user)
+        resp = self._test_update(person=person, patch=patch)
+        self.assertEqual(3, PatchComment.objects.all().count())
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertTrue(resp.data['addressed'])
+
+        # Update as patch comment submitter
+        person = create_person(name='comment-submitter', user=create_user())
+        patch = create_patch()
+        resp = self._test_update(person=person, patch=patch, submitter=person)
+        self.assertEqual(4, PatchComment.objects.all().count())
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertTrue(resp.data['addressed'])
+
+    @utils.store_samples('patch-comment-detail-update-not-authorized')
+    def test_update_not_authorized(self):
+        """Update an existing patch comment when not signed in and not authorized.
+
+        To be authorized users must meet at least one of the following:
+        - project maintainer, patch submitter, patch delegate, or
+          patch comment submitter
+
+        Ensure updates can only be performed by authorized users.
+        """
+        person = create_person(user=self.user)
+        resp = self._test_update(person=person, authenticate=False)
+        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+        person = create_person()  # normal user without edit permissions
+        resp = self._test_update(person=person)  # signed-in
+        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+    @utils.store_samples('patch-comment-detail-update-error-bad-request')
+    def test_update_invalid_addressed(self):
+        """Update an existing patch comment using invalid values.
+
+        Ensure we handle invalid patch comment addressed values.
+        """
+        person = create_person(name='patch-submitter', user=create_user())
+        patch = create_patch(submitter=person)
+        resp = self._test_update(person=person,
+                                 patch=patch,
+                                 addressed='not-valid',
+                                 validate_request=False)
+        self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
+        self.assertFalse(
+            getattr(PatchComment.objects.all().first(), 'addressed')
+        )
+
+    def test_create_delete(self):
+        """Ensure creates and deletes aren't allowed"""
+        comment = create_patch_comment(patch=self.patch)
+        self.user.is_superuser = True
+        self.user.save()
+        self.client.force_authenticate(user=self.user)
+
+        resp = self.client.post(self.api_url(self.patch, item=comment))
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
+        resp = self.client.delete(self.api_url(self.patch, item=comment))
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index cc09e84..2d5bd4d 100644
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -260,6 +260,7 @@ def create_patch_comment(**kwargs):
         'patch': create_patch() if 'patch' not in kwargs else None,
         'msgid': make_msgid(),
         'content': SAMPLE_CONTENT,
+        'addressed': False,
     }
     values.update(kwargs)
 
-- 
2.32.0.554.ge1b32706d8-goog



More information about the Patchwork mailing list