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.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Fixes: 02ffb1315 ("models: Add list archive lookup")
Closes: #391

4 files changed, 81 insertions, 5 deletions

diff --git a/patchwork/models.py b/patchwork/models.py
index 6f90627..e650e8b 100644
--- a/patchwork/models.py
+++ b/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})
diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
index dfbf904..5bbebf2 100644
--- a/patchwork/tests/api/test_comment.py
+++ b/patchwork/tests/api/test_comment.py
@@ -54,6 +54,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_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 @@ 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_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."""
diff --git a/patchwork/tests/api/test_project.py b/patchwork/tests/api/test_project.py
index bf87a56..5c2fbe1 100644
--- 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 17dc3fc..cc09e84 100644
--- a/patchwork/tests/utils.py
+++ b/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)