Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix commenting thread notifications being sent to non-thread users
  • Loading branch information
jacobtoppm authored and gasman committed Jan 18, 2022
1 parent 335ece5 commit 5fe901e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
22 changes: 22 additions & 0 deletions wagtail/admin/tests/pages/test_edit_page.py
Expand Up @@ -2100,6 +2100,7 @@ def setUp(self):
self.subscriber = self.create_user('subscriber')
self.non_subscriber = self.create_user('non-subscriber')
self.non_subscriber_2 = self.create_user('non-subscriber-2')
self.never_emailed_user = self.create_user('never-emailed')

PageSubscription.objects.create(
page=self.child_page,
Expand All @@ -2113,6 +2114,23 @@ def setUp(self):
comment_notifications=True
)

# Add comment and reply on a different page for the never_emailed_user
# They should never be notified
comment_on_other_page = Comment.objects.create(
page=self.root_page,
user=self.never_emailed_user,
text='a comment'
)

CommentReply.objects.create(
user=self.never_emailed_user,
comment=comment_on_other_page,
text='a reply'
)

def assertNeverEmailedWrongUser(self):
self.assertNotIn(self.never_emailed_user.email, [to for email in mail.outbox for to in email.to])

def test_new_comment(self):
post_data = {
'title': "I've been edited!",
Expand Down Expand Up @@ -2144,6 +2162,7 @@ def test_new_comment(self):

# Check notification email
self.assertEqual(len(mail.outbox), 1)
self.assertNeverEmailedWrongUser()
self.assertEqual(mail.outbox[0].to, [self.subscriber.email])
self.assertEqual(mail.outbox[0].subject, 'test@email.com has updated comments on "I\'ve been edited! (simple page)"')
self.assertIn('New comments:\n - "A test comment"\n\n', mail.outbox[0].body)
Expand Down Expand Up @@ -2283,6 +2302,7 @@ def test_resolve_comment(self):

# Check notification email
self.assertEqual(len(mail.outbox), 2)
self.assertNeverEmailedWrongUser()
# The non subscriber created the comment, so should also get an email
self.assertEqual(mail.outbox[0].to, [self.non_subscriber.email])
self.assertEqual(mail.outbox[0].subject, 'test@email.com has updated comments on "I\'ve been edited! (simple page)"')
Expand Down Expand Up @@ -2337,6 +2357,7 @@ def test_delete_comment(self):

# Check notification email
self.assertEqual(len(mail.outbox), 1)
self.assertNeverEmailedWrongUser()
self.assertEqual(mail.outbox[0].to, [self.subscriber.email])
self.assertEqual(mail.outbox[0].subject, 'test@email.com has updated comments on "I\'ve been edited! (simple page)"')
self.assertIn('Deleted comments:\n - "A test comment"\n\n', mail.outbox[0].body)
Expand Down Expand Up @@ -2398,6 +2419,7 @@ def test_new_reply(self):

# Check notification email
self.assertEqual(len(mail.outbox), 3)
self.assertNeverEmailedWrongUser()

recipients = [mail.to for mail in mail.outbox]
# The other non subscriber replied in the thread, so should get an email
Expand Down
6 changes: 3 additions & 3 deletions wagtail/admin/views/pages/edit.py
Expand Up @@ -141,11 +141,11 @@ def send_commenting_notifications(self, changes):
# Get subscribers to individual threads
replies = CommentReply.objects.filter(comment_id__in=relevant_comment_ids)
comments = Comment.objects.filter(id__in=relevant_comment_ids)
thread_users = get_user_model().objects.exclude(pk=self.request.user.pk).exclude(pk__in=subscribers.values_list('user_id', flat=True)).prefetch_related(
thread_users = get_user_model().objects.exclude(pk=self.request.user.pk).exclude(pk__in=subscribers.values_list('user_id', flat=True)).filter(
Q(comment_replies__comment_id__in=relevant_comment_ids) | Q(**{('%s__pk__in' % COMMENTS_RELATION_NAME): relevant_comment_ids})
).prefetch_related(
Prefetch('comment_replies', queryset=replies),
Prefetch(COMMENTS_RELATION_NAME, queryset=comments)
).exclude(
Q(comment_replies__isnull=True) & Q(**{('%s__isnull' % COMMENTS_RELATION_NAME): True})
)

# Skip if no recipients
Expand Down

0 comments on commit 5fe901e

Please sign in to comment.