From 22388432866953cd5dfd54d0859166bb6a728a42 Mon Sep 17 00:00:00 2001 From: afzaledx Date: Wed, 30 May 2018 14:57:53 +0500 Subject: [PATCH] Add cohorts to BulkEnroll endpoint. --- lms/djangoapps/bulk_enroll/serializers.py | 21 ++ .../bulk_enroll/tests/test_views.py | 290 ++++++++++++++++++ lms/djangoapps/bulk_enroll/views.py | 52 +++- 3 files changed, 359 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/bulk_enroll/serializers.py b/lms/djangoapps/bulk_enroll/serializers.py index 9762498ac3..47daca20b5 100644 --- a/lms/djangoapps/bulk_enroll/serializers.py +++ b/lms/djangoapps/bulk_enroll/serializers.py @@ -3,6 +3,7 @@ Serializers for Bulk Enrollment. """ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.course_groups.cohorts import is_cohort_exists from rest_framework import serializers @@ -22,6 +23,7 @@ class BulkEnrollmentSerializer(serializers.Serializer): """ identifiers = serializers.CharField(required=True) courses = StringListField(required=True) + cohorts = StringListField(required=False) action = serializers.ChoiceField( choices=( ('enroll', 'enroll'), @@ -43,3 +45,22 @@ class BulkEnrollmentSerializer(serializers.Serializer): except InvalidKeyError: raise serializers.ValidationError("Course key not valid: {}".format(course)) return value + + def validate(self, attrs): + """ + Check that the cohorts list is the same size as the courses list. + """ + if attrs.get('cohorts'): + if attrs['action'] != 'enroll': + raise serializers.ValidationError("Cohorts can only be used for enrollments.") + if len(attrs['cohorts']) != len(attrs['courses']): + raise serializers.ValidationError( + "If provided, the cohorts and courses should have equal number of items.") + + for course_id, cohort_name in zip(attrs['courses'], attrs['cohorts']): + if not is_cohort_exists(course_key=CourseKey.from_string(course_id), name=cohort_name): + raise serializers.ValidationError("cohort {cohort_name} not found in course {course_id}.".format( + cohort_name=cohort_name, course_id=course_id) + ) + + return attrs diff --git a/lms/djangoapps/bulk_enroll/tests/test_views.py b/lms/djangoapps/bulk_enroll/tests/test_views.py index 45a84b290a..125b08793a 100644 --- a/lms/djangoapps/bulk_enroll/tests/test_views.py +++ b/lms/djangoapps/bulk_enroll/tests/test_views.py @@ -14,6 +14,9 @@ from bulk_enroll.serializers import BulkEnrollmentSerializer from bulk_enroll.views import BulkEnrollView from courseware.tests.helpers import LoginEnrollmentTestCase from microsite_configuration import microsite +from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id +from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts from student.models import ( CourseEnrollment, ManualEnrollmentAudit, @@ -336,3 +339,290 @@ class BulkEnrollmentTest(ModuleStoreTestCase, LoginEnrollmentTestCase, APITestCa # Check the outbox self.assertEqual(len(mail.outbox), 0) + + def test_fail_on_unequal_cohorts(self): + """ + Test unequal items in cohorts and courses. + """ + response = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'enroll', + 'email_students': False, + 'courses': self.course_key, + 'cohorts': "cohort1,cohort2" + }) + self.assertEqual(response.status_code, 400) + self.assertIn('If provided, the cohorts and courses should have equal number of items.', response.content) + + def test_fail_on_missing_cohorts(self): + """ + Test cohorts don't exist in the course. + """ + response = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'enroll', + 'email_students': False, + 'cohorts': 'cohort1', + 'courses': self.course_key + }) + self.assertEqual(response.status_code, 400) + self.assertIn('cohort {cohort_name} not found in course {course_id}.'.format( + cohort_name='cohort1', course_id=self.course_key + ), response.content) + + def test_allow_cohorts_when_enrolling(self): + """ + Test if the cohorts are given but the action is unenroll. + """ + config_course_cohorts(self.course, is_cohorted=True, manual_cohorts=["cohort1", "cohort2"]) + response = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'unenroll', + 'email_students': False, + 'cohorts': 'cohort1', + 'courses': self.course_key + }) + self.assertEqual(response.status_code, 400) + self.assertIn('Cohorts can only be used for enrollments.', response.content) + + def test_add_to_valid_cohort(self): + config_course_cohorts(self.course, is_cohorted=True, manual_cohorts=["cohort1", "cohort2"]) + response = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'enroll', + 'email_students': False, + 'courses': self.course_key, + 'cohorts': "cohort1" + }) + + self.assertEqual(response.status_code, 200) + + # test the response data + expected = { + "action": "enroll", + 'auto_enroll': False, + "email_students": False, + "courses": { + self.course_key: { + "action": "enroll", + 'auto_enroll': False, + "results": [ + { + "identifier": self.notenrolled_student.username, + "before": { + "enrollment": False, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": None, + }, + "after": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": 'cohort1', + } + } + ] + } + } + } + manual_enrollments = ManualEnrollmentAudit.objects.all() + self.assertEqual(manual_enrollments.count(), 1) + self.assertEqual(manual_enrollments[0].state_transition, UNENROLLED_TO_ENROLLED) + res_json = json.loads(response.content) + self.assertIsNotNone(get_cohort_id(self.notenrolled_student, CourseKey.from_string(self.course_key))) + + self.assertEqual(res_json, expected) + + def test_readd_to_different_cohort(self): + config_course_cohorts(self.course, is_cohorted=True, manual_cohorts=["cohort1", "cohort2"]) + response = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'enroll', + 'email_students': False, + 'courses': self.course_key, + 'cohorts': "cohort1" + }) + + self.assertEqual(response.status_code, 200) + + # test the response data + expected = { + "action": "enroll", + 'auto_enroll': False, + "email_students": False, + "courses": { + self.course_key: { + "action": "enroll", + 'auto_enroll': False, + "results": [ + { + "identifier": self.notenrolled_student.username, + "before": { + "enrollment": False, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": None, + }, + "after": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": 'cohort1', + } + } + ] + } + } + } + manual_enrollments = ManualEnrollmentAudit.objects.all() + self.assertEqual(manual_enrollments.count(), 1) + self.assertEqual(manual_enrollments[0].state_transition, UNENROLLED_TO_ENROLLED) + res_json = json.loads(response.content) + self.assertIsNotNone(get_cohort_id(self.notenrolled_student, CourseKey.from_string(self.course_key))) + self.assertEqual(res_json, expected) + + response2 = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'enroll', + 'email_students': False, + 'courses': self.course_key, + 'cohorts': "cohort2" + }) + + self.assertEqual(response2.status_code, 200) + + # test the response data + expected2 = { + "action": "enroll", + 'auto_enroll': False, + "email_students": False, + "courses": { + self.course_key: { + "action": "enroll", + 'auto_enroll': False, + "results": [ + { + "identifier": self.notenrolled_student.username, + "before": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": 'cohort1', + }, + "after": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": 'cohort2', + } + } + ] + } + } + } + res2_json = json.loads(response2.content) + self.assertIsNotNone(get_cohort_id(self.notenrolled_student, CourseKey.from_string(self.course_key))) + self.assertEqual(res2_json, expected2) + + def test_readd_to_same_cohort(self): + config_course_cohorts(self.course, is_cohorted=True, manual_cohorts=["cohort1", "cohort2"]) + response = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'enroll', + 'email_students': False, + 'courses': self.course_key, + 'cohorts': "cohort1" + }) + + self.assertEqual(response.status_code, 200) + + # test the response data + expected = { + "action": "enroll", + 'auto_enroll': False, + "email_students": False, + "courses": { + self.course_key: { + "action": "enroll", + 'auto_enroll': False, + "results": [ + { + "identifier": self.notenrolled_student.username, + "before": { + "enrollment": False, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": None, + }, + "after": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": 'cohort1', + } + } + ] + } + } + } + manual_enrollments = ManualEnrollmentAudit.objects.all() + self.assertEqual(manual_enrollments.count(), 1) + self.assertEqual(manual_enrollments[0].state_transition, UNENROLLED_TO_ENROLLED) + res_json = json.loads(response.content) + self.assertIsNotNone(get_cohort_id(self.notenrolled_student, CourseKey.from_string(self.course_key))) + + self.assertEqual(res_json, expected) + + response2 = self.request_bulk_enroll({ + 'identifiers': self.notenrolled_student.username, + 'action': 'enroll', + 'email_students': False, + 'courses': self.course_key, + 'cohorts': "cohort1" + }) + + self.assertEqual(response2.status_code, 200) + + # test the response data + expected2 = { + "action": "enroll", + 'auto_enroll': False, + "email_students": False, + "courses": { + self.course_key: { + "action": "enroll", + 'auto_enroll': False, + "results": [ + { + "identifier": self.notenrolled_student.username, + "before": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": 'cohort1', + }, + "after": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + "cohort": 'cohort1', + } + } + ] + } + } + } + res2_json = json.loads(response2.content) + self.assertIsNotNone(get_cohort_id(self.notenrolled_student, CourseKey.from_string(self.course_key))) + self.assertEqual(res2_json, expected2) diff --git a/lms/djangoapps/bulk_enroll/views.py b/lms/djangoapps/bulk_enroll/views.py index 583d68fec1..998bb75282 100644 --- a/lms/djangoapps/bulk_enroll/views.py +++ b/lms/djangoapps/bulk_enroll/views.py @@ -1,7 +1,9 @@ """ API views for Bulk Enrollment """ +import itertools import json + from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from rest_framework import status from rest_framework.response import Response @@ -10,6 +12,13 @@ from rest_framework.views import APIView from bulk_enroll.serializers import BulkEnrollmentSerializer from enrollment.views import EnrollmentUserThrottle from instructor.views.api import students_update_enrollment +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.course_groups.cohorts import ( + get_cohort_by_name, + add_user_to_cohort, +) +from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.lib.api.authentication import OAuth2Authentication from openedx.core.lib.api.permissions import IsStaff from util.disable_rate_limit import can_disable_rate_limit @@ -29,6 +38,7 @@ class BulkEnrollView(APIView): "email_students": true, "action": "enroll", "courses": "course-v1:edX+Demo+123,course-v1:edX+Demo2+456", + "cohorts": "cohortA,cohortA", "identifiers": "brandon@example.com,yamilah@example.com" } @@ -40,7 +50,10 @@ class BulkEnrollView(APIView): as they register. * email_students: When set to `true`, students will be sent email notifications upon enrollment. - * action: Can either be set to "enroll" or "unenroll". This determines the behabior + * action: Can either be set to "enroll" or "unenroll". This determines the behavior + * cohorts: Optional. If provided, the number of items in the list should be equal to + the number of courses. first cohort coressponds with the first course and so on. + The learners will be added to the corresponding cohort. **Response Values** @@ -51,6 +64,9 @@ class BulkEnrollView(APIView): enrollment. (See the `instructor.views.api.students_update_enrollment` docstring for the specifics of the response data available for each enrollment) + + If a cohorts list is provided, additional 'cohort' keys will be added + to the 'before' and 'after' states. """ authentication_classes = JwtAuthentication, OAuth2Authentication @@ -72,9 +88,37 @@ class BulkEnrollView(APIView): 'action': serializer.data.get('action'), 'courses': {} } - for course in serializer.data.get('courses'): - response = students_update_enrollment(self.request, course_id=course) - response_dict['courses'][course] = json.loads(response.content) + for course_id, cohort_name in itertools.izip_longest(serializer.data.get('courses'), + serializer.data.get('cohorts', [])): + response = students_update_enrollment(self.request, course_id=course_id) + response_content = json.loads(response.content) + + if cohort_name: + try: + course_key = CourseKey.from_string(course_id) + cohort = get_cohort_by_name(course_key=course_key, name=cohort_name) + except (CourseUserGroup.DoesNotExist, InvalidKeyError) as exc: + return Response(exc.message, status=status.HTTP_400_BAD_REQUEST) + + for user_data in response_content['results']: + if "after" in user_data and ( + user_data["after"].get("enrollment", False) is True or + user_data["after"].get("allowed", False) is True + ): + user_id = user_data['identifier'] + try: + _user_obj, previous_cohort, _pre_assigned = add_user_to_cohort(cohort, user_id) + except ValueError: + # User already present in cohort + previous_cohort = cohort_name + + if previous_cohort: + user_data['before']['cohort'] = previous_cohort + else: + user_data['before']['cohort'] = None + user_data['after']['cohort'] = cohort_name + + response_dict['courses'][course_id] = response_content return Response(data=response_dict, status=status.HTTP_200_OK) else: return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)