Merge pull request #8576 from edx/gprice/discussion-api-max-depth

Enforce MAX_COMMENT_DEPTH in discussion_api

lms/djangoapps/discussion_api/serializers.py

@@ -12,6 +12,7 @@ from rest_framework import serializers
 
 from discussion_api.permissions import get_editable_fields
 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,
@@ -297,11 +298,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:
@@ -310,6 +312,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": ["Comment level is too deep."]})
         return attrs
 
     def restore_object(self, attrs, instance=None):

lms/djangoapps/discussion_api/tests/test_serializers.py

@@ -670,6 +670,37 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
             }
         )
 
+    @ddt.data(None, -1, 0, 2, 5)
+    def test_create_parent_id_too_deep(self, max_depth):
+        with mock.patch("django_comment_client.utils.MAX_COMMENT_DEPTH", max_depth):
+            data = self.minimal_data.copy()
+            context = get_context(self.course, self.request, make_minimal_cs_thread())
+            if max_depth is None or max_depth >= 0:
+                if max_depth != 0:
+                    self.register_get_comment_response({
+                        "id": "not_too_deep",
+                        "thread_id": "test_thread",
+                        "depth": max_depth - 1 if max_depth else 100
+                    })
+                    data["parent_id"] = "not_too_deep"
+                else:
+                    data["parent_id"] = None
+                serializer = CommentSerializer(data=data, context=context)
+                self.assertTrue(serializer.is_valid(), serializer.errors)
+            if max_depth is not None:
+                if max_depth >= 0:
+                    self.register_get_comment_response({
+                        "id": "too_deep",
+                        "thread_id": "test_thread",
+                        "depth": max_depth
+                    })
+                    data["parent_id"] = "too_deep"
+                else:
+                    data["parent_id"] = None
+                serializer = CommentSerializer(data=data, context=context)
+                self.assertFalse(serializer.is_valid())
+                self.assertEqual(serializer.errors, {"parent_id": ["Comment level is too deep."]})
+
     def test_create_missing_field(self):
         for field in self.minimal_data:
             data = self.minimal_data.copy()

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)
 
 

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)
+        )
+    )