diff --git a/cms/static/sass/elements/_header.scss b/cms/static/sass/elements/_header.scss index 3bfafc7d28..afc4eabaaa 100644 --- a/cms/static/sass/elements/_header.scss +++ b/cms/static/sass/elements/_header.scss @@ -201,7 +201,7 @@ > .label { display: inline-block; - max-width: 85%; + max-width: 84%; overflow: hidden; white-space: nowrap; text-overflow: ellipsis; diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index 072f8e79a0..94466bf20d 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -163,6 +163,25 @@ class CourseMode(models.Model): return mode.min_price return 0 + @classmethod + def has_payment_options(cls, course_id): + """Determines if there is any mode that has payment options + + Check the dict of course modes and see if any of them have a minimum price or + suggested prices. Returns True if any course mode has a payment option. + + Args: + course_mode_dict (dict): Dictionary mapping course mode slugs to Modes + + Returns: + True if any course mode has a payment option. + + """ + for mode in cls.modes_for_course(course_id): + if mode.min_price > 0 or mode.suggested_prices != '': + return True + return False + @classmethod def min_course_price_for_currency(cls, course_id, currency): """ diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index c369aaaf0f..88571d449d 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -127,3 +127,22 @@ class CourseModeModelTest(TestCase): mode = CourseMode.verified_mode_for_course(self.course_key) self.assertEqual(mode.slug, 'professional') + + def test_course_has_payment_options(self): + # Has no payment options. + honor, _ = self.create_mode('honor', 'Honor') + self.assertFalse(CourseMode.has_payment_options(self.course_key)) + + # Now we do have a payment option. + verified, _ = self.create_mode('verified', 'Verified', min_price=5) + self.assertTrue(CourseMode.has_payment_options(self.course_key)) + + # Unset verified's minimum price. + verified.min_price = 0 + verified.save() + self.assertFalse(CourseMode.has_payment_options(self.course_key)) + + # Finally, give the honor mode payment options + honor.suggested_prices = '5, 10, 15' + honor.save() + self.assertTrue(CourseMode.has_payment_options(self.course_key)) diff --git a/common/djangoapps/student/management/commands/transfer_students.py b/common/djangoapps/student/management/commands/transfer_students.py index c1b46cdeff..041402e19e 100644 --- a/common/djangoapps/student/management/commands/transfer_students.py +++ b/common/djangoapps/student/management/commands/transfer_students.py @@ -1,81 +1,135 @@ +""" +Transfer Student Management Command +""" +from django.db import transaction +from opaque_keys.edx.keys import CourseKey from optparse import make_option -from django.core.management.base import BaseCommand from django.contrib.auth.models import User from student.models import CourseEnrollment from shoppingcart.models import CertificateItem -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from track.management.tracked_command import TrackedCommand -class Command(BaseCommand): +class TransferStudentError(Exception): + """Generic Error when handling student transfers.""" + pass + + +class Command(TrackedCommand): + """Management Command for transferring students from one course to new courses.""" help = """ This command takes two course ids as input and transfers all students enrolled in one course into the other. This will - remove them from the first class and enroll them in the second - class in the same mode as the first one. eg. honor, verified, + remove them from the first class and enroll them in the specified + class(es) in the same mode as the first one. eg. honor, verified, audit. example: # Transfer students from the old demoX class to a new one. manage.py ... transfer_students -f edX/Open_DemoX/edx_demo_course -t edX/Open_DemoX/new_demoX + + # Transfer students from old course to new, with original certificate items. + manage.py ... transfer_students -f edX/Open_DemoX/edx_demo_course -t edX/Open_DemoX/new_demoX -c true + + # Transfer students from the old demoX class into two new classes. + manage.py ... transfer_students -f edX/Open_DemoX/edx_demo_course + -t edX/Open_DemoX/new_demoX,edX/Open_DemoX/edX_Insider + """ - option_list = BaseCommand.option_list + ( + option_list = TrackedCommand.option_list + ( make_option('-f', '--from', metavar='SOURCE_COURSE', dest='source_course', help='The course to transfer students from.'), make_option('-t', '--to', - metavar='DEST_COURSE', - dest='dest_course', - help='The new course to enroll the student into.'), + metavar='DEST_COURSE_LIST', + dest='dest_course_list', + help='The new course(es) to enroll the student into.'), + make_option('-c', '--transfer-certificates', + metavar='TRANSFER_CERTIFICATES', + dest='transfer_certificates', + help="If True, try to transfer certificate items to the new course.") ) - def handle(self, *args, **options): - source_key = SlashSeparatedCourseKey.from_deprecated_string(options['source_course']) - dest_key = SlashSeparatedCourseKey.from_deprecated_string(options['dest_course']) + @transaction.commit_manually + def handle(self, *args, **options): # pylint: disable=unused-argument + source_key = CourseKey.from_string(options.get('source_course', '')) + dest_keys = [] + for course_key in options.get('dest_course_list', '').split(','): + dest_keys.append(CourseKey.from_string(course_key)) + + if not source_key or not dest_keys: + raise TransferStudentError(u"Must have a source course and destination course specified.") + + tc_option = options.get('transfer_certificates', '') + transfer_certificates = ('true' == tc_option.lower()) if tc_option else False + if transfer_certificates and len(dest_keys) != 1: + raise TransferStudentError(u"Cannot transfer certificate items from one course to many.") source_students = User.objects.filter( courseenrollment__course_id=source_key ) for user in source_students: - if CourseEnrollment.is_enrolled(user, dest_key): - # Un Enroll from source course but don't mess - # with the enrollment in the destination course. - CourseEnrollment.unenroll(user, source_key) - print("Unenrolled {} from {}".format(user.username, source_key.to_deprecated_string())) - msg = "Skipping {}, already enrolled in destination course {}" - print(msg.format(user.username, dest_key.to_deprecated_string())) - continue + with transaction.commit_on_success(): + print("Moving {}.".format(user.username)) + # Find the old enrollment. + enrollment = CourseEnrollment.objects.get( + user=user, + course_id=source_key + ) - print("Moving {}.".format(user.username)) - # Find the old enrollment. - enrollment = CourseEnrollment.objects.get( - user=user, - course_id=source_key + # Move the Student between the classes. + mode = enrollment.mode + old_is_active = enrollment.is_active + CourseEnrollment.unenroll(user, source_key, emit_unenrollment_event=False) + print(u"Unenrolled {} from {}".format(user.username, unicode(source_key))) + + for dest_key in dest_keys: + if CourseEnrollment.is_enrolled(user, dest_key): + # Un Enroll from source course but don't mess + # with the enrollment in the destination course. + msg = u"Skipping {}, already enrolled in destination course {}" + print(msg.format(user.username, unicode(dest_key))) + else: + new_enrollment = CourseEnrollment.enroll(user, dest_key, mode=mode) + + # Un-enroll from the new course if the user had un-enrolled + # form the old course. + if not old_is_active: + new_enrollment.update_enrollment(is_active=False, emit_unenrollment_event=False) + + if transfer_certificates: + self._transfer_certificate_item(source_key, enrollment, user, dest_keys, new_enrollment) + + @staticmethod + def _transfer_certificate_item(source_key, enrollment, user, dest_keys, new_enrollment): + """ Transfer the certificate item from one course to another. + + Do not use this generally, since certificate items are directly associated with a particular purchase. + This should only be used when a single course to a new location. This cannot be used when transferring + from one course to many. + + Args: + source_key (str): The course key string representation for the original course. + enrollment (CourseEnrollment): The original enrollment to move the certificate item from. + user (User): The user to transfer the item for. + dest_keys (list): A list of course key strings to transfer the item to. + new_enrollment (CourseEnrollment): The new enrollment to associate the certificate item with. + + Returns: + None + + """ + try: + certificate_item = CertificateItem.objects.get( + course_id=source_key, + course_enrollment=enrollment ) + except CertificateItem.DoesNotExist: + print(u"No certificate for {}".format(user)) + return - # Move the Student between the classes. - mode = enrollment.mode - old_is_active = enrollment.is_active - CourseEnrollment.unenroll(user, source_key) - new_enrollment = CourseEnrollment.enroll(user, dest_key, mode=mode) - - # Unenroll from the new coures if the user had unenrolled - # form the old course. - if not old_is_active: - new_enrollment.update_enrollment(is_active=False) - - if mode == 'verified': - try: - certificate_item = CertificateItem.objects.get( - course_id=source_key, - course_enrollment=enrollment - ) - except CertificateItem.DoesNotExist: - print("No certificate for {}".format(user)) - continue - - certificate_item.course_id = dest_key - certificate_item.course_enrollment = new_enrollment - certificate_item.save() + certificate_item.course_id = dest_keys[0] + certificate_item.course_enrollment = new_enrollment diff --git a/common/djangoapps/student/management/tests/__init__.py b/common/djangoapps/student/management/tests/__init__.py new file mode 100644 index 0000000000..b7d6ea0722 --- /dev/null +++ b/common/djangoapps/student/management/tests/__init__.py @@ -0,0 +1 @@ +"""Tests for Student Management Commands.""" diff --git a/common/djangoapps/student/management/tests/test_transfer_students.py b/common/djangoapps/student/management/tests/test_transfer_students.py new file mode 100644 index 0000000000..caebeeace2 --- /dev/null +++ b/common/djangoapps/student/management/tests/test_transfer_students.py @@ -0,0 +1,60 @@ +""" +Tests the transfer student management command +""" +from django.conf import settings +from opaque_keys.edx import locator +import unittest +import ddt +from student.management.commands import transfer_students +from student.models import CourseEnrollment +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +@ddt.ddt +class TestTransferStudents(ModuleStoreTestCase): + """Tests for transferring students between courses.""" + + PASSWORD = 'test' + + def test_transfer_students(self): + student = UserFactory() + student.set_password(self.PASSWORD) # pylint: disable=E1101 + student.save() # pylint: disable=E1101 + + # Original Course + original_course_location = locator.CourseLocator('Org0', 'Course0', 'Run0') + course = self._create_course(original_course_location) + # Enroll the student in 'verified' + CourseEnrollment.enroll(student, course.id, mode="verified") + + # New Course 1 + course_location_one = locator.CourseLocator('Org1', 'Course1', 'Run1') + new_course_one = self._create_course(course_location_one) + + # New Course 2 + course_location_two = locator.CourseLocator('Org2', 'Course2', 'Run2') + new_course_two = self._create_course(course_location_two) + original_key = unicode(course.id) + new_key_one = unicode(new_course_one.id) + new_key_two = unicode(new_course_two.id) + + # Run the actual management command + transfer_students.Command().handle( + source_course=original_key, dest_course_list=new_key_one + "," + new_key_two + ) + + # Confirm the enrollment mode is verified on the new courses, and enrollment is enabled as appropriate. + self.assertEquals(('verified', False), CourseEnrollment.enrollment_mode_for_user(student, course.id)) + self.assertEquals(('verified', True), CourseEnrollment.enrollment_mode_for_user(student, new_course_one.id)) + self.assertEquals(('verified', True), CourseEnrollment.enrollment_mode_for_user(student, new_course_two.id)) + + def _create_course(self, course_location): + """ Creates a course """ + return CourseFactory.create( + org=course_location.org, + number=course_location.course, + run=course_location.run + ) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 70691adf28..dda87a80e5 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -776,7 +776,7 @@ class CourseEnrollment(models.Model): is_course_full = cls.num_enrolled_in(course.id) >= course.max_student_enrollments_allowed return is_course_full - def update_enrollment(self, mode=None, is_active=None): + def update_enrollment(self, mode=None, is_active=None, emit_unenrollment_event=True): """ Updates an enrollment for a user in a class. This includes options like changing the mode, toggling is_active True/False, etc. @@ -784,6 +784,7 @@ class CourseEnrollment(models.Model): Also emits relevant events for analytics purposes. This saves immediately. + """ activation_changed = False # if is_active is None, then the call to update_enrollment didn't specify @@ -813,7 +814,7 @@ class CourseEnrollment(models.Model): u"mode:{}".format(self.mode)] ) - else: + elif emit_unenrollment_event: UNENROLL_DONE.send(sender=None, course_enrollment=self) self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) @@ -987,7 +988,7 @@ class CourseEnrollment(models.Model): raise @classmethod - def unenroll(cls, user, course_id): + def unenroll(cls, user, course_id, emit_unenrollment_event=True): """ Remove the user from a given course. If the relevant `CourseEnrollment` object doesn't exist, we log an error but don't throw an exception. @@ -997,10 +998,12 @@ class CourseEnrollment(models.Model): adding an enrollment for it. `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall) + + `emit_unenrollment_events` can be set to False to suppress events firing. """ try: record = CourseEnrollment.objects.get(user=user, course_id=course_id) - record.update_enrollment(is_active=False) + record.update_enrollment(is_active=False, emit_unenrollment_event=emit_unenrollment_event) except cls.DoesNotExist: err_msg = u"Tried to unenroll student {} from {} but they were not enrolled" diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 94a81590e4..eb23c1f876 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -697,7 +697,10 @@ def _allow_donation(course_modes, course_id): True if the course is allowing donations. """ - return DonationConfiguration.current().enabled and not CourseMode.has_verified_mode(course_modes[course_id]) + donations_enabled = DonationConfiguration.current().enabled + is_verified_mode = CourseMode.has_verified_mode(course_modes[course_id]) + has_payment_option = CourseMode.has_payment_options(course_id) + return donations_enabled and not is_verified_mode and not has_payment_option def try_change_enrollment(request): diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 7e6ac0f2f1..f0198e92a5 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -331,6 +331,10 @@ def send_mail_to_student(student, param_dict): 'emails/remove_beta_tester_email_subject.txt', 'emails/remove_beta_tester_email_message.txt' ), + 'account_creation_and_enrollment': ( + 'emails/enroll_email_enrolledsubject.txt', + 'emails/account_creation_and_enroll_emailMessage.txt' + ), } subject_template, message_template = email_template_dict.get(message_type, (None, None)) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 94c980fa5a..3c529ec077 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -9,6 +9,7 @@ import requests import datetime import ddt import random +import io from urllib import quote from django.test import TestCase from nose.tools import raises @@ -40,6 +41,7 @@ from courseware.models import StudentModule # modules which are mocked in test cases. import instructor_task.api import instructor.views.api +from instructor.views.api import generate_unique_password from instructor.views.api import _split_input_list, common_exceptions_400 from instructor_task.api_helper import AlreadyRunningError from opaque_keys.edx.locations import SlashSeparatedCourseKey @@ -48,6 +50,8 @@ from shoppingcart.models import ( PaidCourseRegistration, Coupon, Invoice, CourseRegistrationCode ) from course_modes.models import CourseMode +from django.core.files.uploadedfile import SimpleUploadedFile +from student.models import NonExistentCourseError from .test_tools import msk_from_problem_urlname from ..views.tools import get_extended_due @@ -285,6 +289,242 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase): ) +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +@patch.dict(settings.FEATURES, {'ALLOW_AUTOMATED_SIGNUPS': True}) +class TestInstructorAPIBulkAccountCreationAndEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): + """ + Test Bulk account creation and enrollment from csv file + """ + def setUp(self): + self.request = RequestFactory().request() + self.course = CourseFactory.create() + self.instructor = InstructorFactory(course_key=self.course.id) + self.client.login(username=self.instructor.username, password='test') + self.url = reverse('register_and_enroll_students', kwargs={'course_id': self.course.id.to_deprecated_string()}) + + self.not_enrolled_student = UserFactory( + username='NotEnrolledStudent', + email='nonenrolled@test.com', + first_name='NotEnrolled', + last_name='Student' + ) + + @patch('instructor.views.api.log.info') + def test_account_creation_and_enrollment_with_csv(self, info_log): + """ + Happy path test to create a single new user + """ + csv_content = "test_student@example.com,test_student_1,tester1,USA" + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertEquals(len(data['row_errors']), 0) + self.assertEquals(len(data['warnings']), 0) + self.assertEquals(len(data['general_errors']), 0) + + # test the log for email that's send to new created user. + info_log.assert_called_with('email sent to new created user at test_student@example.com') + + @patch('instructor.views.api.log.info') + def test_account_creation_and_enrollment_with_csv_with_blank_lines(self, info_log): + """ + Happy path test to create a single new user + """ + csv_content = "\ntest_student@example.com,test_student_1,tester1,USA\n\n" + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertEquals(len(data['row_errors']), 0) + self.assertEquals(len(data['warnings']), 0) + self.assertEquals(len(data['general_errors']), 0) + + # test the log for email that's send to new created user. + info_log.assert_called_with('email sent to new created user at test_student@example.com') + + @patch('instructor.views.api.log.info') + def test_email_and_username_already_exist(self, info_log): + """ + If the email address and username already exists + and the user is enrolled in the course, do nothing (including no email gets sent out) + """ + csv_content = "test_student@example.com,test_student_1,tester1,USA\n" \ + "test_student@example.com,test_student_1,tester2,US" + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertEquals(len(data['row_errors']), 0) + self.assertEquals(len(data['warnings']), 0) + self.assertEquals(len(data['general_errors']), 0) + + # test the log for email that's send to new created user. + info_log.assert_called_with("user already exists with username '{username}' and email '{email}'".format(username='test_student_1', email='test_student@example.com')) + + def test_bad_file_upload_type(self): + """ + Try uploading some non-CSV file and verify that it is rejected + """ + uploaded_file = SimpleUploadedFile("temp.jpg", io.BytesIO(b"some initial binary data: \x00\x01").read()) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertNotEquals(len(data['general_errors']), 0) + self.assertEquals(data['general_errors'][0]['response'], 'Could not read uploaded file.') + + def test_insufficient_data(self): + """ + Try uploading a CSV file which does not have the exact four columns of data + """ + csv_content = "test_student@example.com,test_student_1\n" + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertEquals(len(data['row_errors']), 0) + self.assertEquals(len(data['warnings']), 0) + self.assertEquals(len(data['general_errors']), 1) + self.assertEquals(data['general_errors'][0]['response'], 'Data in row #1 must have exactly four columns: email, username, full name, and country') + + def test_invalid_email_in_csv(self): + """ + Test failure case of a poorly formatted email field + """ + csv_content = "test_student.example.com,test_student_1,tester1,USA" + + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + data = json.loads(response.content) + self.assertEqual(response.status_code, 200) + self.assertNotEquals(len(data['row_errors']), 0) + self.assertEquals(len(data['warnings']), 0) + self.assertEquals(len(data['general_errors']), 0) + self.assertEquals(data['row_errors'][0]['response'], 'Invalid email {0}.'.format('test_student.example.com')) + + @patch('instructor.views.api.log.info') + def test_csv_user_exist_and_not_enrolled(self, info_log): + """ + If the email address and username already exists + and the user is not enrolled in the course, enrolled him/her and iterate to next one. + """ + csv_content = "nonenrolled@test.com,NotEnrolledStudent,tester1,USA" + + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + info_log.assert_called_with('user {username} enrolled in the course {course}'.format(username='NotEnrolledStudent', course=self.course.id)) + + def test_user_with_already_existing_email_in_csv(self): + """ + If the email address already exists, but the username is different, + assume it is the correct user and just register the user in the course. + """ + csv_content = "test_student@example.com,test_student_1,tester1,USA\n" \ + "test_student@example.com,test_student_2,tester2,US" + + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + warning_message = 'An account with email {email} exists but the provided username {username} ' \ + 'is different. Enrolling anyway with {email}.'.format(email='test_student@example.com', username='test_student_2') + self.assertNotEquals(len(data['warnings']), 0) + self.assertEquals(data['warnings'][0]['response'], warning_message) + user = User.objects.get(email='test_student@example.com') + self.assertTrue(CourseEnrollment.is_enrolled(user, self.course.id)) + + def test_user_with_already_existing_username_in_csv(self): + """ + If the username already exists (but not the email), + assume it is a different user and fail to create the new account. + """ + csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ + "test_student2@example.com,test_student_1,tester2,US" + + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertNotEquals(len(data['row_errors']), 0) + self.assertEquals(data['row_errors'][0]['response'], 'Username {user} already exists.'.format(user='test_student_1')) + + def test_csv_file_not_attached(self): + """ + Test when the user does not attach a file + """ + csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ + "test_student2@example.com,test_student_1,tester2,US" + + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + + response = self.client.post(self.url, {'file_not_found': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertNotEquals(len(data['general_errors']), 0) + self.assertEquals(data['general_errors'][0]['response'], 'File is not attached.') + + def test_raising_exception_in_auto_registration_and_enrollment_case(self): + """ + Test that exceptions are handled well + """ + csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ + "test_student2@example.com,test_student_1,tester2,US" + + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + with patch('instructor.views.api.create_and_enroll_user') as mock: + mock.side_effect = NonExistentCourseError() + response = self.client.post(self.url, {'students_list': uploaded_file}) + + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertNotEquals(len(data['row_errors']), 0) + self.assertEquals(data['row_errors'][0]['response'], 'NonExistentCourseError') + + def test_generate_unique_password(self): + """ + generate_unique_password should generate a unique password string that excludes certain characters. + """ + password = generate_unique_password([], 12) + self.assertEquals(len(password), 12) + for letter in password: + self.assertNotIn(letter, 'aAeEiIoOuU1l') + + def test_users_created_and_enrolled_successfully_if_others_fail(self): + + csv_content = "test_student1@example.com,test_student_1,tester1,USA\n" \ + "test_student3@example.com,test_student_1,tester3,CA\n" \ + "test_student2@example.com,test_student_2,tester2,USA" + + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEqual(response.status_code, 200) + data = json.loads(response.content) + self.assertNotEquals(len(data['row_errors']), 0) + self.assertEquals(data['row_errors'][0]['response'], 'Username {user} already exists.'.format(user='test_student_1')) + self.assertTrue(User.objects.filter(username='test_student_1', email='test_student1@example.com').exists()) + self.assertTrue(User.objects.filter(username='test_student_2', email='test_student2@example.com').exists()) + self.assertFalse(User.objects.filter(email='test_student3@example.com').exists()) + + @patch.object(instructor.views.api, 'generate_random_string', + Mock(side_effect=['first', 'first', 'second'])) + def test_generate_unique_password_no_reuse(self): + """ + generate_unique_password should generate a unique password string that hasn't been generated before. + """ + generated_password = ['first'] + password = generate_unique_password(generated_password, 12) + self.assertNotEquals(password, 'first') + + @patch.dict(settings.FEATURES, {'ALLOW_AUTOMATED_SIGNUPS': False}) + def test_allow_automated_signups_flag_not_set(self): + csv_content = "test_student1@example.com,test_student_1,tester1,USA" + uploaded_file = SimpleUploadedFile("temp.csv", csv_content) + response = self.client.post(self.url, {'students_list': uploaded_file}) + self.assertEquals(response.status_code, 403) + + @ddt.ddt @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 7b92a13516..3d5bb7a566 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -50,6 +50,7 @@ from instructor_task.models import ReportStore import instructor.enrollment as enrollment from instructor.enrollment import ( enroll_email, + send_mail_to_student, get_email_params, send_beta_role_email, unenroll_email @@ -83,6 +84,7 @@ from .tools import ( from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys import InvalidKeyError +from student.models import UserProfile, Registration log = logging.getLogger(__name__) @@ -216,6 +218,187 @@ def require_level(level): return decorator +EMAIL_INDEX = 0 +USERNAME_INDEX = 1 +NAME_INDEX = 2 +COUNTRY_INDEX = 3 + + +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@require_level('staff') +def register_and_enroll_students(request, course_id): # pylint: disable=R0915 + """ + Create new account and Enroll students in this course. + Passing a csv file that contains a list of students. + Order in csv should be the following email = 0; username = 1; name = 2; country = 3. + Requires staff access. + + -If the email address and username already exists and the user is enrolled in the course, + do nothing (including no email gets sent out) + + -If the email address already exists, but the username is different, + match on the email address only and continue to enroll the user in the course using the email address + as the matching criteria. Note the change of username as a warning message (but not a failure). Send a standard enrollment email + which is the same as the existing manual enrollment + + -If the username already exists (but not the email), assume it is a different user and fail to create the new account. + The failure will be messaged in a response in the browser. + """ + + if not microsite.get_value('ALLOW_AUTOMATED_SIGNUPS', settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False)): + return HttpResponseForbidden() + + course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) + warnings = [] + row_errors = [] + general_errors = [] + + if 'students_list' in request.FILES: + students = [] + + try: + upload_file = request.FILES.get('students_list') + students = [row for row in csv.reader(upload_file.read().splitlines())] + except Exception: # pylint: disable=W0703 + general_errors.append({ + 'username': '', 'email': '', 'response': _('Could not read uploaded file.') + }) + finally: + upload_file.close() + + generated_passwords = [] + course = get_course_by_id(course_id) + row_num = 0 + for student in students: + row_num = row_num + 1 + + # verify that we have exactly four columns in every row but allow for blank lines + if len(student) != 4: + if len(student) > 0: + general_errors.append({ + 'username': '', + 'email': '', + 'response': _('Data in row #{row_num} must have exactly four columns: email, username, full name, and country').format(row_num=row_num) + }) + continue + + # Iterate each student in the uploaded csv file. + email = student[EMAIL_INDEX] + username = student[USERNAME_INDEX] + name = student[NAME_INDEX] + country = student[COUNTRY_INDEX][:2] + + email_params = get_email_params(course, True, secure=request.is_secure()) + try: + validate_email(email) # Raises ValidationError if invalid + except ValidationError: + row_errors.append({ + 'username': username, 'email': email, 'response': _('Invalid email {email_address}.').format(email_address=email)}) + else: + if User.objects.filter(email=email).exists(): + # Email address already exists. assume it is the correct user + # and just register the user in the course and send an enrollment email. + user = User.objects.get(email=email) + + # see if it is an exact match with email and username + # if it's not an exact match then just display a warning message, but continue onwards + if not User.objects.filter(email=email, username=username).exists(): + warning_message = _( + 'An account with email {email} exists but the provided username {username} ' + 'is different. Enrolling anyway with {email}.' + ).format(email=email, username=username) + + warnings.append({ + 'username': username, 'email': email, 'response': warning_message}) + log.warning('email {email} already exist'.format(email=email)) + else: + log.info("user already exists with username '{username}' and email '{email}'".format(email=email, username=username)) + + # make sure user is enrolled in course + if not CourseEnrollment.is_enrolled(user, course_id): + CourseEnrollment.enroll(user, course_id) + log.info('user {username} enrolled in the course {course}'.format(username=username, course=course.id)) + enroll_email(course_id=course_id, student_email=email, auto_enroll=True, email_students=True, email_params=email_params) + else: + # This email does not yet exist, so we need to create a new account + # If username already exists in the database, then create_and_enroll_user + # will raise an IntegrityError exception. + password = generate_unique_password(generated_passwords) + + try: + create_and_enroll_user(email, username, name, country, password, course_id) + except IntegrityError: + row_errors.append({ + 'username': username, 'email': email, 'response': _('Username {user} already exists.').format(user=username)}) + except Exception as ex: + log.exception(type(ex).__name__) + row_errors.append({ + 'username': username, 'email': email, 'response': _(type(ex).__name__)}) + else: + # It's a new user, an email will be sent to each newly created user. + email_params['message'] = 'account_creation_and_enrollment' + email_params['email_address'] = email + email_params['password'] = password + email_params['platform_name'] = microsite.get_value('platform_name', settings.PLATFORM_NAME) + send_mail_to_student(email, email_params) + log.info('email sent to new created user at {email}'.format(email=email)) + + else: + general_errors.append({ + 'username': '', 'email': '', 'response': _('File is not attached.') + }) + + results = { + 'row_errors': row_errors, + 'general_errors': general_errors, + 'warnings': warnings + } + return JsonResponse(results) + + +def generate_random_string(length): + """ + Create a string of random characters of specified length + """ + chars = [ + char for char in string.ascii_uppercase + string.digits + string.ascii_lowercase + if char not in 'aAeEiIoOuU1l' + ] + + return string.join((random.choice(chars) for __ in range(length)), '') + + +def generate_unique_password(generated_passwords, password_length=12): + """ + generate a unique password for each student. + """ + + password = generate_random_string(password_length) + while password in generated_passwords: + password = generate_random_string(password_length) + + generated_passwords.append(password) + + return password + + +def create_and_enroll_user(email, username, name, country, password, course_id): + """ Creates a user and enroll him/her in the course""" + + user = User.objects.create_user(username, email, password) + reg = Registration() + reg.register(user) + + profile = UserProfile(user=user) + profile.name = name + profile.country = country + profile.save() + + # try to enroll the user in this course + CourseEnrollment.enroll(user, course_id) + + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') @@ -852,13 +1035,8 @@ def random_code_generator(): generate a random alphanumeric code of length defined in REGISTRATION_CODE_LENGTH settings """ - chars = '' - for char in string.ascii_uppercase + string.digits + string.ascii_lowercase: - # removing vowel words and specific characters - chars += char.strip('aAeEiIoOuU1l') - code_length = getattr(settings, 'REGISTRATION_CODE_LENGTH', 8) - return string.join((random.choice(chars) for _ in range(code_length)), '') + return generate_random_string(code_length) @ensure_csrf_cookie diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 4968f1b104..d7abd9269d 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -7,6 +7,8 @@ from django.conf.urls import patterns, url urlpatterns = patterns('', # nopep8 url(r'^students_update_enrollment$', 'instructor.views.api.students_update_enrollment', name="students_update_enrollment"), + url(r'^register_and_enroll_students$', + 'instructor.views.api.register_and_enroll_students', name="register_and_enroll_students"), url(r'^list_course_role_members$', 'instructor.views.api.list_course_role_members', name="list_course_role_members"), url(r'^modify_access$', diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 12e01a32c3..cdcf2b4e31 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -250,6 +250,7 @@ def _section_membership(course, access): 'access': access, 'enroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_key.to_deprecated_string()}), 'unenroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_key.to_deprecated_string()}), + 'upload_student_csv_button_url': reverse('register_and_enroll_students', kwargs={'course_id': course_key.to_deprecated_string()}), 'modify_beta_testers_button_url': reverse('bulk_beta_modify_access', kwargs={'course_id': course_key.to_deprecated_string()}), 'list_course_role_members_url': reverse('list_course_role_members', kwargs={'course_id': course_key.to_deprecated_string()}), 'modify_access_url': reverse('modify_access', kwargs={'course_id': course_key.to_deprecated_string()}), diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 14ce5e39df..928da2360e 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -223,14 +223,21 @@ class Order(models.Model): if is_order_type_business: for cart_item in cart_items: if hasattr(cart_item, 'paidcourseregistration'): - CourseRegCodeItem.add_to_order(self, cart_item.paidcourseregistration.course_id, cart_item.qty) + course_reg_code_item = CourseRegCodeItem.add_to_order(self, cart_item.paidcourseregistration.course_id, cart_item.qty) + # update the discounted prices if coupon redemption applied + course_reg_code_item.list_price = cart_item.list_price + course_reg_code_item.unit_cost = cart_item.unit_cost + course_reg_code_item.save() items_to_delete.append(cart_item) else: for cart_item in cart_items: if hasattr(cart_item, 'courseregcodeitem'): - PaidCourseRegistration.add_to_order(self, cart_item.courseregcodeitem.course_id) + paid_course_registration = PaidCourseRegistration.add_to_order(self, cart_item.courseregcodeitem.course_id) + # update the discounted prices if coupon redemption applied + paid_course_registration.list_price = cart_item.list_price + paid_course_registration.unit_cost = cart_item.unit_cost + paid_course_registration.save() items_to_delete.append(cart_item) - # CourseRegCodeItem.add_to_order for item in items_to_delete: item.delete() @@ -254,7 +261,7 @@ class Order(models.Model): for registration_code in registration_codes: redemption_url = reverse('register_code_redemption', args=[registration_code.code]) url = '{base_url}{redemption_url}'.format(base_url=site_name, redemption_url=redemption_url) - csv_writer.writerow([course.display_name, registration_code.code, url]) + csv_writer.writerow([unicode(course.display_name).encode("utf-8"), registration_code.code, url]) return csv_file, course_info @@ -312,7 +319,12 @@ class Order(models.Model): from_email=from_address, to=[recipient[1]] ) - email.content_subtype = "html" + + # only the business order is HTML formatted + # the single seat is simple text + if is_order_type_business: + email.content_subtype = "html" + if csv_file: email.attach(u'RegistrationCodesRedemptionUrls.csv', csv_file.getvalue(), 'text/csv') email.send() diff --git a/lms/envs/common.py b/lms/envs/common.py index c57ff34565..5569e6223f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -287,6 +287,11 @@ FEATURES = { # Enable the new dashboard, account, and profile pages 'ENABLE_NEW_DASHBOARD': False, + + # Show a section in the membership tab of the instructor dashboard + # to allow an upload of a CSV file that contains a list of new accounts to create + # and register for course. + 'ALLOW_AUTOMATED_SIGNUPS': False, } # Ignore static asset files on import which match this pattern diff --git a/lms/lib/xblock/runtime.py b/lms/lib/xblock/runtime.py index f02657c511..6195048190 100644 --- a/lms/lib/xblock/runtime.py +++ b/lms/lib/xblock/runtime.py @@ -196,3 +196,11 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract ) services['fs'] = xblock.reference.plugins.FSService() super(LmsModuleSystem, self).__init__(**kwargs) + + # backward compatibility fix for callers not knowing this is a ModuleSystem v DescriptorSystem + @property + def resources_fs(self): + """ + Return what would be the resources_fs on a DescriptorSystem + """ + return getattr(self, 'filestore', None) diff --git a/lms/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index 744c2b225e..132ae7e1d0 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/lms/static/coffee/src/instructor_dashboard/membership.coffee @@ -141,7 +141,7 @@ class AuthListWidget extends MemberListWidget url: @list_endpoint data: rolename: @rolename success: (data) => cb? null, data[@rolename] - error: std_ajax_err => + error: std_ajax_err => `// Translators: A rolename appears this sentence. A rolename is something like "staff" or "beta tester".` cb? gettext("Error fetching list for role") + " '#{@rolename}'" @@ -174,6 +174,108 @@ class AuthListWidget extends MemberListWidget else @reload_list() +class AutoEnrollmentViaCsv + constructor: (@$container) -> + # Wrapper for the AutoEnrollmentViaCsv subsection. + # This object handles buttons, success and failure reporting, + # and server communication. + @$student_enrollment_form = @$container.find("form#student-auto-enroll-form") + @$enrollment_signup_button = @$container.find("[name='enrollment_signup_button']") + @$students_list_file = @$container.find("input[name='students_list']") + @$csrf_token = @$container.find("input[name='csrfmiddlewaretoken']") + @$results = @$container.find("div.results") + @$browse_button = @$container.find("#browseBtn") + @$browse_file = @$container.find("#browseFile") + + @processing = false + + @$browse_button.on "change", (event) => + if event.currentTarget.files.length == 1 + @$browse_file.val(event.currentTarget.value.substring(event.currentTarget.value.lastIndexOf("\\") + 1)) + + # attach click handler for @$enrollment_signup_button + @$enrollment_signup_button.click => + @$student_enrollment_form.submit (event) => + if @processing + return false + + @processing = true + + event.preventDefault() + data = new FormData(event.currentTarget) + $.ajax + dataType: 'json' + type: 'POST' + url: event.currentTarget.action + data: data + processData: false + contentType: false + success: (data) => + @processing = false + @display_response data + + return false + + display_response: (data_from_server) -> + @$results.empty() + errors = [] + warnings = [] + + result_from_server_is_success = true + + if data_from_server.general_errors.length + result_from_server_is_success = false + for general_error in data_from_server.general_errors + general_error['is_general_error'] = true + errors.push general_error + + if data_from_server.row_errors.length + result_from_server_is_success = false + for error in data_from_server.row_errors + error['is_general_error'] = false + errors.push error + + if data_from_server.warnings.length + result_from_server_is_success = false + for warning in data_from_server.warnings + warning['is_general_error'] = false + warnings.push warning + + render_response = (label, type, student_results) => + if type is 'success' + task_res_section = $ '
', class: 'message message-confirmation' + message_title = $ '', class: 'message-title', text: label + task_res_section.append message_title + @$results.append task_res_section + return + + if type is 'error' + task_res_section = $ '', class: 'message message-error' + if type is 'warning' + task_res_section = $ '', class: 'message message-warning' + + message_title = $ '', class: 'message-title', text: label + task_res_section. append message_title + messages_copy = $ '', class: 'message-copy' + task_res_section. append messages_copy + messages_summary = $ '