diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 982abde32e..5c7eb990f8 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/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, { diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 4b8d940543..99b19dbbd8 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/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")) diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 7052b17d2b..9fd36cfae5 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/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): diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 4500dfb875..f88fc3d82c 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/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)