Skip to content

Commit 6c73a55

Browse files
stephenfinstephenfin
committed
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)
1 parent efd9ab0 commit 6c73a55

File tree

5 files changed

+87
-5
lines changed

5 files changed

+87
-5
lines changed

‎patchwork/models.py‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,13 @@ class SubmissionMixin(FilenameMixin, EmailMixin, models.Model):
372372
def list_archive_url(self):
373373
if not self.project.list_archive_url_format:
374374
return None
375+
375376
if not self.msgid:
376377
return None
378+
377379
return self.project.list_archive_url_format.format(
378-
self.url_msgid)
380+
self.url_msgid,
381+
)
379382

380383
# patchwork metadata
381384

@@ -653,9 +656,13 @@ class CoverComment(EmailMixin, models.Model):
653656
def list_archive_url(self):
654657
if not self.cover.project.list_archive_url_format:
655658
return None
659+
656660
if not self.msgid:
657661
return None
658-
return self.project.list_archive_url_format.format(self.url_msgid)
662+
663+
return self.cover.project.list_archive_url_format.format(
664+
self.url_msgid,
665+
)
659666

660667
def get_absolute_url(self):
661668
return reverse('comment-redirect', kwargs={'comment_id': self.id})
@@ -685,10 +692,13 @@ class PatchComment(EmailMixin, models.Model):
685692
def list_archive_url(self):
686693
if not self.patch.project.list_archive_url_format:
687694
return None
695+
688696
if not self.msgid:
689697
return None
690-
return self.patch.list_archive_url_format.format(
691-
self.url_msgid)
698+
699+
return self.patch.project.list_archive_url_format.format(
700+
self.url_msgid,
701+
)
692702

693703
def get_absolute_url(self):
694704
return reverse('comment-redirect', kwargs={'comment_id': self.id})

‎patchwork/tests/api/test_comment.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@ def test_list(self):
5454
self.assertEqual(status.HTTP_200_OK, resp.status_code)
5555
self.assertEqual(1, len(resp.data))
5656
self.assertSerialized(comment, resp.data[0])
57+
self.assertIn('list_archive_url', resp.data[0])
58+
59+
def test_list_version_1_1(self):
60+
"""List cover letter comments using API v1.1."""
61+
cover = create_cover()
62+
comment = create_cover_comment(cover=cover)
63+
64+
resp = self.client.get(self.api_url(cover, version='1.1'))
65+
self.assertEqual(status.HTTP_200_OK, resp.status_code)
66+
self.assertEqual(1, len(resp.data))
67+
self.assertSerialized(comment, resp.data[0])
68+
self.assertNotIn('list_archive_url', resp.data[0])
5769

5870
def test_list_version_1_0(self):
5971
"""List cover letter comments using API v1.0."""
@@ -105,6 +117,18 @@ def test_list(self):
105117
self.assertEqual(status.HTTP_200_OK, resp.status_code)
106118
self.assertEqual(1, len(resp.data))
107119
self.assertSerialized(comment, resp.data[0])
120+
self.assertIn('list_archive_url', resp.data[0])
121+
122+
def test_list_version_1_1(self):
123+
"""List patch comments using API v1.1."""
124+
patch = create_patch()
125+
comment = create_patch_comment(patch=patch)
126+
127+
resp = self.client.get(self.api_url(patch, version='1.1'))
128+
self.assertEqual(status.HTTP_200_OK, resp.status_code)
129+
self.assertEqual(1, len(resp.data))
130+
self.assertSerialized(comment, resp.data[0])
131+
self.assertNotIn('list_archive_url', resp.data[0])
108132

109133
def test_list_version_1_0(self):
110134
"""List patch comments using API v1.0."""

