diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index 290ec49b32..feafcc0e37 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -11,6 +11,7 @@ from django.core.urlresolvers import reverse from rest_framework import serializers from discussion_api.render import render_body +from django_comment_client.utils import is_comment_too_deep from django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, @@ -287,11 +288,12 @@ class CommentSerializer(_ContentSerializer): def validate(self, attrs): """ Ensure that parent_id identifies a comment that is actually in the - thread identified by thread_id. + thread identified by thread_id and does not violate the configured + maximum depth. """ + parent = None parent_id = attrs.get("parent_id") if parent_id: - parent = None try: parent = Comment(id=parent_id).retrieve() except CommentClientRequestError: @@ -300,6 +302,8 @@ class CommentSerializer(_ContentSerializer): raise ValidationError( "parent_id does not identify a comment in the thread identified by thread_id." ) + if is_comment_too_deep(parent): + raise ValidationError({"parent_id": ["Parent is too deep."]}) return attrs def restore_object(self, attrs, instance=None): diff --git a/lms/djangoapps/discussion_api/tests/test_serializers.py b/lms/djangoapps/discussion_api/tests/test_serializers.py index 813119d8f6..0829222d06 100644 --- a/lms/djangoapps/discussion_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion_api/tests/test_serializers.py @@ -644,6 +644,19 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore } ) + def test_create_parent_id_too_deep(self): + self.register_get_comment_response({ + "id": "test_parent", + "thread_id": "test_thread", + "depth": 2 + }) + data = self.minimal_data.copy() + data["parent_id"] = "test_parent" + context = get_context(self.course, self.request, make_minimal_cs_thread()) + serializer = CommentSerializer(data=data, context=context) + self.assertFalse(serializer.is_valid()) + self.assertEqual(serializer.errors, {"parent_id": ["Parent is too deep."]}) + def test_create_missing_field(self): for field in self.minimal_data: data = self.minimal_data.copy() diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 10e98655e1..bfe0cc7715 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -22,6 +22,7 @@ from django_comment_client.utils import ( add_courseware_context, get_annotated_content_info, get_ability, + is_comment_too_deep, JsonError, JsonResponse, prepare_content, @@ -313,9 +314,8 @@ def create_comment(request, course_id, thread_id): given a course_id and thread_id, test for comment depth. if not too deep, call _create_comment to create the actual comment. """ - if cc_settings.MAX_COMMENT_DEPTH is not None: - if cc_settings.MAX_COMMENT_DEPTH < 0: - return JsonError(_("Comment level too deep")) + if is_comment_too_deep(parent=None): + return JsonError(_("Comment level too deep")) return _create_comment(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), thread_id=thread_id) @@ -397,9 +397,8 @@ def create_sub_comment(request, course_id, comment_id): given a course_id and comment_id, create a response to a comment after checking the max depth allowed, if allowed """ - if cc_settings.MAX_COMMENT_DEPTH is not None: - if cc_settings.MAX_COMMENT_DEPTH <= cc.Comment.find(comment_id).depth: - return JsonError(_("Comment level too deep")) + if is_comment_too_deep(parent=cc.Comment(comment_id)): + return JsonError(_("Comment level too deep")) return _create_comment(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), parent_id=comment_id) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 153a3b2b12..7d8e2cf079 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -16,6 +16,7 @@ from xmodule.modulestore.django import modulestore from django_comment_common.models import Role, FORUM_ROLE_STUDENT from django_comment_client.permissions import check_permissions_by_view, has_permission +from django_comment_client.settings import MAX_COMMENT_DEPTH from edxmako import lookup_template from courseware.access import has_access @@ -568,3 +569,17 @@ def get_group_id_for_comments_service(request, course_key, commentable_id=None): # Never pass a group_id to the comments service for a non-cohorted # commentable return None + + +def is_comment_too_deep(parent): + """ + Determine whether a comment with the given parent violates MAX_COMMENT_DEPTH + + parent can be None to determine whether root comments are allowed + """ + return ( + MAX_COMMENT_DEPTH is not None and ( + MAX_COMMENT_DEPTH < 0 or + (parent and parent["depth"] >= MAX_COMMENT_DEPTH) + ) + )