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.

NOTE(stephenfin): Squashed in 5fe02b6
which added the release note for this fix.

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

Author: stephenfin

patchwork/models.py

@@ -372,10 +372,13 @@ class SubmissionMixin(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
 
@@ -653,9 +656,13 @@ class CoverComment(EmailMixin, models.Model):
     def list_archive_url(self):
         if not self.cover.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.cover.project.list_archive_url_format.format(
+            self.url_msgid,
+        )
 
     def get_absolute_url(self):
         return reverse('comment-redirect', kwargs={'comment_id': self.id})
@@ -685,10 +692,13 @@ class PatchComment(EmailMixin, models.Model):
     def list_archive_url(self):
         if not self.patch.project.list_archive_url_format:
             return None
+
         if not self.msgid:
             return None
-        return self.patch.list_archive_url_format.format(
-            self.url_msgid)
+
+        return self.patch.project.list_archive_url_format.format(
+            self.url_msgid,
+        )
 
     def get_absolute_url(self):
         return reverse('comment-redirect', kwargs={'comment_id': self.id})

patchwork/tests/api/test_comment.py

@@ -54,6 +54,18 @@ def test_list(self):
         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_cover_comment(cover=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."""
@@ -105,6 +117,18 @@ def test_list(self):
         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_patch_comment(patch=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."""

patchwork/tests/api/test_project.py

@@ -71,6 +71,26 @@ def test_list_authenticated(self):
         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 @@ def test_list_version_1_0(self):
         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 @@ def test_detail_by_id(self):
         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 @@ def test_detail_by_numeric_linkname(self):
         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.

patchwork/tests/utils.py

@@ -61,6 +61,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)
 

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>`__)