models: Use parent model to get comment's 'list_archive_url'

We were attempting to retrieve the 'list_archive_url' attribute from the
'PatchComment' or 'CoverComment' instances, rather than the parent
'Patch' and 'Cover' object, respectively. Correct this and add plenty of
tests to prevent this regressing.

Conflicts:
  patchwork/models.py

Changes:
  patchwork/tests/api/test_comment.py

NOTE(stephenfin): Conflicts and changes are necessary to deal with the
fact we have a single comment model instead of two comment models as in
stable/3.0.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Fixes: 02ffb1315 ("models: Add list archive lookup")
Closes: #391
(cherry picked from commit 93ff4db29262c0560122f61eadf78d9626def238)
(cherry picked from commit 6c73a55c52ecd6d756901c9e9647fae57aceda01)

diff --git a/patchwork/models.py b/patchwork/models.py
index e295e17368f07dc19d593f55fb116d55bfb4426d..a3c34da7eee41a819cb4c68cf2727c0fc97d543a 100644 (file)
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -380,10 +380,13 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
     def list_archive_url(self):
         if not self.project.list_archive_url_format:
             return None
+
         if not self.msgid:
             return None
+
         return self.project.list_archive_url_format.format(
-            self.url_msgid)
+            self.url_msgid,
+        )
 
     # patchwork metadata
 
@@ -640,10 +643,13 @@ class Comment(EmailMixin, models.Model):
     def list_archive_url(self):
         if not self.submission.project.list_archive_url_format:
             return None
+
         if not self.msgid:
             return None
-        return self.project.list_archive_url_format.format(
-            self.url_msgid)
+
+        return self.submission.project.list_archive_url_format.format(
+            self.url_msgid,
+        )
 
     def get_absolute_url(self):
         return reverse('comment-redirect', kwargs={'comment_id': self.id})

diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
index f48bfce1abf06ac948d51dd0956bc8c9b9069368..e9520562985e1ed7ee509348485a9d58b2e054f1 100644 (file)
--- a/patchwork/tests/api/test_comment.py
+++ b/patchwork/tests/api/test_comment.py
@@ -53,6 +53,18 @@ class TestCoverComments(utils.APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
         self.assertSerialized(comment, resp.data[0])
+        self.assertIn('list_archive_url', resp.data[0])
+
+    def test_list_version_1_1(self):
+        """List cover letter comments using API v1.1."""
+        cover = create_cover()
+        comment = create_comment(submission=cover)
+
+        resp = self.client.get(self.api_url(cover, 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 cover letter comments using API v1.0."""
@@ -104,6 +116,18 @@ class TestPatchComments(utils.APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
         self.assertSerialized(comment, resp.data[0])
+        self.assertIn('list_archive_url', resp.data[0])
+
+    def test_list_version_1_1(self):
+        """List patch comments using API v1.1."""
+        patch = create_patch()
+        comment = create_comment(submission=patch)
+
+        resp = self.client.get(self.api_url(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."""

diff --git a/patchwork/tests/api/test_project.py b/patchwork/tests/api/test_project.py
index 5a7676740affe779b5c952d7501bbda827acb830..701a4ed0478060dd5bb4b5d31353ad738e0ba589 100644 (file)
--- a/patchwork/tests/api/test_project.py
+++ b/patchwork/tests/api/test_project.py
@@ -71,6 +71,26 @@ class TestProjectAPI(utils.APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
         self.assertSerialized(project, resp.data[0])
+        self.assertIn('subject_match', resp.data[0])
+        self.assertIn('list_archive_url', resp.data[0])
+        self.assertIn('list_archive_url_format', resp.data[0])
+        self.assertIn('commit_url_format', resp.data[0])
+
+    @utils.store_samples('project-list-1.1')
+    def test_list_version_1_1(self):
+        """List projects using API v1.1.
+
+        Validate that newer fields are dropped for older API versions.
+        """
+        create_project()
+
+        resp = self.client.get(self.api_url(version='1.1'))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(1, len(resp.data))
+        self.assertIn('subject_match', resp.data[0])
+        self.assertNotIn('list_archive_url', resp.data[0])
+        self.assertNotIn('list_archive_url_format', resp.data[0])
+        self.assertNotIn('commit_url_format', resp.data[0])
 
     @utils.store_samples('project-list-1.0')
     def test_list_version_1_0(self):
@@ -86,7 +106,7 @@ class TestProjectAPI(utils.APITestCase):
         self.assertNotIn('subject_match', resp.data[0])
 
     @utils.store_samples('project-detail')
-    def test_detail_by_id(self):
+    def test_detail(self):
         """Show project using ID lookup.
 
         Validate that it's possible to filter by pk.
@@ -96,6 +116,10 @@ class TestProjectAPI(utils.APITestCase):
         resp = self.client.get(self.api_url(project.pk))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertSerialized(project, resp.data)
+        self.assertIn('subject_match', resp.data)
+        self.assertIn('list_archive_url', resp.data)
+        self.assertIn('list_archive_url_format', resp.data)
+        self.assertIn('commit_url_format', resp.data)
 
     def test_detail_by_linkname(self):
         """Show project using linkname lookup.
@@ -119,6 +143,22 @@ class TestProjectAPI(utils.APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertSerialized(project, resp.data)
 
+    @utils.store_samples('project-detail-1.1')
+    def test_detail_version_1_1(self):
+        """Show project using API v1.1.
+
+        Validate that newer fields are dropped for older API versions.
+        """
+        project = create_project()
+
+        resp = self.client.get(self.api_url(project.pk, version='1.1'))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertIn('name', resp.data)
+        self.assertIn('subject_match', resp.data)
+        self.assertNotIn('list_archive_url', resp.data)
+        self.assertNotIn('list_archive_url_format', resp.data)
+        self.assertNotIn('commit_url_format', resp.data)
+
     @utils.store_samples('project-detail-1.0')
     def test_detail_version_1_0(self):
         """Show project using API v1.0.

diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index a7151e177f44c0c48815e546e28f7f89c0e7fddb..2cb0d8aa22e6622cc4b206d07021d0dfdbdf3f2a 100644 (file)
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -60,6 +60,8 @@ def create_project(**kwargs):
         'listid': 'test%d.example.com' % num,
         'listemail': 'test%d@example.com' % num,
         'subject_match': '',
+        'list_archive_url': 'https://lists.example.com/',
+        'list_archive_url_format': 'https://lists.example.com/mail/{}',
     }
     values.update(kwargs)
 

diff --git a/releasenotes/notes/issue-391-4088c856247f228e.yaml b/releasenotes/notes/issue-391-4088c856247f228e.yaml
new file mode 100644 (file)
index 0000000..597902b
--- /dev/null
+++ b/releasenotes/notes/issue-391-4088c856247f228e.yaml
@@ -0,0 +1,6 @@
+---
+api:
+  - |
+    The ``list_archive_url`` field will now be correctly shown for patch
+    comments and cover letter comments.
+    (`#391 <https://github.com/getpatchwork/patchwork/issues/391>`__)