Merge pull request #21723 from edx/aehsan/prod-655/configuration_model_added_for_bulk_unenroll

Configuration model added for bulk unenroll command

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.

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()]))

common/djangoapps/student/management/tests/test_bulk_unenroll.py

 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()])
+                )
+            )

common/djangoapps/student/migrations/0023_bulkunenrollconfiguration.py

+# -*- 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,
+            },
+        ),
+    ]

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):
     """