From 4563b87236d46d8f43ee8d0eb118150a85138c31 Mon Sep 17 00:00:00 2001
From: uzairr <uzairr@yahoo.com>
Date: Fri, 27 Mar 2020 19:16:11 +0500
Subject: [PATCH] Refactor bulk unenroll mgmt cmd
Bulk unenroll command is not supporting unenrolling all the learners
from multiple courses.This PR will enhance this mgmt cmd so that
it can support un-enrollment of learners from multiple courses.
PROD-1347
---
.../management/commands/bulk_unenroll.py | 71 +++++--------------
.../management/tests/test_bulk_unenroll.py | 65 ++++++-----------
2 files changed, 41 insertions(+), 95 deletions(-)
diff --git a/common/djangoapps/student/management/commands/bulk_unenroll.py b/common/djangoapps/student/management/commands/bulk_unenroll.py
index 12d043a1e7b..70aa59687c4 100644
--- a/common/djangoapps/student/management/commands/bulk_unenroll.py
+++ b/common/djangoapps/student/management/commands/bulk_unenroll.py
@@ -1,5 +1,5 @@
"""
-Un-enroll Bulk users course wide as well as provided in csv
+Un-enroll Bulk users course wide as well as specified in csv
"""
import logging
@@ -20,8 +20,11 @@ class Command(BaseCommand):
help = """"
Un-enroll bulk users from the courses.
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
+ first row being the header and columns will be either one of the
+ following:
+ username,course-id
+ OR
+ course-id
"""
def add_arguments(self, parser):
@@ -30,20 +33,9 @@ class Command(BaseCommand):
dest='csv_path',
required=False,
help='Path to CSV file.')
- parser.add_argument('-c', '--course-id',
- metavar='course_id',
- dest='course-id',
- required=False,
- help='unenroll all users from provided course-id')
def handle(self, *args, **options):
-
csv_path = options['csv_path']
- course_id = options['course-id']
-
- if course_id:
- self.unenroll_all_users(course_id=course_id)
- return
if csv_path:
with open(csv_path, 'rb') as csv_file:
@@ -57,52 +49,25 @@ class Command(BaseCommand):
Un-enroll the given set of users provided in csv
"""
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()]
+ self.unenroll_learners(row.get('course_id'), username=row.get('username', None))
- except ObjectDoesNotExist:
- msg = 'Enrollment for the user {} in course {} does not exist!'.format(username, course_key)
- logger.info(msg)
-
- except Exception as err: # pylint: disable=W0703
- msg = 'Error un-enrolling User {} from course {}: with error: {}'.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()]))
-
- def unenroll_all_users(self, course_id):
+ def unenroll_learners(self, course_id, username=None):
"""
- Un-enroll all users from a given course
+ Un-enroll learners from course_id(s)
"""
try:
- course_id = CourseKey.from_string(course_id)
+ course_key = CourseKey.from_string(course_id)
except InvalidKeyError:
msg = 'Invalid course id {}, skipping un-enrollement.'.format(course_id)
logger.warning(msg)
return
- try:
- updated_count = CourseEnrollment.objects.filter(course_id=course_id, is_active=True).update(is_active=False)
- logger.info("{} users have been unenrolled successfully from the provided course: {}"
- .format(updated_count, course_id))
- except Exception as err: # pylint: disable=W0703
- msg = 'Error un-enrolling Users from course {}: with error: {}'.format(course_id, err)
- logger.error(msg, exc_info=True)
+ enrollments = CourseEnrollment.objects.filter(course_id=course_key, is_active=True)
+ if username:
+ enrollments = enrollments.filter(user__username=username)
+
+ for enrollment in enrollments:
+ enrollment.update_enrollment(is_active=False, skip_refund=True)
+ logger.info("User [{}] have been successfully unenrolled from the course: {}"
+ .format(enrollment.user.username, course_key))
diff --git a/common/djangoapps/student/management/tests/test_bulk_unenroll.py b/common/djangoapps/student/management/tests/test_bulk_unenroll.py
index a755b9dbbf6..d8f02ee3c48 100644
--- a/common/djangoapps/student/management/tests/test_bulk_unenroll.py
+++ b/common/djangoapps/student/management/tests/test_bulk_unenroll.py
@@ -1,14 +1,15 @@
+"""Tests for Bulk Un-enroll Management command"""
+from tempfile import NamedTemporaryFile
import six
-
-from tempfile import NamedTemporaryFile
-from course_modes.tests.factories import CourseModeFactory
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 course_modes.tests.factories import CourseModeFactory
+from student.models import BulkUnenrollConfiguration, CourseEnrollment
+from student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -35,13 +36,17 @@ class BulkUnenrollTests(SharedModuleStoreTestCase):
self.users = []
for username, email, password in self.user_info:
- user = UserFactory.create(username=username, email=email, password=password)
+ user = UserFactory.create(
+ username=username, email=email, password=password
+ )
self.users.append(user)
- self.enrollments.append(CourseEnrollment.enroll(user, self.course.id, mode='audit'))
+ self.enrollments.append(
+ CourseEnrollment.enroll(user, self.course.id, mode='audit')
+ )
def _write_test_csv(self, csv, lines):
"""Write a test csv file with the lines provided"""
- csv.write(b"user_id,username,email,course_id\n")
+ csv.write(b"username,course_id\n")
for line in lines:
csv.write(six.b(line))
csv.seek(0)
@@ -50,36 +55,22 @@ class BulkUnenrollTests(SharedModuleStoreTestCase):
def test_invalid_course_key(self):
"""Verify in case of invalid course key warning is logged."""
with NamedTemporaryFile() as csv:
- csv = self._write_test_csv(csv, lines=["111,amy,amy@pond.com,test_course\n"])
+ csv = self._write_test_csv(csv, lines=["amy,test_course\n"])
with LogCapture(LOGGER_NAME) as log:
call_command("bulk_unenroll", "--csv_path={}".format(csv.name))
- expected_message = 'Invalid course id {}, skipping un-enrollement for {}, {}'.\
- format('test_course', 'amy', 'amy@pond.com')
+ expected_message = 'Invalid course id {}, skipping un-enrollement.'.\
+ format('test_course')
log.check_present(
(LOGGER_NAME, 'WARNING', expected_message)
)
- def test_user_not_enrolled(self):
- """Verify in case of user not enrolled in course warning is logged."""
- with NamedTemporaryFile() as csv:
- csv = self._write_test_csv(csv, lines=["111,amy,amy@pond.com,course-v1:edX+DemoX+Demo_Course\n"])
-
- with LogCapture(LOGGER_NAME) as log:
- call_command("bulk_unenroll", "--csv_path={}".format(csv.name))
- 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):
"""Verify users are unenrolled using the command."""
lines = [
- str(enrollment.user.id) + "," + enrollment.user.username + "," +
- enrollment.user.email + "," + str(enrollment.course.id) + "\n"
+ enrollment.user.username + "," +
+ str(enrollment.course.id) + "\n"
for enrollment in self.enrollments
]
with NamedTemporaryFile() as csv:
@@ -105,18 +96,8 @@ class BulkUnenrollTests(SharedModuleStoreTestCase):
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).encode('utf-8'))
- else:
- users_unenrolled[username] = [str(enrollment.course.id).encode('utf-8')]
-
- lines += str(enrollment.user.id) + "," + username + "," + \
- enrollment.user.email + "," + str(enrollment.course.id) + "\n"
-
+ lines = "username,course_id\n"
+ lines += self.enrollments[0].username + "," + str(self.enrollments[0].course.id) + "\n"
csv_file = SimpleUploadedFile(name='test.csv', content=lines.encode('utf-8'), content_type='text/csv')
BulkUnenrollConfiguration.objects.create(enabled=True, csv_file=csv_file)
@@ -126,8 +107,8 @@ class BulkUnenrollTests(SharedModuleStoreTestCase):
(
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()])
+ 'User [{}] have been successfully unenrolled from the course: {}'.format(
+ self.enrollments[0].username, self.enrollments[0].course.id
+ )
)
)
--
GitLab