diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 46919297fa..fd35f9a91c 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -5,9 +5,16 @@ from django.http import Http404 from collections import defaultdict -from lms.lib.comment_client.thread import Thread from discussion_api.pagination import get_paginated_data from django_comment_client.utils import get_accessible_discussion_modules +from django_comment_common.models import ( + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_MODERATOR, + Role, +) +from lms.lib.comment_client.thread import Thread +from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id def get_course_topics(course, user): @@ -113,10 +120,18 @@ def get_thread_list(request, course_key, page, page_size): A paginated result containing a list of threads; see discussion_api.views.ThreadViewSet for more detail. """ + user_is_privileged = Role.objects.filter( + course_id=course_key, + name__in=[FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA], + users=request.user + ).exists() threads, result_page, num_pages, _ = Thread.search({ "course_id": unicode(course_key), + "group_id": None if user_is_privileged else get_cohort_id(request.user, course_key), + "sort_key": "date", + "sort_order": "desc", "page": page, - "per_page": page_size + "per_page": page_size, }) # The comments service returns the last page of results if the requested # page is beyond the last page, but we want be consistent with DRF's general diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index 79fead35d3..643f7eb51c 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -2,7 +2,9 @@ Tests for Discussion API internal interface """ from datetime import datetime, timedelta +import itertools +import ddt import httpretty import mock from pytz import UTC @@ -15,6 +17,13 @@ from opaque_keys.edx.locator import CourseLocator from courseware.tests.factories import BetaTesterFactory, StaffFactory from discussion_api.api import get_course_topics, get_thread_list from discussion_api.tests.utils import CommentsServiceMockMixin +from django_comment_common.models import ( + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_STUDENT, + Role, +) from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from student.tests.factories import UserFactory @@ -313,22 +322,26 @@ class GetCourseTopicsTest(ModuleStoreTestCase): self.assertEqual(staff_actual, staff_expected) +@ddt.ddt @httpretty.activate class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): """Test for get_thread_list""" def setUp(self): super(GetThreadListTest, self).setUp() self.maxDiff = None # pylint: disable=invalid-name + self.user = UserFactory.create() self.request = RequestFactory().get("/test_path") - self.course_key = CourseLocator.from_string("a/b/c") + self.request.user = self.user + self.course = CourseFactory.create() - def get_thread_list(self, threads, page=1, page_size=1, num_pages=1): + def get_thread_list(self, threads, page=1, page_size=1, num_pages=1, course=None): """ Register the appropriate comments service response, then call get_thread_list and return the result. """ + course = course or self.course self.register_get_threads_response(threads, page, num_pages) - ret = get_thread_list(self.request, self.course_key, page, page_size) + ret = get_thread_list(self.request, course.id, page, page_size) return ret def test_empty(self): @@ -344,7 +357,9 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): def test_basic_query_params(self): self.get_thread_list([], page=6, page_size=14) self.assert_last_query_params({ - "course_id": [unicode(self.course_key)], + "course_id": [unicode(self.course.id)], + "sort_key": ["date"], + "sort_order": ["desc"], "page": ["6"], "per_page": ["14"], "recursive": ["False"], @@ -354,7 +369,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): source_threads = [ { "id": "test_thread_id_0", - "course_id": unicode(self.course_key), + "course_id": unicode(self.course.id), "commentable_id": "topic_x", "created_at": "2015-04-28T00:00:00Z", "updated_at": "2015-04-28T11:11:11Z", @@ -368,7 +383,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): }, { "id": "test_thread_id_1", - "course_id": unicode(self.course_key), + "course_id": unicode(self.course.id), "commentable_id": "topic_y", "created_at": "2015-04-28T22:22:22Z", "updated_at": "2015-04-28T00:33:33Z", @@ -382,7 +397,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): }, { "id": "test_thread_id_2", - "course_id": unicode(self.course_key), + "course_id": unicode(self.course.id), "commentable_id": "topic_x", "created_at": "2015-04-28T00:44:44Z", "updated_at": "2015-04-28T00:55:55Z", @@ -398,7 +413,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): expected_threads = [ { "id": "test_thread_id_0", - "course_id": unicode(self.course_key), + "course_id": unicode(self.course.id), "topic_id": "topic_x", "created_at": "2015-04-28T00:00:00Z", "updated_at": "2015-04-28T11:11:11Z", @@ -412,7 +427,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): }, { "id": "test_thread_id_1", - "course_id": unicode(self.course_key), + "course_id": unicode(self.course.id), "topic_id": "topic_y", "created_at": "2015-04-28T22:22:22Z", "updated_at": "2015-04-28T00:33:33Z", @@ -426,7 +441,7 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): }, { "id": "test_thread_id_2", - "course_id": unicode(self.course_key), + "course_id": unicode(self.course.id), "topic_id": "topic_x", "created_at": "2015-04-28T00:44:44Z", "updated_at": "2015-04-28T00:55:55Z", @@ -448,6 +463,28 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): } ) + @ddt.data( + *itertools.product( + [ + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_STUDENT, + ], + [True, False] + ) + ) + @ddt.unpack + def test_request_group(self, role_name, course_is_cohorted): + cohort_course = CourseFactory.create(cohort_config={"cohorted": course_is_cohorted}) + CohortFactory.create(course_id=cohort_course.id, users=[self.user]) + role = Role.objects.create(name=role_name, course_id=cohort_course.id) + role.users = [self.user] + self.get_thread_list([], course=cohort_course) + actual_has_group = "group_id" in httpretty.last_request().querystring + expected_has_group = (course_is_cohorted and role_name == FORUM_ROLE_STUDENT) + self.assertEqual(actual_has_group, expected_has_group) + def test_pagination(self): # N.B. Empty thread list is not realistic but convenient for this test self.assertEqual( @@ -478,4 +515,4 @@ class GetThreadListTest(CommentsServiceMockMixin, ModuleStoreTestCase): # Test page past the last one self.register_get_threads_response([], page=3, num_pages=3) with self.assertRaises(Http404): - get_thread_list(self.request, self.course_key, page=4, page_size=10) + get_thread_list(self.request, self.course.id, page=4, page_size=10) diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 1bf852eceb..df6d63cfcc 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -182,6 +182,8 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ) self.assert_last_query_params({ "course_id": [unicode(self.course.id)], + "sort_key": ["date"], + "sort_order": ["desc"], "page": ["1"], "per_page": ["10"], "recursive": ["False"], @@ -200,6 +202,8 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): ) self.assert_last_query_params({ "course_id": [unicode(self.course.id)], + "sort_key": ["date"], + "sort_order": ["desc"], "page": ["18"], "per_page": ["4"], "recursive": ["False"],