diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py index 70121dc43e..6fc68d5d50 100644 --- a/lms/djangoapps/ccx/models.py +++ b/lms/djangoapps/ccx/models.py @@ -53,6 +53,16 @@ class CustomCourseForEdX(models.Model): from .overrides import get_override_for_ccx return get_override_for_ccx(self, self.course, 'due') + @lazy + def max_student_enrollments_allowed(self): + """ + Get the value of the override of the 'max_student_enrollments_allowed' + datetime for this CCX + """ + # avoid circular import problems + from .overrides import get_override_for_ccx + return get_override_for_ccx(self, self.course, 'max_student_enrollments_allowed') + def has_started(self): """Return True if the CCX start date is in the past""" return datetime.now(UTC()) > self.start diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py index 0710a26a77..f3e918310c 100644 --- a/lms/djangoapps/ccx/tests/test_models.py +++ b/lms/djangoapps/ccx/tests/test_models.py @@ -200,3 +200,12 @@ class TestCCX(ModuleStoreTestCase): self.assertEqual(expected, actual) actual = self.ccx.end_datetime_text('DATE_TIME') # pylint: disable=no-member self.assertEqual(expected, actual) + + def test_ccx_max_student_enrollment_correct(self): + """ + Verify the override value for max_student_enrollments_allowed + """ + expected = 200 + self.set_ccx_override('max_student_enrollments_allowed', expected) + actual = self.ccx.max_student_enrollments_allowed # pylint: disable=no-member + self.assertEqual(expected, actual) diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index 6dcda9e291..04f1a8bbed 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -15,6 +15,9 @@ from courseware.courses import get_course_by_id from courseware.tests.factories import StudentModuleFactory from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tabs import get_course_tab_list +from django.conf import settings +from django.core.exceptions import ValidationError +from django.core.validators import validate_email from django.core.urlresolvers import reverse, resolve from django.utils.timezone import UTC from django.test.utils import override_settings @@ -114,6 +117,17 @@ def setup_students_and_grades(context): ) +def is_email(identifier): + """ + Checks if an `identifier` string is a valid email + """ + try: + validate_email(identifier) + except ValidationError: + return False + return True + + @attr('shard_1') @ddt.ddt class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @@ -179,11 +193,12 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase): role = CourseCcxCoachRole(self.course.id) role.add_users(self.coach) - def make_ccx(self): + def make_ccx(self, max_students_allowed=settings.CCX_MAX_STUDENTS_ALLOWED): """ create ccx """ ccx = CcxFactory(course_id=self.course.id, coach=self.coach) + override_field_for_ccx(ccx, self.course, 'max_student_enrollments_allowed', max_students_allowed) return ccx def get_outbox(self): @@ -270,6 +285,11 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.assertTrue(CourseEnrollment.is_enrolled(self.coach, course_key)) self.assertTrue(re.search('id="ccx-schedule"', response.content)) + # check if the max amount of student that can be enrolled has been overridden + ccx = CustomCourseForEdX.objects.get() + course_enrollments = get_override_for_ccx(ccx, self.course, 'max_student_enrollments_allowed') + self.assertEqual(course_enrollments, settings.CCX_MAX_STUDENTS_ALLOWED) + @SharedModuleStoreTestCase.modifies_courseware @patch('ccx.views.render_to_response', intercept_renderer) @patch('ccx.views.TODAY') @@ -430,8 +450,20 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase): ) self.assertEqual(response.status_code, 200) - def test_enroll_member_student(self): - """enroll a list of students who are members of the class + @ddt.data( + ('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Enroll')), + ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll')), + ('ccx_manage_student', True, 1, 'student-id', ('student-action', 'add')), + ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add')), + ) + @ddt.unpack + def test_enroll_member_student(self, view_name, send_email, outbox_count, student_form_input_name, button_tuple): + """ + Tests the enrollment of a list of students who are members + of the class. + + It tests 2 different views that use slightly different parameters, + but that perform the same task. """ self.make_coach() ccx = self.make_ccx() @@ -441,28 +473,123 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(outbox, []) url = reverse( - 'ccx_invite', + view_name, kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)} ) data = { - 'enrollment-button': 'Enroll', - 'student-ids': u','.join([student.email, ]), # pylint: disable=no-member - 'email-students': 'Notify-students-by-email', + button_tuple[0]: button_tuple[1], + student_form_input_name: u','.join([student.email, ]), # pylint: disable=no-member } + if send_email: + data['email-students'] = 'Notify-students-by-email' response = self.client.post(url, data=data, follow=True) self.assertEqual(response.status_code, 200) # we were redirected to our current location self.assertEqual(len(response.redirect_chain), 1) self.assertIn(302, response.redirect_chain[0]) - self.assertEqual(len(outbox), 1) - self.assertIn(student.email, outbox[0].recipients()) # pylint: disable=no-member + self.assertEqual(len(outbox), outbox_count) + if send_email: + self.assertIn(student.email, outbox[0].recipients()) # pylint: disable=no-member # a CcxMembership exists for this student self.assertTrue( CourseEnrollment.objects.filter(course_id=self.course.id, user=student).exists() ) - def test_unenroll_member_student(self): - """unenroll a list of students who are members of the class + def test_ccx_invite_enroll_up_to_limit(self): + """ + Enrolls a list of students up to the enrollment limit. + + This test is specific to one of the enrollment views: the reason is because + the view used in this test can perform bulk enrollments. + """ + self.make_coach() + # create ccx and limit the maximum amount of students that can be enrolled to 2 + ccx = self.make_ccx(max_students_allowed=2) + ccx_course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) + # create some users + students = [ + UserFactory.create(is_staff=False) for _ in range(3) + ] + url = reverse( + 'ccx_invite', + kwargs={'course_id': ccx_course_key} + ) + data = { + 'enrollment-button': 'Enroll', + 'student-ids': u','.join([student.email for student in students]), # pylint: disable=no-member + } + response = self.client.post(url, data=data, follow=True) + self.assertEqual(response.status_code, 200) + # a CcxMembership exists for the first two students but not the third + self.assertTrue( + CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[0]).exists() + ) + self.assertTrue( + CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[1]).exists() + ) + self.assertFalse( + CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[2]).exists() + ) + + def test_manage_student_enrollment_limit(self): + """ + Enroll students up to the enrollment limit. + + This test is specific to one of the enrollment views: the reason is because + the view used in this test cannot perform bulk enrollments. + """ + students_limit = 1 + self.make_coach() + ccx = self.make_ccx(max_students_allowed=students_limit) + ccx_course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) + students = [ + UserFactory.create(is_staff=False) for _ in range(2) + ] + url = reverse( + 'ccx_manage_student', + kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)} + ) + # enroll the first student + data = { + 'student-action': 'add', + 'student-id': u','.join([students[0].email, ]), # pylint: disable=no-member + } + response = self.client.post(url, data=data, follow=True) + self.assertEqual(response.status_code, 200) + # a CcxMembership exists for this student + self.assertTrue( + CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[0]).exists() + ) + # try to enroll the second student without success + # enroll the first student + data = { + 'student-action': 'add', + 'student-id': u','.join([students[1].email, ]), # pylint: disable=no-member + } + response = self.client.post(url, data=data, follow=True) + self.assertEqual(response.status_code, 200) + # a CcxMembership does not exist for this student + self.assertFalse( + CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[1]).exists() + ) + error_message = 'The course is full: the limit is {students_limit}'.format( + students_limit=students_limit + ) + self.assertContains(response, error_message, status_code=200) + + @ddt.data( + ('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Unenroll')), + ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll')), + ('ccx_manage_student', True, 1, 'student-id', ('student-action', 'revoke')), + ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'revoke')), + ) + @ddt.unpack + def test_unenroll_member_student(self, view_name, send_email, outbox_count, student_form_input_name, button_tuple): + """ + Tests the unenrollment of a list of students who are members of the class. + + It tests 2 different views that use slightly different parameters, + but that perform the same task. """ self.make_coach() ccx = self.make_ccx() @@ -473,26 +600,47 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(outbox, []) url = reverse( - 'ccx_invite', + view_name, kwargs={'course_id': course_key} ) data = { - 'enrollment-button': 'Unenroll', - 'student-ids': u','.join([student.email, ]), # pylint: disable=no-member - 'email-students': 'Notify-students-by-email', + button_tuple[0]: button_tuple[1], + student_form_input_name: u','.join([student.email, ]), # pylint: disable=no-member } + if send_email: + data['email-students'] = 'Notify-students-by-email' response = self.client.post(url, data=data, follow=True) self.assertEqual(response.status_code, 200) # we were redirected to our current location self.assertEqual(len(response.redirect_chain), 1) self.assertIn(302, response.redirect_chain[0]) - self.assertEqual(len(outbox), 1) - self.assertIn(student.email, outbox[0].recipients()) # pylint: disable=no-member + self.assertEqual(len(outbox), outbox_count) + if send_email: + self.assertIn(student.email, outbox[0].recipients()) # pylint: disable=no-member + # a CcxMembership does not exists for this student + self.assertFalse( + CourseEnrollment.objects.filter(course_id=self.course.id, user=student).exists() + ) - def test_enroll_non_user_student(self): - """enroll a list of students who are not users yet + @ddt.data( + ('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody@nowhere.com'), + ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody@nowhere.com'), + ('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'), + ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'), + ('ccx_manage_student', True, 0, 'student-id', ('student-action', 'add'), 'dummy_student_id'), + ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add'), 'dummy_student_id'), + ('ccx_manage_student', True, 1, 'student-id', ('student-action', 'add'), 'xyz@gmail.com'), + ('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add'), 'xyz@gmail.com'), + ) + @ddt.unpack + def test_enroll_non_user_student( + self, view_name, send_email, outbox_count, student_form_input_name, button_tuple, identifier): + """ + Tests the enrollment of a list of students who are not users yet. + + It tests 2 different views that use slightly different parameters, + but that perform the same task. """ - test_email = "nobody@nowhere.com" self.make_coach() ccx = self.make_ccx() course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) @@ -500,146 +648,82 @@ class TestCoachDashboard(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(outbox, []) url = reverse( - 'ccx_invite', + view_name, kwargs={'course_id': course_key} ) data = { - 'enrollment-button': 'Enroll', - 'student-ids': u','.join([test_email, ]), - 'email-students': 'Notify-students-by-email', + button_tuple[0]: button_tuple[1], + student_form_input_name: u','.join([identifier, ]), } + if send_email: + data['email-students'] = 'Notify-students-by-email' response = self.client.post(url, data=data, follow=True) self.assertEqual(response.status_code, 200) # we were redirected to our current location self.assertEqual(len(response.redirect_chain), 1) self.assertIn(302, response.redirect_chain[0]) - self.assertEqual(len(outbox), 1) - self.assertIn(test_email, outbox[0].recipients()) - self.assertTrue( - CourseEnrollmentAllowed.objects.filter( - course_id=course_key, email=test_email - ).exists() - ) + self.assertEqual(len(outbox), outbox_count) - def test_unenroll_non_user_student(self): - """unenroll a list of students who are not users yet + # some error messages are returned for one of the views only + if view_name == 'ccx_manage_student' and not is_email(identifier): + error_message = 'Could not find a user with name or email "{identifier}" '.format( + identifier=identifier + ) + self.assertContains(response, error_message, status_code=200) + + if is_email(identifier): + if send_email: + self.assertIn(identifier, outbox[0].recipients()) + self.assertTrue( + CourseEnrollmentAllowed.objects.filter(course_id=course_key, email=identifier).exists() + ) + else: + self.assertFalse( + CourseEnrollmentAllowed.objects.filter(course_id=course_key, email=identifier).exists() + ) + + @ddt.data( + ('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody@nowhere.com'), + ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody@nowhere.com'), + ('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'), + ('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'), + ) + @ddt.unpack + def test_unenroll_non_user_student( + self, view_name, send_email, outbox_count, student_form_input_name, button_tuple, identifier): + """ + Unenroll a list of students who are not users yet """ - test_email = "nobody@nowhere.com" self.make_coach() course = CourseFactory.create() ccx = self.make_ccx() course_key = CCXLocator.from_course_locator(course.id, ccx.id) outbox = self.get_outbox() - CourseEnrollmentAllowed(course_id=course_key, email=test_email) + CourseEnrollmentAllowed(course_id=course_key, email=identifier) self.assertEqual(outbox, []) url = reverse( - 'ccx_invite', + view_name, kwargs={'course_id': course_key} ) data = { - 'enrollment-button': 'Unenroll', - 'student-ids': u','.join([test_email, ]), - 'email-students': 'Notify-students-by-email', + button_tuple[0]: button_tuple[1], + student_form_input_name: u','.join([identifier, ]), } + if send_email: + data['email-students'] = 'Notify-students-by-email' response = self.client.post(url, data=data, follow=True) self.assertEqual(response.status_code, 200) # we were redirected to our current location self.assertEqual(len(response.redirect_chain), 1) self.assertIn(302, response.redirect_chain[0]) + self.assertEqual(len(outbox), outbox_count) self.assertFalse( CourseEnrollmentAllowed.objects.filter( - course_id=course_key, email=test_email + course_id=course_key, email=identifier ).exists() ) - @ddt.data("dummy_student_id", "xyz@gmail.com") - def test_manage_add_single_invalid_student(self, student_id): - """enroll a single non valid student - """ - self.make_coach() - ccx = self.make_ccx() - course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) - url = reverse( - 'ccx_manage_student', - kwargs={'course_id': course_key} - ) - redirect_url = reverse( - 'ccx_coach_dashboard', - kwargs={'course_id': course_key} - ) - data = { - 'student-action': 'add', - 'student-id': u','.join([student_id, ]), # pylint: disable=no-member - } - response = self.client.post(url, data=data, follow=True) - - error_message = 'Could not find a user with name or email "{student_id}" '.format( - student_id=student_id - ) - self.assertContains(response, error_message, status_code=200) - - # we were redirected to our current location - self.assertRedirects(response, redirect_url, status_code=302) - - def test_manage_add_single_student(self): - """enroll a single student who is a member of the class already - """ - self.make_coach() - ccx = self.make_ccx() - course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) - enrollment = CourseEnrollmentFactory(course_id=course_key) - student = enrollment.user - # no emails have been sent so far - outbox = self.get_outbox() - self.assertEqual(outbox, []) - - url = reverse( - 'ccx_manage_student', - kwargs={'course_id': course_key} - ) - data = { - 'student-action': 'add', - 'student-id': u','.join([student.email, ]), # pylint: disable=no-member - } - response = self.client.post(url, data=data, follow=True) - self.assertEqual(response.status_code, 200) - # we were redirected to our current location - self.assertEqual(len(response.redirect_chain), 1) - self.assertIn(302, response.redirect_chain[0]) - self.assertEqual(outbox, []) - # a CcxMembership exists for this student - self.assertTrue( - CourseEnrollment.objects.filter(course_id=course_key, user=student).exists() - ) - - def test_manage_remove_single_student(self): - """unenroll a single student who is a member of the class already - """ - self.make_coach() - ccx = self.make_ccx() - course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) - enrollment = CourseEnrollmentFactory(course_id=course_key) - student = enrollment.user - # no emails have been sent so far - outbox = self.get_outbox() - self.assertEqual(outbox, []) - - url = reverse( - 'ccx_manage_student', - kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)} - ) - data = { - 'student-action': 'revoke', - 'student-id': u','.join([student.email, ]), # pylint: disable=no-member - } - response = self.client.post(url, data=data, follow=True) - self.assertEqual(response.status_code, 200) - # we were redirected to our current location - self.assertEqual(len(response.redirect_chain), 1) - self.assertIn(302, response.redirect_chain[0]) - self.assertEqual(outbox, []) - GET_CHILDREN = XModuleMixin.get_children diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index e88fa0b623..804391c6a6 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -12,6 +12,7 @@ from contextlib import contextmanager from copy import deepcopy from cStringIO import StringIO +from django.conf import settings from django.core.urlresolvers import reverse from django.http import ( HttpResponse, @@ -35,6 +36,7 @@ from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from edxmako.shortcuts import render_to_response from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from ccx_keys.locator import CCXLocator from student.roles import CourseCcxCoachRole from student.models import CourseEnrollment @@ -59,6 +61,13 @@ log = logging.getLogger(__name__) TODAY = datetime.datetime.today # for patching in tests +class CCXUserValidationException(Exception): + """ + Custom Exception for validation of users in CCX + """ + pass + + def coach_dashboard(view): """ View decorator which enforces that the user have the CCX coach role on the @@ -171,6 +180,9 @@ def create_ccx(request, course, ccx=None): override_field_for_ccx(ccx, course, 'start', start) override_field_for_ccx(ccx, course, 'due', None) + # Enforce a static limit for the maximum amount of students that can be enrolled + override_field_for_ccx(ccx, course, 'max_student_enrollments_allowed', settings.CCX_MAX_STUDENTS_ALLOWED) + # Hide anything that can show up in the schedule hidden = 'visible_to_staff_only' for chapter in course.get_children(): @@ -407,6 +419,90 @@ def ccx_schedule(request, course, ccx=None): # pylint: disable=unused-argument return HttpResponse(json_schedule, mimetype='application/json') +def get_valid_student_email(identifier): + """ + Helper function to get an user email from an identifier and validate it. + + In the UI a Coach can enroll users using both an email and an username. + This function takes care of: + - in case the identifier is an username, extracting the user object from + the DB and then the associated email + - validating the email + + Arguments: + identifier (str): Username or email of the user to enroll + + Returns: + str: A validated email for the user to enroll + + Raises: + CCXUserValidationException: if the username is not found or the email + is not valid. + """ + user = email = None + try: + user = get_student_from_identifier(identifier) + except User.DoesNotExist: + email = identifier + else: + email = user.email + try: + validate_email(email) + except ValidationError: + raise CCXUserValidationException('Could not find a user with name or email "{0}" '.format(identifier)) + return email + + +def _ccx_students_enrrolling_center(action, identifiers, email_students, course_key, email_params): + """ + Function to enroll/add or unenroll/revoke students. + + This function exists for backwards compatibility: in CCX there are + two different views to manage students that used to implement + a different logic. Now the logic has been reconciled at the point that + this function can be used by both. + The two different views can be merged after some UI refactoring. + + Arguments: + action (str): type of action to perform (add, Enroll, revoke, Unenroll) + identifiers (list): list of students username/email + email_students (bool): Flag to send an email to students + course_key (CCXLocator): a CCX course key + email_params (dict): dictionary of settings for the email to be sent + + Returns: + list: list of error + """ + errors = [] + + if action == 'Enroll' or action == 'add': + ccx_course_overview = CourseOverview.get_from_id(course_key) + for identifier in identifiers: + if CourseEnrollment.objects.is_course_full(ccx_course_overview): + error = ('The course is full: the limit is {0}'.format( + ccx_course_overview.max_student_enrollments_allowed)) + log.info("%s", error) + errors.append(error) + break + try: + email = get_valid_student_email(identifier) + except CCXUserValidationException as exp: + log.info("%s", exp) + errors.append("{0}".format(exp)) + continue + enroll_email(course_key, email, auto_enroll=True, email_students=email_students, email_params=email_params) + elif action == 'Unenroll' or action == 'revoke': + for identifier in identifiers: + try: + email = get_valid_student_email(identifier) + except CCXUserValidationException as exp: + log.info("%s", exp) + errors.append("{0}".format(exp)) + continue + unenroll_email(course_key, email, email_students=email_students, email_params=email_params) + return errors + + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @coach_dashboard @@ -420,99 +516,36 @@ def ccx_invite(request, course, ccx=None): action = request.POST.get('enrollment-button') identifiers_raw = request.POST.get('student-ids') identifiers = _split_input_list(identifiers_raw) - auto_enroll = True - email_students = True if 'email-students' in request.POST else False - for identifier in identifiers: - user = None - email = None - try: - user = get_student_from_identifier(identifier) - except User.DoesNotExist: - email = identifier - else: - email = user.email - try: - validate_email(email) - course_key = CCXLocator.from_course_locator(course.id, ccx.id) - email_params = get_email_params(course, auto_enroll, course_key=course_key, display_name=ccx.display_name) - if action == 'Enroll': - enroll_email( - course_key, - email, - auto_enroll=auto_enroll, - email_students=email_students, - email_params=email_params - ) - if action == "Unenroll": - unenroll_email(course_key, email, email_students=email_students, email_params=email_params) - except ValidationError: - log.info('Invalid user name or email when trying to invite students: %s', email) - url = reverse( - 'ccx_coach_dashboard', - kwargs={'course_id': CCXLocator.from_course_locator(course.id, ccx.id)} - ) + email_students = 'email-students' in request.POST + course_key = CCXLocator.from_course_locator(course.id, ccx.id) + email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name) + + _ccx_students_enrrolling_center(action, identifiers, email_students, course_key, email_params) + + url = reverse('ccx_coach_dashboard', kwargs={'course_id': course_key}) return redirect(url) -def validate_student_email(email): - """ - validate student's email id - """ - error_message = None - try: - validate_email(email) - except ValidationError: - log.info( - 'Invalid user name or email when trying to enroll student: %s', - email - ) - if email: - error_message = _( - 'Could not find a user with name or email "{email}" ' - ).format(email=email) - else: - error_message = _( - 'Please enter a valid username or email.' - ) - - return error_message - - @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @coach_dashboard def ccx_student_management(request, course, ccx=None): - """Manage the enrollment of individual students in a CCX + """ + Manage the enrollment of individual students in a CCX """ if not ccx: raise Http404 action = request.POST.get('student-action', None) student_id = request.POST.get('student-id', '') - user = email = None - error_message = "" + email_students = 'email-students' in request.POST + identifiers = [student_id] course_key = CCXLocator.from_course_locator(course.id, ccx.id) - try: - user = get_student_from_identifier(student_id) - except User.DoesNotExist: - email = student_id - error_message = validate_student_email(email) - if email and not error_message: - error_message = _( - 'Could not find a user with name or email "{email}" ' - ).format(email=email) - else: - email = user.email - error_message = validate_student_email(email) + email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name) - if error_message is None: - if action == 'add': - # by decree, no emails sent to students added this way - # by decree, any students added this way are auto_enrolled - enroll_email(course_key, email, auto_enroll=True, email_students=False) - elif action == 'revoke': - unenroll_email(course_key, email, email_students=False) - else: + errors = _ccx_students_enrrolling_center(action, identifiers, email_students, course_key, email_params) + + for error_message in errors: messages.error(request, error_message) url = reverse('ccx_coach_dashboard', kwargs={'course_id': course_key}) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index cdb61f972a..5e7fb5310d 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -669,6 +669,7 @@ if FEATURES.get('CUSTOM_COURSES_EDX'): FIELD_OVERRIDE_PROVIDERS += ( 'ccx.overrides.CustomCoursesForEdxOverrideProvider', ) +CCX_MAX_STUDENTS_ALLOWED = ENV_TOKENS.get('CCX_MAX_STUDENTS_ALLOWED', CCX_MAX_STUDENTS_ALLOWED) ##### Individual Due Date Extensions ##### if FEATURES.get('INDIVIDUAL_DUE_DATES'): diff --git a/lms/envs/common.py b/lms/envs/common.py index e8b30cfb78..e12b9155bf 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2705,3 +2705,10 @@ PROCTORING_BACKEND_PROVIDER = { 'options': {}, } PROCTORING_SETTINGS = {} + +#### Custom Courses for EDX (CCX) configuration + +# This is an arbitrary hard limit. +# The reason we introcuced this number is because we do not want the CCX +# to compete with the MOOC. +CCX_MAX_STUDENTS_ALLOWED = 200 diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index b411be43cc..158f86fb91 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -109,12 +109,14 @@ class CourseOverview(TimeStampedModel): display_name = course.display_name start = course.start end = course.end + max_student_enrollments_allowed = course.max_student_enrollments_allowed if isinstance(course.id, CCXLocator): from ccx.utils import get_ccx_from_ccx_locator # pylint: disable=import-error ccx = get_ccx_from_ccx_locator(course.id) display_name = ccx.display_name start = ccx.start end = ccx.due + max_student_enrollments_allowed = ccx.max_student_enrollments_allowed return cls( version=cls.VERSION, @@ -150,7 +152,7 @@ class CourseOverview(TimeStampedModel): enrollment_end=course.enrollment_end, enrollment_domain=course.enrollment_domain, invitation_only=course.invitation_only, - max_student_enrollments_allowed=course.max_student_enrollments_allowed, + max_student_enrollments_allowed=max_student_enrollments_allowed, ) @classmethod