From 93ff4db29262c0560122f61eadf78d9626def238 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sat, 20 Feb 2021 14:35:07 +0000 Subject: 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 Fixes: 02ffb1315 ("models: Add list archive lookup") Closes: #391 --- patchwork/models.py | 18 ++++++++++++---- patchwork/tests/api/test_comment.py | 24 +++++++++++++++++++++ patchwork/tests/api/test_project.py | 42 ++++++++++++++++++++++++++++++++++++- patchwork/tests/utils.py | 2 ++ 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) -- cgit v1.2.3