From 546c021d9cd26de080c416bac7be8006c58c1bb0 Mon Sep 17 00:00:00 2001 From: Guruprasad Lakshmi Narayanan Date: Mon, 1 Oct 2018 11:12:53 +0530 Subject: [PATCH] Implement an alternative discussion settings, roles management API This is intended to be used for server-to-server communication. --- lms/djangoapps/discussion_api/forms.py | 56 +++ lms/djangoapps/discussion_api/serializers.py | 179 ++++++- .../discussion_api/tests/test_views.py | 441 +++++++++++++++++- lms/djangoapps/discussion_api/urls.py | 24 +- lms/djangoapps/discussion_api/views.py | 266 ++++++++++- 5 files changed, 960 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/discussion_api/forms.py b/lms/djangoapps/discussion_api/forms.py index e291177b23..346b83afec 100644 --- a/lms/djangoapps/discussion_api/forms.py +++ b/lms/djangoapps/discussion_api/forms.py @@ -1,11 +1,17 @@ """ Discussion API forms """ +import urllib + from django.core.exceptions import ValidationError from django.forms import BooleanField, CharField, ChoiceField, Form, IntegerField from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator +from six import text_type +from courseware.courses import get_course_with_access +from django_comment_common.models import Role, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR from openedx.core.djangoapps.util.forms import ExtendedNullBooleanField, MultiValueField @@ -119,3 +125,53 @@ class CommentGetForm(_PaginationForm): A form to validate query parameters in the comment retrieval endpoint """ requested_fields = MultiValueField(required=False) + + +class CourseDiscussionSettingsForm(Form): + """ + A form to validate the fields in the course discussion settings requests. + """ + course_id = CharField() + + def __init__(self, *args, **kwargs): + self.request_user = kwargs.pop('request_user') + super(CourseDiscussionSettingsForm, self).__init__(*args, **kwargs) + + def clean_course_id(self): + """Validate the 'course_id' value""" + course_id = self.cleaned_data['course_id'] + try: + course_key = CourseKey.from_string(course_id) + self.cleaned_data['course'] = get_course_with_access(self.request_user, 'staff', course_key) + self.cleaned_data['course_key'] = course_key + return course_id + except InvalidKeyError: + raise ValidationError("'{}' is not a valid course key".format(text_type(course_id))) + + +class CourseDiscussionRolesForm(CourseDiscussionSettingsForm): + """ + A form to validate the fields in the course discussion roles requests. + """ + ROLE_CHOICES = ( + (FORUM_ROLE_MODERATOR, FORUM_ROLE_MODERATOR), + (FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR), + (FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_GROUP_MODERATOR), + ) + rolename = ChoiceField( + ROLE_CHOICES, + error_messages={"invalid_choice": "Role '%(value)s' does not exist"} + ) + + def clean_rolename(self): + """Validate the 'rolename' value.""" + rolename = urllib.unquote(self.cleaned_data.get('rolename')) + course_id = self.cleaned_data.get('course_key') + if course_id and rolename: + try: + role = Role.objects.get(name=rolename, course_id=course_id) + except Role.DoesNotExist: + raise ValidationError("Role '{}' does not exist".format(rolename)) + + self.cleaned_data['role'] = role + return rolename diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index 9c8f7dd50a..74e4fcb290 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -9,16 +9,23 @@ from django.core.exceptions import ValidationError from django.urls import reverse from rest_framework import serializers +from discussion.views import get_divided_discussions from discussion_api.permissions import NON_UPDATABLE_COMMENT_FIELDS, NON_UPDATABLE_THREAD_FIELDS, 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, FORUM_ROLE_MODERATOR, Role +from django_comment_client.utils import is_comment_too_deep, get_group_id_for_user, get_group_name +from django_comment_common.models import ( + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_MODERATOR, + Role, +) from django_comment_common.utils import get_course_discussion_settings from lms.djangoapps.django_comment_client.utils import course_discussion_division_enabled, get_group_names_by_id from lms.lib.comment_client.comment import Comment from lms.lib.comment_client.thread import Thread from lms.lib.comment_client.user import User as CommentClientUser from lms.lib.comment_client.utils import CommentClientRequestError +from student.models import get_user_by_username_or_email def get_context(course, request, thread=None): @@ -442,3 +449,171 @@ class DiscussionTopicSerializer(serializers.Serializer): Overriden update abstract method """ pass + + +class DiscussionSettingsSerializer(serializers.Serializer): + """ + Serializer for course discussion settings. + """ + divided_course_wide_discussions = serializers.ListField( + child=serializers.CharField(), + ) + divided_inline_discussions = serializers.ListField( + child=serializers.CharField(), + ) + always_divide_inline_discussions = serializers.BooleanField() + division_scheme = serializers.CharField() + + def __init__(self, *args, **kwargs): + self.course = kwargs.pop('course') + self.discussion_settings = kwargs.pop('discussion_settings') + super(DiscussionSettingsSerializer, self).__init__(*args, **kwargs) + + def validate(self, attrs): + """ + Validate the fields in combination. + """ + if not any(field in attrs for field in self.fields): + raise ValidationError('Bad request') + + settings_to_change = {} + divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions( + self.course, self.discussion_settings + ) + + if any(item in attrs for item in ('divided_course_wide_discussions', 'divided_inline_discussions')): + divided_course_wide_discussions = attrs.get( + 'divided_course_wide_discussions', + divided_course_wide_discussions + ) + divided_inline_discussions = attrs.get('divided_inline_discussions', divided_inline_discussions) + settings_to_change['divided_discussions'] = divided_course_wide_discussions + divided_inline_discussions + + for item in ('always_divide_inline_discussions', 'division_scheme'): + if item in attrs: + settings_to_change[item] = attrs[item] + attrs['settings_to_change'] = settings_to_change + return attrs + + def create(self, validated_data): + """ + Overriden create abstract method + """ + pass + + def update(self, instance, validated_data): + """ + Overriden update abstract method + """ + pass + + +class DiscussionRolesSerializer(serializers.Serializer): + """ + Serializer for course discussion roles. + """ + + ACTION_CHOICES = ( + ('allow', 'allow'), + ('revoke', 'revoke') + ) + action = serializers.ChoiceField(ACTION_CHOICES) + user_id = serializers.CharField() + + def __init__(self, *args, **kwargs): + super(DiscussionRolesSerializer, self).__init__(*args, **kwargs) + self.user = None + + def validate_user_id(self, user_id): + try: + self.user = get_user_by_username_or_email(user_id) + return user_id + except DjangoUser.DoesNotExist: + raise ValidationError("'{}' is not a valid student identifier".format(user_id)) + + def validate(self, attrs): + """Validate the data at an object level.""" + + # Store the user object to avoid fetching it again. + if hasattr(self, 'user'): + attrs['user'] = self.user + return attrs + + def create(self, validated_data): + """ + Overriden create abstract method + """ + pass + + def update(self, instance, validated_data): + """ + Overriden update abstract method + """ + pass + + +class DiscussionRolesMemberSerializer(serializers.Serializer): + """ + Serializer for course discussion roles member data. + """ + username = serializers.CharField() + email = serializers.EmailField() + first_name = serializers.CharField() + last_name = serializers.CharField() + group_name = serializers.SerializerMethodField() + + def __init__(self, *args, **kwargs): + super(DiscussionRolesMemberSerializer, self).__init__(*args, **kwargs) + self.course_discussion_settings = self.context['course_discussion_settings'] + + def get_group_name(self, instance): + """Return the group name of the user.""" + group_id = get_group_id_for_user(instance, self.course_discussion_settings) + group_name = get_group_name(group_id, self.course_discussion_settings) + return group_name + + def create(self, validated_data): + """ + Overriden create abstract method + """ + pass + + def update(self, instance, validated_data): + """ + Overriden update abstract method + """ + pass + + +class DiscussionRolesListSerializer(serializers.Serializer): + """ + Serializer for course discussion roles member list. + """ + course_id = serializers.CharField() + results = serializers.SerializerMethodField() + division_scheme = serializers.SerializerMethodField() + + def get_results(self, obj): + """Return the nested serializer data representing a list of member users.""" + context = { + 'course_id': obj['course_id'], + 'course_discussion_settings': self.context['course_discussion_settings'] + } + serializer = DiscussionRolesMemberSerializer(obj['users'], context=context, many=True) + return serializer.data + + def get_division_scheme(self, obj): # pylint: disable=unused-argument + """Return the division scheme for the course.""" + return self.context['course_discussion_settings'].division_scheme + + def create(self, validated_data): + """ + Overriden create abstract method + """ + pass + + def update(self, instance, validated_data): + """ + Overriden update abstract method + """ + pass diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 95cafa87e2..aafad0df20 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -11,12 +11,16 @@ import ddt import httpretty import mock from django.urls import reverse +from edx_oauth2_provider.tests.factories import ClientFactory, AccessTokenFactory +from opaque_keys.edx.keys import CourseKey from pytz import UTC from rest_framework.parsers import JSONParser -from rest_framework.test import APIClient +from rest_framework.test import APIClient, APITestCase from six import text_type from common.test.utils import disable_signal +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory from discussion_api import api from discussion_api.tests.utils import ( CommentsServiceMockMixin, @@ -25,7 +29,10 @@ from discussion_api.tests.utils import ( make_minimal_cs_thread, make_paginated_api_response ) -from django_comment_client.tests.utils import ForumsEnableMixin +from django_comment_client.tests.utils import ForumsEnableMixin, config_course_discussions, topic_name_to_id +from django_comment_common.models import CourseDiscussionSettings, Role +from django_comment_common.utils import seed_permissions_roles +from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage from openedx.core.djangoapps.user_api.models import RetirementState, UserRetirementStatus @@ -1774,3 +1781,433 @@ class CommentViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase expected_profile_data = self.get_expected_user_profile(response_comment['author']) response_users = response_comment['users'] self.assertEqual(expected_profile_data, response_users[response_comment['author']]) + + +@ddt.ddt +class CourseDiscussionSettingsAPIViewTest(APITestCase, UrlResetMixin, ModuleStoreTestCase): + """ + Test the course discussion settings handler API endpoint. + """ + @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super(CourseDiscussionSettingsAPIViewTest, self).setUp() + self.course = CourseFactory.create( + org="x", + course="y", + run="z", + start=datetime.now(UTC), + discussion_topics={"Test Topic": {"id": "test_topic"}} + ) + self.path = reverse('discussion_course_settings', kwargs={'course_id': text_type(self.course.id)}) + self.password = 'edx' + self.user = UserFactory(username='staff', password=self.password, is_staff=True) + + def _get_oauth_headers(self, user): + """Return the OAuth headers for testing OAuth authentication""" + access_token = AccessTokenFactory.create(user=user, client=ClientFactory()).token + headers = { + 'HTTP_AUTHORIZATION': 'Bearer ' + access_token + } + return headers + + def _login_as_staff(self): + """Log the client in as the staff.""" + self.client.login(username=self.user.username, password=self.password) + + def _create_divided_discussions(self): + """Create some divided discussions for testing.""" + divided_inline_discussions = ['Topic A', ] + divided_course_wide_discussions = ['Topic B', ] + divided_discussions = divided_inline_discussions + divided_course_wide_discussions + + ItemFactory.create( + parent_location=self.course.location, + category='discussion', + discussion_id=topic_name_to_id(self.course, 'Topic A'), + discussion_category='Chapter', + discussion_target='Discussion', + start=datetime.now() + ) + discussion_topics = { + "Topic B": {"id": "Topic B"}, + } + config_course_cohorts(self.course, is_cohorted=True) + config_course_discussions( + self.course, + discussion_topics=discussion_topics, + divided_discussions=divided_discussions + ) + return divided_inline_discussions, divided_course_wide_discussions + + def _get_expected_response(self): + """Return the default expected response before any changes to the discussion settings.""" + return { + u'always_divide_inline_discussions': False, + u'divided_inline_discussions': [], + u'divided_course_wide_discussions': [], + u'id': 1, + u'division_scheme': u'cohort', + u'available_division_schemes': [u'cohort'] + } + + def patch_request(self, data, headers=None): + headers = headers if headers else {} + return self.client.patch(self.path, json.dumps(data), content_type='application/merge-patch+json', **headers) + + def _assert_current_settings(self, expected_response): + """Validate the current discussion settings against the expected response.""" + response = self.client.get(self.path) + self.assertEqual(response.status_code, 200) + content = json.loads(response.content) + self.assertEqual(content, expected_response) + + def _assert_patched_settings(self, data, expected_response): + """Validate the patched settings against the expected response.""" + response = self.patch_request(data) + self.assertEqual(response.status_code, 204) + self._assert_current_settings(expected_response) + + @ddt.data('get', 'patch') + def test_authentication_required(self, method): + """Test and verify that authentication is required for this endpoint.""" + self.client.logout() + response = getattr(self.client, method)(self.path) + self.assertEqual(response.status_code, 401) + + @ddt.data( + {'is_staff': False, 'get_status': 403, 'put_status': 403}, + {'is_staff': True, 'get_status': 200, 'put_status': 204}, + ) + @ddt.unpack + def test_oauth(self, is_staff, get_status, put_status): + """Test that OAuth authentication works for this endpoint.""" + user = UserFactory(is_staff=is_staff) + headers = self._get_oauth_headers(user) + self.client.logout() + + response = self.client.get(self.path, **headers) + self.assertEqual(response.status_code, get_status) + + response = self.patch_request( + {'always_divide_inline_discussions': True}, headers + ) + self.assertEqual(response.status_code, put_status) + + def test_non_existent_course_id(self): + """Test the response when this endpoint is passed a non-existent course id.""" + self._login_as_staff() + response = self.client.get( + reverse('discussion_course_settings', kwargs={ + 'course_id': 'a/b/c' + }) + ) + self.assertEqual(response.status_code, 404) + + def test_get_settings(self): + """Test the current discussion settings against the expected response.""" + divided_inline_discussions, divided_course_wide_discussions = self._create_divided_discussions() + self._login_as_staff() + response = self.client.get(self.path) + self.assertEqual(response.status_code, 200) + expected_response = self._get_expected_response() + expected_response['divided_course_wide_discussions'] = [ + topic_name_to_id(self.course, name) for name in divided_course_wide_discussions + ] + expected_response['divided_inline_discussions'] = [ + topic_name_to_id(self.course, name) for name in divided_inline_discussions + ] + content = json.loads(response.content) + self.assertEqual(content, expected_response) + + def test_available_schemes(self): + """Test the available division schemes against the expected response.""" + config_course_cohorts(self.course, is_cohorted=False) + self._login_as_staff() + expected_response = self._get_expected_response() + expected_response['available_division_schemes'] = [] + self._assert_current_settings(expected_response) + + CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.AUDIT) + CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED) + + expected_response['available_division_schemes'] = [CourseDiscussionSettings.ENROLLMENT_TRACK] + self._assert_current_settings(expected_response) + + config_course_cohorts(self.course, is_cohorted=True) + expected_response['available_division_schemes'] = [ + CourseDiscussionSettings.COHORT, CourseDiscussionSettings.ENROLLMENT_TRACK + ] + self._assert_current_settings(expected_response) + + def test_empty_body_patch_request(self): + """Test the response status code on sending a PATCH request with an empty body or missing fields.""" + self._login_as_staff() + response = self.patch_request("") + self.assertEqual(response.status_code, 400) + + response = self.patch_request({}) + self.assertEqual(response.status_code, 400) + + @ddt.data( + {'abc': 123}, + {'divided_course_wide_discussions': 3}, + {'divided_inline_discussions': 'a'}, + {'always_divide_inline_discussions': ['a']}, + {'division_scheme': True} + ) + def test_invalid_body_parameters(self, body): + """Test the response status code on sending a PATCH request with parameters having incorrect types.""" + self._login_as_staff() + response = self.patch_request(body) + self.assertEqual(response.status_code, 400) + + def test_update_always_divide_inline_discussion_settings(self): + """Test whether the 'always_divide_inline_discussions' setting is updated.""" + config_course_cohorts(self.course, is_cohorted=True) + self._login_as_staff() + expected_response = self._get_expected_response() + self._assert_current_settings(expected_response) + expected_response['always_divide_inline_discussions'] = True + + self._assert_patched_settings({'always_divide_inline_discussions': True}, expected_response) + + def test_update_course_wide_discussion_settings(self): + """Test whether the 'divided_course_wide_discussions' setting is updated.""" + discussion_topics = { + 'Topic B': {'id': 'Topic B'} + } + config_course_cohorts(self.course, is_cohorted=True) + config_course_discussions(self.course, discussion_topics=discussion_topics) + expected_response = self._get_expected_response() + self._login_as_staff() + self._assert_current_settings(expected_response) + expected_response['divided_course_wide_discussions'] = [ + topic_name_to_id(self.course, "Topic B") + ] + self._assert_patched_settings( + {'divided_course_wide_discussions': [topic_name_to_id(self.course, "Topic B")]}, + expected_response + ) + expected_response['divided_course_wide_discussions'] = [] + self._assert_patched_settings( + {'divided_course_wide_discussions': []}, + expected_response + ) + + def test_update_inline_discussion_settings(self): + """Test whether the 'divided_inline_discussions' setting is updated.""" + config_course_cohorts(self.course, is_cohorted=True) + self._login_as_staff() + expected_response = self._get_expected_response() + self._assert_current_settings(expected_response) + + now = datetime.now() + ItemFactory.create( + parent_location=self.course.location, + category='discussion', + discussion_id='Topic_A', + discussion_category='Chapter', + discussion_target='Discussion', + start=now + ) + expected_response['divided_inline_discussions'] = ['Topic_A', ] + self._assert_patched_settings({'divided_inline_discussions': ['Topic_A']}, expected_response) + + expected_response['divided_inline_discussions'] = [] + self._assert_patched_settings({'divided_inline_discussions': []}, expected_response) + + def test_update_division_scheme(self): + """Test whether the 'division_scheme' setting is updated.""" + config_course_cohorts(self.course, is_cohorted=True) + self._login_as_staff() + expected_response = self._get_expected_response() + self._assert_current_settings(expected_response) + expected_response['division_scheme'] = 'none' + self._assert_patched_settings({'division_scheme': 'none'}, expected_response) + + +@ddt.ddt +class CourseDiscussionRolesAPIViewTest(APITestCase, UrlResetMixin, ModuleStoreTestCase): + """ + Test the course discussion roles management endpoint. + """ + @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super(CourseDiscussionRolesAPIViewTest, self).setUp() + self.course = CourseFactory.create( + org="x", + course="y", + run="z", + start=datetime.now(UTC), + ) + self.password = 'edx' + self.user = UserFactory(username='staff', password=self.password, is_staff=True) + course_key = CourseKey.from_string('x/y/z') + seed_permissions_roles(course_key) + + @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def path(self, course_id=None, role=None): + """Return the URL path to the endpoint based on the provided arguments.""" + course_id = text_type(self.course.id) if course_id is None else course_id + role = 'Moderator' if role is None else role + return reverse( + 'discussion_course_roles', + kwargs={'course_id': course_id, 'rolename': role} + ) + + def _get_oauth_headers(self, user): + """Return the OAuth headers for testing OAuth authentication.""" + access_token = AccessTokenFactory.create(user=user, client=ClientFactory()).token + headers = { + 'HTTP_AUTHORIZATION': 'Bearer ' + access_token + } + return headers + + def _login_as_staff(self): + """Log the client is as the staff user.""" + self.client.login(username=self.user.username, password=self.password) + + def _create_and_enroll_users(self, count): + """Create 'count' number of users and enroll them in self.course.""" + users = [] + for _ in range(count): + user = UserFactory() + CourseEnrollmentFactory.create(user=user, course_id=self.course.id) + users.append(user) + return users + + def _add_users_to_role(self, users, rolename): + """Add the given users to the given role.""" + role = Role.objects.get(name=rolename, course_id=self.course.id) + for user in users: + role.users.add(user) + + def post(self, role, user_id, action): + """Make a POST request to the endpoint using the provided parameters.""" + self._login_as_staff() + return self.client.post(self.path(role=role), {'user_id': user_id, 'action': action}) + + @ddt.data('get', 'post') + def test_authentication_required(self, method): + """Test and verify that authentication is required for this endpoint.""" + self.client.logout() + response = getattr(self.client, method)(self.path()) + self.assertEqual(response.status_code, 401) + + def test_oauth(self): + """Test that OAuth authentication works for this endpoint.""" + oauth_headers = self._get_oauth_headers(self.user) + self.client.logout() + response = self.client.get(self.path(), **oauth_headers) + self.assertEqual(response.status_code, 200) + body = {'user_id': 'staff', 'action': 'allow'} + response = self.client.post(self.path(), body, format='json', **oauth_headers) + self.assertEqual(response.status_code, 200) + + @ddt.data( + {'username': 'u1', 'is_staff': False, 'expected_status': 403}, + {'username': 'u2', 'is_staff': True, 'expected_status': 200}, + ) + @ddt.unpack + def test_staff_permission_required(self, username, is_staff, expected_status): + """Test and verify that only users with staff permission can access this endpoint.""" + UserFactory(username=username, password='edx', is_staff=is_staff) + self.client.login(username=username, password='edx') + response = self.client.get(self.path()) + self.assertEqual(response.status_code, expected_status) + + response = self.client.post(self.path(), {'user_id': username, 'action': 'allow'}, format='json') + self.assertEqual(response.status_code, expected_status) + + def test_non_existent_course_id(self): + """Test the response when the endpoint URL contains a non-existent course id.""" + self._login_as_staff() + path = self.path(course_id='a/b/c') + response = self.client.get(path) + + self.assertEqual(response.status_code, 404) + + response = self.client.post(path) + self.assertEqual(response.status_code, 404) + + def test_non_existent_course_role(self): + """Test the response when the endpoint URL contains a non-existent role.""" + self._login_as_staff() + path = self.path(role='A') + response = self.client.get(path) + + self.assertEqual(response.status_code, 400) + + response = self.client.post(path) + self.assertEqual(response.status_code, 400) + + @ddt.data( + {'role': 'Moderator', 'count': 0}, + {'role': 'Moderator', 'count': 1}, + {'role': 'Group Moderator', 'count': 2}, + {'role': 'Community TA', 'count': 3}, + ) + @ddt.unpack + def test_get_role_members(self, role, count): + """Test the get role members endpoint response.""" + config_course_cohorts(self.course, is_cohorted=True) + users = self._create_and_enroll_users(count=count) + + self._add_users_to_role(users, role) + self._login_as_staff() + response = self.client.get(self.path(role=role)) + + self.assertEqual(response.status_code, 200) + + content = json.loads(response.content) + self.assertEqual(content['course_id'], 'x/y/z') + self.assertEqual(len(content['results']), count) + expected_fields = ('username', 'email', 'first_name', 'last_name', 'group_name') + for item in content['results']: + for expected_field in expected_fields: + self.assertIn(expected_field, item) + self.assertEqual(content['division_scheme'], 'cohort') + + def test_post_missing_body(self): + """Test the response with a POST request without a body.""" + self._login_as_staff() + response = self.client.post(self.path()) + self.assertEqual(response.status_code, 400) + + @ddt.data( + {'a': 1}, + {'user_id': 'xyz', 'action': 'allow'}, + {'user_id': 'staff', 'action': 123}, + ) + def test_missing_or_invalid_parameters(self, body): + """ + Test the response when the POST request has missing required parameters or + invalid values for the required parameters. + """ + self._login_as_staff() + response = self.client.post(self.path(), body) + self.assertEqual(response.status_code, 400) + + response = self.client.post(self.path(), body, format='json') + self.assertEqual(response.status_code, 400) + + @ddt.data( + {'action': 'allow', 'user_in_role': False}, + {'action': 'allow', 'user_in_role': True}, + {'action': 'revoke', 'user_in_role': False}, + {'action': 'revoke', 'user_in_role': True} + ) + @ddt.unpack + def test_post_update_user_role(self, action, user_in_role): + """Test the response when updating the user's role""" + users = self._create_and_enroll_users(count=1) + user = users[0] + role = 'Moderator' + if user_in_role: + self._add_users_to_role(users, role) + + response = self.post(role, user.username, action) + self.assertEqual(response.status_code, 200) + content = json.loads(response.content) + assertion = self.assertTrue if action == 'allow' else self.assertFalse + assertion(any(user.username in x['username'] for x in content['results'])) diff --git a/lms/djangoapps/discussion_api/urls.py b/lms/djangoapps/discussion_api/urls.py index a060f9c47b..f138bfcf41 100644 --- a/lms/djangoapps/discussion_api/urls.py +++ b/lms/djangoapps/discussion_api/urls.py @@ -5,13 +5,35 @@ from django.conf import settings from django.conf.urls import include, url from rest_framework.routers import SimpleRouter -from discussion_api.views import CommentViewSet, CourseTopicsView, CourseView, ThreadViewSet, RetireUserView +from discussion_api.views import ( + CommentViewSet, + CourseDiscussionSettingsAPIView, + CourseDiscussionRolesAPIView, + CourseTopicsView, + CourseView, + ThreadViewSet, + RetireUserView, +) ROUTER = SimpleRouter() ROUTER.register("threads", ThreadViewSet, base_name="thread") ROUTER.register("comments", CommentViewSet, base_name="comment") urlpatterns = [ + url( + r"^v1/courses/{}/settings$".format( + settings.COURSE_ID_PATTERN + ), + CourseDiscussionSettingsAPIView.as_view(), + name="discussion_course_settings", + ), + url( + r'^v1/courses/{}/roles/(?P[A-Za-z0-9+ _-]+)/?$'.format( + settings.COURSE_ID_PATTERN + ), + CourseDiscussionRolesAPIView.as_view(), + name="discussion_course_roles", + ), url( r"^v1/courses/{}".format(settings.COURSE_ID_PATTERN), CourseView.as_view(), diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index a17952ea56..872897ab20 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -3,6 +3,7 @@ Discussion API views """ from django.core.exceptions import ValidationError from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from opaque_keys.edx.keys import CourseKey from rest_framework import permissions from rest_framework import status @@ -13,7 +14,13 @@ from rest_framework.views import APIView from rest_framework.viewsets import ViewSet from six import text_type +from django_comment_client.utils import available_division_schemes +from django_comment_common.models import Role +from django_comment_common.utils import get_course_discussion_settings, set_course_discussion_settings + +from instructor.access import update_forum_role from lms.lib import comment_client +from discussion.views import get_divided_discussions from discussion_api.api import ( create_comment, create_thread, @@ -28,11 +35,23 @@ from discussion_api.api import ( update_comment, update_thread ) -from discussion_api.forms import CommentGetForm, CommentListGetForm, ThreadListGetForm +from discussion_api.forms import ( + CommentGetForm, + CommentListGetForm, + CourseDiscussionSettingsForm, + ThreadListGetForm, + CourseDiscussionRolesForm) +from discussion_api.serializers import ( + DiscussionRolesSerializer, + DiscussionRolesListSerializer, + DiscussionSettingsSerializer, +) +from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser from openedx.core.lib.api.parsers import MergePatchParser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from openedx.core.djangoapps.user_api.accounts.permissions import CanRetireUser from openedx.core.djangoapps.user_api.models import UserRetirementStatus +from util.json_request import JsonResponse from xmodule.modulestore.django import modulestore @@ -567,3 +586,248 @@ class RetireUserView(APIView): return Response(text_type(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) return Response(status=status.HTTP_204_NO_CONTENT) + + +class CourseDiscussionSettingsAPIView(DeveloperErrorViewMixin, APIView): + """ + **Use Cases** + Retrieve all the discussion settings for a course or update one or more of them. + + **Example Requests** + + GET /api/discussion/v1/courses/{course_id}/settings + + PATCH /api/discussion/v1/courses/{course_id}/settings + {"always_divide_inline_discussions": true} + + **GET Discussion Settings Parameters**: + + * course_id (required): The course to retrieve the discussion settings for. + + **PATCH Discussion Settings Parameters**: + + * course_id (required): The course to retrieve the discussion settings for. + + The body should have the 'application/merge-patch+json' content type. + + * divided_inline_discussions: A list of IDs of the topics to be marked as divided inline discussions. + + * divided_course_wide_discussions: A list of IDs of the topics to be marked as divided course-wide discussions. + + * always_divide_inline_discussions: A boolean indicating whether inline discussions should always be + divided or not. + + * division_scheme: A string corresponding to the division scheme to be used from the list of + available division schemes. + + **GET and PATCH Discussion Settings Parameters Response Values**: + + A HTTP 404 Not Found response status code is returned when the requested course is invalid. + + A HTTP 400 Bad Request response status code is returned when the request is invalid. + + A HTTP 200 OK response status denote is returned to denote success. + + * id: The discussion settings id. + + * divided_inline_discussions: A list of divided inline discussions. + + * divided_course_wide_discussions: A list of divided course-wide discussions. + + * division_scheme: The division scheme used for the course discussions. + + * available_division_schemes: A list of available division schemes for the course. + + """ + authentication_classes = ( + JwtAuthentication, + OAuth2AuthenticationAllowInactiveUser, + SessionAuthenticationAllowInactiveUser, + ) + parser_classes = (JSONParser, MergePatchParser,) + permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser) + + def _get_representation(self, course, course_key, discussion_settings): + """ + Return a serialized representation of the course discussion settings. + """ + divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions( + course, discussion_settings + ) + return JsonResponse({ + 'id': discussion_settings.id, + 'divided_inline_discussions': divided_inline_discussions, + 'divided_course_wide_discussions': divided_course_wide_discussions, + 'always_divide_inline_discussions': discussion_settings.always_divide_inline_discussions, + 'division_scheme': discussion_settings.division_scheme, + 'available_division_schemes': available_division_schemes(course_key) + }) + + def _get_request_kwargs(self, course_id): + return dict(course_id=course_id) + + def get(self, request, course_id): + """ + Implement a handler for the GET method. + """ + kwargs = self._get_request_kwargs(course_id) + form = CourseDiscussionSettingsForm(kwargs, request_user=request.user) + + if not form.is_valid(): + raise ValidationError(form.errors) + + course_key = form.cleaned_data['course_key'] + course = form.cleaned_data['course'] + discussion_settings = get_course_discussion_settings(course_key) + return self._get_representation(course, course_key, discussion_settings) + + def patch(self, request, course_id): + """ + Implement a handler for the PATCH method. + """ + if request.content_type != MergePatchParser.media_type: + raise UnsupportedMediaType(request.content_type) + + kwargs = self._get_request_kwargs(course_id) + form = CourseDiscussionSettingsForm(kwargs, request_user=request.user) + if not form.is_valid(): + raise ValidationError(form.errors) + + course = form.cleaned_data['course'] + course_key = form.cleaned_data['course_key'] + discussion_settings = get_course_discussion_settings(course_key) + + serializer = DiscussionSettingsSerializer( + data=request.data, + partial=True, + course=course, + discussion_settings=discussion_settings + ) + if not serializer.is_valid(): + raise ValidationError(serializer.errors) + + settings_to_change = serializer.validated_data['settings_to_change'] + + try: + discussion_settings = set_course_discussion_settings(course_key, **settings_to_change) + except ValueError as e: + raise ValidationError(text_type(e)) + + return Response(status=status.HTTP_204_NO_CONTENT) + + +class CourseDiscussionRolesAPIView(DeveloperErrorViewMixin, APIView): + """ + **Use Cases** + Retrieve all the members of a given forum discussion role or update the membership of a role. + + **Example Requests** + + GET /api/discussion/v1/courses/{course_id}/roles/{rolename} + + POST /api/discussion/v1/courses/{course_id}/roles/{rolename} + {"user_id": "", "action": ""} + + **GET List Members of a Role Parameters**: + + * course_id (required): The course to which the role belongs to. + + * rolename (required): The name of the forum discussion role, the members of which have to be listed. + Currently supported values are 'Moderator', 'Group Moderator', 'Community TA'. If the value has a space + it has to be URL encoded. + + **POST Update the membership of a Role Parameters**: + + * course_id (required): The course to which the role belongs to. + + * rolename (required): The name of the forum discussion role, the members of which have to be listed. + Currently supported values are 'Moderator', 'Group Moderator', 'Community TA'. If the value has a space + it has to be URL encoded. + + The body can use either 'application/x-www-form-urlencoded' or 'application/json' content type. + + * user_id (required): The username or email address of the user whose membership has to be updated. + + * action (required): Either 'allow' or 'revoke', depending on the action to be performed on the membership. + + **GET and POST Response Values**: + + A HTTP 404 Not Found response status code is returned when the requested course is invalid. + + A HTTP 400 Bad Request response status code is returned when the request is invalid. + + A HTTP 200 OK response status denote is returned to denote success. + + * course_id: The course to which the role belongs to. + + * results: A list of the members belonging to the specified role. + + * username: Username of the user. + + * email: Email address of the user. + + * first_name: First name of the user. + + * last_name: Last name of the user. + + * group_name: Name of the group the user belongs to. + + * division_scheme: The division scheme used by the course. + """ + authentication_classes = ( + JwtAuthentication, + OAuth2AuthenticationAllowInactiveUser, + SessionAuthenticationAllowInactiveUser, + ) + permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser) + + def _get_request_kwargs(self, course_id, rolename): + return dict(course_id=course_id, rolename=rolename) + + def get(self, request, course_id, rolename): + """ + Implement a handler for the GET method. + """ + kwargs = self._get_request_kwargs(course_id, rolename) + form = CourseDiscussionRolesForm(kwargs, request_user=request.user) + + if not form.is_valid(): + raise ValidationError(form.errors) + + course_id = form.cleaned_data['course_key'] + role = form.cleaned_data['role'] + + data = {'course_id': course_id, 'users': role.users.all()} + context = {'course_discussion_settings': get_course_discussion_settings(course_id)} + + serializer = DiscussionRolesListSerializer(data, context=context) + return Response(serializer.data) + + def post(self, request, course_id, rolename): + """ + Implement a handler for the POST method. + """ + kwargs = self._get_request_kwargs(course_id, rolename) + form = CourseDiscussionRolesForm(kwargs, request_user=request.user) + if not form.is_valid(): + raise ValidationError(form.errors) + + course_id = form.cleaned_data['course_key'] + rolename = form.cleaned_data['rolename'] + + serializer = DiscussionRolesSerializer(data=request.data) + if not serializer.is_valid(): + raise ValidationError(serializer.errors) + + action = serializer.validated_data['action'] + user = serializer.validated_data['user'] + try: + update_forum_role(course_id, user, rolename, action) + except Role.DoesNotExist: + raise ValidationError("Role '{}' does not exist".format(rolename)) + + role = form.cleaned_data['role'] + data = {'course_id': course_id, 'users': role.users.all()} + context = {'course_discussion_settings': get_course_discussion_settings(course_id)} + serializer = DiscussionRolesListSerializer(data, context=context) + return Response(serializer.data)