diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 5cc8150770..df4ff342de 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -18,6 +18,7 @@ from django_comment_client.tests.group_id import ( from django_comment_client.tests.unicode import UnicodeTestMixin from django_comment_client.tests.utils import CohortedContentTestCase from django_comment_client.forum import views +from django_comment_client.utils import strip_none from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from courseware.courses import UserNotEnrolled @@ -106,9 +107,9 @@ def make_mock_thread_data(text, thread_id, include_children, group_id=None, grou "resp_total": 42, "resp_skip": 25, "resp_limit": 5, + "group_id": group_id } if group_id is not None: - thread_data['group_id'] = group_id thread_data['group_name'] = group_name if include_children: thread_data["children"] = [{ @@ -194,9 +195,11 @@ class SingleThreadTestCase(ModuleStoreTestCase): self.assertEquals(response.status_code, 200) response_data = json.loads(response.content) + # strip_none is being used to perform the same transform that the + # django view performs prior to writing thread data to the response self.assertEquals( response_data["content"], - make_mock_thread_data(text, thread_id, True) + strip_none(make_mock_thread_data(text, thread_id, True)) ) mock_request.assert_called_with( "get", @@ -228,9 +231,11 @@ class SingleThreadTestCase(ModuleStoreTestCase): ) self.assertEquals(response.status_code, 200) response_data = json.loads(response.content) + # strip_none is being used to perform the same transform that the + # django view performs prior to writing thread data to the response self.assertEquals( response_data["content"], - make_mock_thread_data(text, thread_id, True) + strip_none(make_mock_thread_data(text, thread_id, True)) ) mock_request.assert_called_with( "get", @@ -366,6 +371,18 @@ class SingleThreadAccessTestCase(CohortedContentTestCase): ) self.assertEqual(resp.status_code, 200) + # this test ensures that a thread response from the cs with group_id: null + # behaves the same as a thread response without a group_id (see: TNL-444) + def test_student_global_thread_in_cohorted_topic(self, mock_request): + resp = self.call_view( + mock_request, + "cohorted_topic", + self.student, + self.student_cohort.id, + thread_group_id=None + ) + self.assertEqual(resp.status_code, 200) + def test_student_different_cohort(self, mock_request): self.assertRaises( Http404, diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 73582db039..0f0830b399 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -251,7 +251,7 @@ def single_thread(request, course_id, discussion_id, thread_id): # verify that the thread belongs to the requesting student's cohort if is_commentable_cohorted(course_key, discussion_id) and not is_moderator: user_group_id = get_cohort_id(request.user, course_key) - if hasattr(thread, "group_id") and user_group_id != thread.group_id: + if getattr(thread, "group_id", None) is not None and user_group_id != thread.group_id: raise Http404 is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)