diff --git a/wagtail/admin/forms/comments.py b/wagtail/admin/forms/comments.py index a55a6f95fa..c689d691d5 100644 --- a/wagtail/admin/forms/comments.py +++ b/wagtail/admin/forms/comments.py @@ -1,6 +1,7 @@ from django.forms import BooleanField, ValidationError from django.utils.timezone import now from django.utils.translation import gettext as _ +from modelcluster.forms import BaseChildFormSet from .models import WagtailAdminModelForm @@ -73,3 +74,14 @@ class CommentForm(WagtailAdminModelForm): self.instance.resolved_by = None self.instance.resolved_at = None return super().save(*args, **kwargs) + + +class CommentFormSet(BaseChildFormSet): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + valid_comment_ids = [ + comment.id + for comment in self.queryset + if comment.has_valid_contentpath(self.instance) + ] + self.queryset = self.queryset.filter(id__in=valid_comment_ids) diff --git a/wagtail/admin/panels/comment_panel.py b/wagtail/admin/panels/comment_panel.py index ad5a5b69a5..ac13f14594 100644 --- a/wagtail/admin/panels/comment_panel.py +++ b/wagtail/admin/panels/comment_panel.py @@ -1,7 +1,7 @@ from django.contrib.auth import get_user_model from modelcluster.models import get_serializable_data_for_fields -from wagtail.admin.forms.comments import CommentForm +from wagtail.admin.forms.comments import CommentForm, CommentFormSet from wagtail.admin.templatetags.wagtailadmin_tags import avatar_url, user_display_name from wagtail.models import COMMENTS_RELATION_NAME @@ -17,6 +17,7 @@ class CommentPanel(Panel): "fields": ["comment_notifications"], "formsets": { COMMENTS_RELATION_NAME: { + "formset": CommentFormSet, "form": CommentForm, "fields": ["text", "contentpath", "position"], "formset_name": "comments", diff --git a/wagtail/admin/tests/pages/test_edit_page.py b/wagtail/admin/tests/pages/test_edit_page.py index dfdf4bc548..8765a41a1a 100644 --- a/wagtail/admin/tests/pages/test_edit_page.py +++ b/wagtail/admin/tests/pages/test_edit_page.py @@ -1,4 +1,5 @@ import datetime +import json import os from unittest import mock @@ -39,6 +40,7 @@ from wagtail.test.testapp.models import ( SimplePage, SingleEventPage, StandardIndex, + StreamPage, TaggedPage, ) from wagtail.test.utils import WagtailTestUtils @@ -3806,3 +3808,67 @@ class TestCommenting(WagtailTestUtils, TestCase): # No emails should be submitted because subscriber is inactive self.assertEqual(len(mail.outbox), 0) + + +class TestCommentOutput(WagtailTestUtils, TestCase): + """ + Test that the correct set of comments is output on the edit page view + """ + + def setUp(self): + # Find root page + self.root_page = Page.objects.get(id=2) + + # Add child page + self.child_page = StreamPage( + title="Hello world!", + body=[ + { + "id": "234", + "type": "product", + "value": {"name": "Cuddly toy", "price": "$9.95"}, + }, + ], + ) + self.root_page.add_child(instance=self.child_page) + self.child_page.save_revision().publish() + + # Login + self.user = self.login() + + def test_only_comments_with_valid_paths_are_shown(self): + # add some comments on self.child_page + Comment.objects.create( + user=self.user, + page=self.child_page, + text="A test comment", + contentpath="title", + ) + Comment.objects.create( + user=self.user, + page=self.child_page, + text="A comment on a field that doesn't exist", + contentpath="sillytitle", + ) + Comment.objects.create( + user=self.user, + page=self.child_page, + text="This is quite expensive", + contentpath="body.234.price", + ) + Comment.objects.create( + user=self.user, + page=self.child_page, + text="A comment on a block that doesn't exist", + contentpath="body.234.colour", + ) + + response = self.client.get( + reverse("wagtailadmin_pages:edit", args=[self.child_page.id]) + ) + soup = self.get_soup(response.content) + comments_data_json = soup.select_one("#comments-data").string + comments_data = json.loads(comments_data_json) + comment_text = [comment["text"] for comment in comments_data["comments"]] + comment_text.sort() + self.assertEqual(comment_text, ["A test comment", "This is quite expensive"]) diff --git a/wagtail/admin/tests/test_edit_handlers.py b/wagtail/admin/tests/test_edit_handlers.py index 650058b498..85ee18b30d 100644 --- a/wagtail/admin/tests/test_edit_handlers.py +++ b/wagtail/admin/tests/test_edit_handlers.py @@ -1651,7 +1651,7 @@ class TestCommentPanel(WagtailTestUtils, TestCase): page=self.event_page, text="test", user=self.other_user, - contentpath="test_contentpath", + contentpath="location", ) self.reply_1 = CommentReply.objects.create( comment=self.comment, text="reply_1", user=self.other_user diff --git a/wagtail/models/__init__.py b/wagtail/models/__init__.py index b1a3674c18..adc807b99b 100644 --- a/wagtail/models/__init__.py +++ b/wagtail/models/__init__.py @@ -24,6 +24,7 @@ from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelatio from django.contrib.contenttypes.models import ContentType from django.core import checks from django.core.exceptions import ( + FieldDoesNotExist, ImproperlyConfigured, PermissionDenied, ValidationError, @@ -4780,6 +4781,30 @@ class Comment(ClusterableModel): def log_delete(self, **kwargs): self._log("wagtail.comments.delete", **kwargs) + def has_valid_contentpath(self, page): + """ + Return True if this comment's contentpath corresponds to a valid field or + StreamField block on the given page object + """ + field_name, *remainder = self.contentpath.split(".") + try: + field = page._meta.get_field(field_name) + except FieldDoesNotExist: + return False + + if not remainder: + # comment applies to the field as a whole + return True + + if not isinstance(field, StreamField): + # only StreamField supports content paths that are deeper than one level + return False + + stream_value = getattr(page, field_name) + block = field.get_block_by_content_path(stream_value, remainder) + # content path is valid if this returns a BoundBlock rather than None + return bool(block) + class CommentReply(models.Model): comment = ParentalKey(Comment, on_delete=models.CASCADE, related_name="replies")