‎patchwork/tests/api/test_project.py‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,26 @@ def test_list_authenticated(self):
7171
self.assertEqual(status.HTTP_200_OK, resp.status_code)
7272
self.assertEqual(1, len(resp.data))
7373
self.assertSerialized(project, resp.data[0])
74+
self.assertIn('subject_match', resp.data[0])
75+
self.assertIn('list_archive_url', resp.data[0])
76+
self.assertIn('list_archive_url_format', resp.data[0])
77+
self.assertIn('commit_url_format', resp.data[0])
78+
79+
@utils.store_samples('project-list-1.1')
80+
def test_list_version_1_1(self):
81+
"""List projects using API v1.1.
82+
83+
Validate that newer fields are dropped for older API versions.
84+
"""
85+
create_project()
86+
87+
resp = self.client.get(self.api_url(version='1.1'))
88+
self.assertEqual(status.HTTP_200_OK, resp.status_code)
89+
self.assertEqual(1, len(resp.data))
90+
self.assertIn('subject_match', resp.data[0])
91+
self.assertNotIn('list_archive_url', resp.data[0])
92+
self.assertNotIn('list_archive_url_format', resp.data[0])
93+
self.assertNotIn('commit_url_format', resp.data[0])
7494

7595
@utils.store_samples('project-list-1.0')
7696
def test_list_version_1_0(self):
@@ -86,7 +106,7 @@ def test_list_version_1_0(self):
86106
self.assertNotIn('subject_match', resp.data[0])
87107

88108
@utils.store_samples('project-detail')
89-
def test_detail_by_id(self):
109+
def test_detail(self):
90110
"""Show project using ID lookup.
91111
92112
Validate that it's possible to filter by pk.
@@ -96,6 +116,10 @@ def test_detail_by_id(self):
96116
resp = self.client.get(self.api_url(project.pk))
97117
self.assertEqual(status.HTTP_200_OK, resp.status_code)
98118
self.assertSerialized(project, resp.data)
119+
self.assertIn('subject_match', resp.data)
120+
self.assertIn('list_archive_url', resp.data)
121+
self.assertIn('list_archive_url_format', resp.data)
122+
self.assertIn('commit_url_format', resp.data)
99123

100124
def test_detail_by_linkname(self):
101125
"""Show project using linkname lookup.
@@ -119,6 +143,22 @@ def test_detail_by_numeric_linkname(self):
119143
self.assertEqual(status.HTTP_200_OK, resp.status_code)
120144
self.assertSerialized(project, resp.data)
121145

146+
@utils.store_samples('project-detail-1.1')
147+
def test_detail_version_1_1(self):
148+
"""Show project using API v1.1.
149+
150+
Validate that newer fields are dropped for older API versions.
151+
"""
152+
project = create_project()
153+
154+
resp = self.client.get(self.api_url(project.pk, version='1.1'))
155+
self.assertEqual(status.HTTP_200_OK, resp.status_code)
156+
self.assertIn('name', resp.data)
157+
self.assertIn('subject_match', resp.data)
158+
self.assertNotIn('list_archive_url', resp.data)
159+
self.assertNotIn('list_archive_url_format', resp.data)
160+
self.assertNotIn('commit_url_format', resp.data)
161+
122162
@utils.store_samples('project-detail-1.0')
123163
def test_detail_version_1_0(self):
124164
"""Show project using API v1.0.

‎patchwork/tests/utils.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ def create_project(**kwargs):
6161
'listid': 'test%d.example.com' % num,
6262
'listemail': 'test%d@example.com' % num,
6363
'subject_match': '',
64+
'list_archive_url': 'https://lists.example.com/',
65+
'list_archive_url_format': 'https://lists.example.com/mail/{}',
6466
}
6567
values.update(kwargs)
6668

‎releasenotes/notes/issue-391-4088c856247f228e.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
api:
3+
- |
4+
The ``list_archive_url`` field will now be correctly shown for patch
5+
comments and cover letter comments.
6+
(`#391 <https://github.com/getpatchwork/patchwork/issues/391>`__)

0 commit comments

Comments
 (0)