Merge pull request #5483 from edx/anton/fix-forum-edit-thread-rc

TNL-549: Fix thread editing.

lms/djangoapps/django_comment_client/base/tests.py

@@ -173,8 +173,11 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
         super(ViewsTestCase, self).setUp(create_user=False)
 
         # create a course
-        self.course = CourseFactory.create(org='MITx', course='999',
-                                           display_name='Robot Super Course')
+        self.course = CourseFactory.create(
+            org='MITx', course='999',
+            discussion_topics={"Some Topic": {"id": "some_topic"}},
+            display_name='Robot Super Course',
+        )
         self.course_id = self.course.id
         # seed the forums permissions and roles
         call_command('seed_permissions_roles', self.course_id.to_deprecated_string())
@@ -368,7 +371,15 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
             mock_request
         )
 
-    @patch('django_comment_client.base.views.get_discussion_id_map', return_value={"test_commentable": {}})
+    def test_update_thread_course_topic(self, mock_request):
+        self._setup_mock_request(mock_request)
+        response = self.client.post(
+            reverse("update_thread", kwargs={"thread_id": "dummy", "course_id": self.course_id.to_deprecated_string()}),
+            data={"body": "foo", "title": "foo", "commentable_id": "some_topic"}
+        )
+        self.assertEqual(response.status_code, 200)
+
+    @patch('django_comment_client.base.views.get_discussion_categories_ids', return_value=["test_commentable"])
     def test_update_thread_wrong_commentable_id(self, mock_get_discussion_id_map, mock_request):
         self._test_request_error(
             "update_thread",
@@ -861,7 +872,7 @@ class UpdateThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockReq
         self.student = UserFactory.create()
         CourseEnrollmentFactory(user=self.student, course_id=self.course.id)
 
-    @patch('django_comment_client.base.views.get_discussion_id_map', return_value={"test_commentable": {}})
+    @patch('django_comment_client.base.views.get_discussion_categories_ids', return_value=["test_commentable"])
     @patch('lms.lib.comment_client.utils.requests.request')
     def _test_unicode_data(self, text, mock_request, mock_get_discussion_id_map):
         self._set_mock_request_data(mock_request, {

lms/djangoapps/django_comment_client/base/views.py

@@ -27,7 +27,7 @@ from django_comment_client.utils import (
     JsonResponse,
     prepare_content,
     get_group_id_for_comments_service,
-    get_discussion_id_map,
+    get_discussion_categories_ids
 )
 from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
 import lms.lib.comment_client as cc
@@ -146,8 +146,8 @@ def update_thread(request, course_id, thread_id):
 
     if "commentable_id" in request.POST:
         course = get_course_with_access(request.user, 'load', course_key)
-        id_map = get_discussion_id_map(course)
-        if request.POST.get("commentable_id") in id_map:
+        commentable_ids = get_discussion_categories_ids(course)
+        if request.POST.get("commentable_id") in commentable_ids:
             thread.commentable_id = request.POST["commentable_id"]
         else:
             return JsonError(_("Topic doesn't exist"))

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -148,7 +148,7 @@ class CategoryMapTestCase(ModuleStoreTestCase):
         self.course.discussion_topics = {}
         self.course.save()
         self.discussion_num = 0
-        self.maxDiff = None # pylint: disable=C0103
+        self.maxDiff = None  # pylint: disable=C0103
 
     def create_discussion(self, discussion_category, discussion_target, **kwargs):
         self.discussion_num += 1
@@ -193,7 +193,7 @@ class CategoryMapTestCase(ModuleStoreTestCase):
                 }
             )
 
-        check_cohorted_topics([]) # default (empty) cohort config
+        check_cohorted_topics([])  # default (empty) cohort config
 
         self.course.cohort_config = {"cohorted": False, "cohorted_discussions": []}
         check_cohorted_topics([])
@@ -211,7 +211,6 @@ class CategoryMapTestCase(ModuleStoreTestCase):
         self.course.cohort_config = {"cohorted": False, "cohorted_discussions": ["Topic_A"]}
         check_cohorted_topics([])
 
-
     def test_single_inline(self):
         self.create_discussion("Chapter", "Discussion")
         self.assertCategoryMapEquals(
@@ -337,7 +336,6 @@ class CategoryMapTestCase(ModuleStoreTestCase):
         self.course.cohort_config = {"cohorted": True}
         check_cohorted(True)
 
-
     def test_start_date_filter(self):
         now = datetime.now()
         later = datetime.max
@@ -564,6 +562,46 @@ class CategoryMapTestCase(ModuleStoreTestCase):
             }
         )
 
+    def test_ids_empty(self):
+        self.assertEqual(utils.get_discussion_categories_ids(self.course), [])
+
+    def test_ids_configured_topics(self):
+        self.course.discussion_topics = {
+            "Topic A": {"id": "Topic_A"},
+            "Topic B": {"id": "Topic_B"},
+            "Topic C": {"id": "Topic_C"}
+        }
+        self.assertItemsEqual(
+            utils.get_discussion_categories_ids(self.course),
+            ["Topic_A", "Topic_B", "Topic_C"]
+        )
+
+    def test_ids_inline(self):
+        self.create_discussion("Chapter 1", "Discussion 1")
+        self.create_discussion("Chapter 1", "Discussion 2")
+        self.create_discussion("Chapter 2", "Discussion")
+        self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
+        self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion")
+        self.create_discussion("Chapter 3 / Section 1", "Discussion")
+        self.assertItemsEqual(
+            utils.get_discussion_categories_ids(self.course),
+            ["discussion1", "discussion2", "discussion3", "discussion4", "discussion5", "discussion6"]
+        )
+
+    def test_ids_mixed(self):
+        self.course.discussion_topics = {
+            "Topic A": {"id": "Topic_A"},
+            "Topic B": {"id": "Topic_B"},
+            "Topic C": {"id": "Topic_C"}
+        }
+        self.create_discussion("Chapter 1", "Discussion 1")
+        self.create_discussion("Chapter 2", "Discussion")
+        self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
+        self.assertItemsEqual(
+            utils.get_discussion_categories_ids(self.course),
+            ["Topic_A", "Topic_B", "Topic_C", "discussion1", "discussion2", "discussion3"]
+        )
+
 
 class JsonResponseTestCase(TestCase, UnicodeTestMixin):
     def _test_unicode_data(self, text):

lms/djangoapps/django_comment_client/utils.py

@@ -90,7 +90,7 @@ def _filter_unstarted_categories(category_map):
     unfiltered_queue = [category_map]
     filtered_queue = [result_map]
 
-    while len(unfiltered_queue) > 0:
+    while unfiltered_queue:
 
         unfiltered_map = unfiltered_queue.pop()
         filtered_map = filtered_queue.pop()
@@ -202,6 +202,23 @@ def get_discussion_category_map(course):
     return _filter_unstarted_categories(category_map)
 
 
+def get_discussion_categories_ids(course):
+    """
+    Returns a list of available ids of categories for the course.
+    """
+    ids = []
+    queue = [get_discussion_category_map(course)]
+    while queue:
+        category_map = queue.pop()
+        for child in category_map["children"]:
+            if child in category_map["entries"]:
+                ids.append(category_map["entries"][child]["id"])
+            else:
+                queue.append(category_map["subcategories"][child])
+
+    return ids
+
+
 class JsonResponse(HttpResponse):
     def __init__(self, data=None):
         content = json.dumps(data, cls=i4xEncoder)