diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index c6a088877971cf803bb064ab751aa6ea3e21226e..b24d395ed4a63973d5877de6ca12246babb31e3d 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -37,7 +37,8 @@ from student.models import ( RegistrationCookieConfiguration, UserAttribute, UserProfile, - UserTestGroup + UserTestGroup, + BulkUnenrollConfiguration ) from student.roles import REGISTERED_ACCESS_ROLES from xmodule.modulestore.django import modulestore @@ -445,6 +446,7 @@ admin.site.register(Registration) admin.site.register(PendingNameChange) admin.site.register(DashboardConfiguration, ConfigurationModelAdmin) admin.site.register(RegistrationCookieConfiguration, ConfigurationModelAdmin) +admin.site.register(BulkUnenrollConfiguration, ConfigurationModelAdmin) # We must first un-register the User model since it may also be registered by the auth app. diff --git a/common/djangoapps/student/management/commands/bulk_unenroll.py b/common/djangoapps/student/management/commands/bulk_unenroll.py index 6ac0a4f0a1f72bcdc0cc2a8bf4dd279d4feea824..e9932afe8528366c651e1050705e0adcf0cd4327 100644 --- a/common/djangoapps/student/management/commands/bulk_unenroll.py +++ b/common/djangoapps/student/management/commands/bulk_unenroll.py @@ -5,11 +5,9 @@ import logging import unicodecsv from django.core.exceptions import ObjectDoesNotExist from django.core.management.base import BaseCommand -from django.db.models import Q from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey - -from student.models import CourseEnrollment, User +from student.models import CourseEnrollment, User, BulkUnenrollConfiguration logger = logging.getLogger(__name__) # pylint: disable=invalid-name @@ -27,39 +25,48 @@ class Command(BaseCommand): parser.add_argument('-p', '--csv_path', metavar='csv_path', dest='csv_path', - required=True, + required=False, help='Path to CSV file.') def handle(self, *args, **options): + csv_path = options['csv_path'] - with open(csv_path, 'rb') as csvfile: - reader = unicodecsv.DictReader(csvfile) - for row in reader: - username = row['username'] - email = row['email'] - course_key = row['course_id'] - try: - user = User.objects.get(Q(username=username) | Q(email=email)) - except ObjectDoesNotExist: - user = None - msg = 'User with username {} or email {} does not exist'.format(username, email) - logger.warning(msg) + if csv_path: + with open(csv_path) as csv_file: + self.unenroll_users(csv_file) + else: + csv_file = BulkUnenrollConfiguration.current().csv_file + self.unenroll_users(csv_file) + + def unenroll_users(self, csv_file): + reader = list(unicodecsv.DictReader(csv_file)) + users_unenrolled = {} + for row in reader: + username = row['username'] + course_key = row['course_id'] + + try: + course_id = CourseKey.from_string(row['course_id']) + except InvalidKeyError: + msg = 'Invalid course id {course_id}, skipping un-enrollement for {username}, {email}'.format(**row) + logger.warning(msg) + continue + + try: + enrollment = CourseEnrollment.objects.get(user__username=username, course_id=course_id) + enrollment.update_enrollment(is_active=False, skip_refund=True) + if username in users_unenrolled: + users_unenrolled[username].append(course_key.encode()) + else: + users_unenrolled[username] = [course_key.encode()] + + except ObjectDoesNotExist: + msg = 'Enrollment for the user {} in course {} does not exist!'.format(username, course_key) + logger.info(msg) - try: - course_id = CourseKey.from_string(course_key) - except InvalidKeyError: - course_id = None - msg = 'Invalid course id {course_id}, skipping un-enrollement for {username}, {email}'.format(**row) - logger.warning(msg) + except Exception as err: + msg = 'Error un-enrolling User {} from course {}: '.format(username, course_key, err) + logger.error(msg, exc_info=True) - if user and course_id: - enrollment = CourseEnrollment.get_enrollment(user, course_id) - if not enrollment: - msg = 'Enrollment for the user {} in course {} does not exist!'.format(username, course_key) - logger.info(msg) - else: - try: - CourseEnrollment.unenroll(user, course_id, skip_refund=True) - except Exception as err: - msg = 'Error un-enrolling User {} from course {}: '.format(username, course_key, err) - logger.error(msg, exc_info=True) + logger.info("Following users have been unenrolled successfully from the following courses: {users_unenrolled}" + .format(users_unenrolled=["{}:{}".format(k, v) for k, v in users_unenrolled.items()])) diff --git a/common/djangoapps/student/management/tests/test_bulk_unenroll.py b/common/djangoapps/student/management/tests/test_bulk_unenroll.py index 5a402e19684f291ff3421903562edd651ce28d66..3417b7c1a2ff4de613239231a5190128228bb5e9 100644 --- a/common/djangoapps/student/management/tests/test_bulk_unenroll.py +++ b/common/djangoapps/student/management/tests/test_bulk_unenroll.py @@ -1,14 +1,14 @@ from __future__ import absolute_import import six -from tempfile import NamedTemporaryFile - -from django.core.management import call_command -from testfixtures import LogCapture +from tempfile import NamedTemporaryFile from course_modes.tests.factories import CourseModeFactory -from student.models import CourseEnrollment +from django.core.files.uploadedfile import SimpleUploadedFile +from django.core.management import call_command +from student.models import CourseEnrollment, BulkUnenrollConfiguration from student.tests.factories import UserFactory +from testfixtures import LogCapture from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -47,21 +47,6 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): csv.seek(0) return csv - def test_user_not_exist(self): - """Verify that warning user not exist is logged for non existing user.""" - with NamedTemporaryFile() as csv: - csv = self._write_test_csv(csv, lines=["111,test,test@example.com,course-v1:edX+DemoX+Demo_Course\n"]) - - with LogCapture(LOGGER_NAME) as log: - call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) - log.check( - ( - LOGGER_NAME, - 'WARNING', - 'User with username {} or email {} does not exist'.format('test', 'test@example.com') - ) - ) - def test_invalid_course_key(self): """Verify in case of invalid course key warning is logged.""" with NamedTemporaryFile() as csv: @@ -69,13 +54,11 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): with LogCapture(LOGGER_NAME) as log: call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) - log.check( - ( - LOGGER_NAME, - 'WARNING', - 'Invalid course id {}, skipping un-enrollement for {}, {}'.format( - 'test_course', 'amy', 'amy@pond.com') - ) + expected_message = 'Invalid course id {}, skipping un-enrollement for {}, {}'.\ + format('test_course', 'amy', 'amy@pond.com') + + log.check_present( + (LOGGER_NAME, 'WARNING', expected_message) ) def test_user_not_enrolled(self): @@ -85,13 +68,11 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): with LogCapture(LOGGER_NAME) as log: call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) - log.check( - ( - LOGGER_NAME, - 'INFO', - 'Enrollment for the user {} in course {} does not exist!'.format( - 'amy', 'course-v1:edX+DemoX+Demo_Course') - ) + expected_message = 'Enrollment for the user {} in course {} does not exist!'.\ + format('amy', 'course-v1:edX+DemoX+Demo_Course') + + log.check_present( + (LOGGER_NAME, 'INFO', expected_message) ) def test_bulk_un_enroll(self): @@ -107,3 +88,46 @@ class BulkUnenrollTests(SharedModuleStoreTestCase): call_command("bulk_unenroll", "--csv_path={}".format(csv.name)) for enrollment in CourseEnrollment.objects.all(): self.assertEqual(enrollment.is_active, False) + + def test_bulk_unenroll_from_config_model(self): + """Verify users are unenrolled using the command.""" + lines = "user_id,username,email,course_id\n" + for enrollment in self.enrollments: + lines += str(enrollment.user.id) + "," + enrollment.user.username + "," + \ + enrollment.user.email + "," + str(enrollment.course.id) + "\n" + + csv_file = SimpleUploadedFile(name='test.csv', content=lines, content_type='text/csv') + BulkUnenrollConfiguration.objects.create(enabled=True, csv_file=csv_file) + + call_command("bulk_unenroll") + for enrollment in CourseEnrollment.objects.all(): + self.assertEqual(enrollment.is_active, False) + + def test_users_unenroll_successfully_logged(self): + """Verify users unenrolled are logged """ + lines = "user_id,username,email,course_id\n" + users_unenrolled = {} + for enrollment in self.enrollments: + username = enrollment.user.username + if username in users_unenrolled: + users_unenrolled[username].append(str(enrollment.course.id)) + else: + users_unenrolled[username] = [str(enrollment.course.id)] + + lines += str(enrollment.user.id) + "," + username + "," + \ + enrollment.user.email + "," + str(enrollment.course.id) + "\n" + + csv_file = SimpleUploadedFile(name='test.csv', content=lines, content_type='text/csv') + BulkUnenrollConfiguration.objects.create(enabled=True, csv_file=csv_file) + + with LogCapture(LOGGER_NAME) as log: + call_command("bulk_unenroll") + log.check( + ( + LOGGER_NAME, + 'INFO', + 'Following users have been unenrolled successfully from the following courses:' + ' {users_unenrolled}'.format(users_unenrolled=["{}:{}".format(k, v) for k, v in + users_unenrolled.items()]) + ) + ) diff --git a/common/djangoapps/student/migrations/0023_bulkunenrollconfiguration.py b/common/djangoapps/student/migrations/0023_bulkunenrollconfiguration.py new file mode 100644 index 0000000000000000000000000000000000000000..774a70b2544ea8ab682158c4b44705d940511c1b --- /dev/null +++ b/common/djangoapps/student/migrations/0023_bulkunenrollconfiguration.py @@ -0,0 +1,33 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.24 on 2019-09-19 19:51 +from __future__ import unicode_literals + +from django.conf import settings +import django.core.validators +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('student', '0022_indexing_in_courseenrollment'), + ] + + operations = [ + migrations.CreateModel( + name='BulkUnenrollConfiguration', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('csv_file', models.FileField(help_text='It expect that the data will be provided in a csv file format with first row being the header and columns will be as follows: user_id, username, email, course_id, is_verified, verification_date', upload_to=b'', validators=[django.core.validators.FileExtensionValidator(allowed_extensions=[b'csv'])])), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ], + options={ + 'ordering': ('-change_date',), + 'abstract': False, + }, + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 4a29fe8eff8612fb118db89dcd65bb4b4df7b29d..a47b608edaad1c0d0df07d8336931295034391ab 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -18,6 +18,7 @@ import logging import uuid from collections import OrderedDict, defaultdict, namedtuple from datetime import datetime, timedelta +from django.core.validators import FileExtensionValidator from functools import total_ordering from importlib import import_module @@ -2845,6 +2846,18 @@ class RegistrationCookieConfiguration(ConfigurationModel): ) +class BulkUnenrollConfiguration(ConfigurationModel): + """ + + """ + csv_file = models.FileField( + validators=[FileExtensionValidator(allowed_extensions=['csv'])], + help_text=_("It expect that the data will be provided in a csv file format with \ + first row being the header and columns will be as follows: \ + user_id, username, email, course_id, is_verified, verification_date") + ) + + @python_2_unicode_compatible class UserAttribute(TimeStampedModel): """