Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Shift enroll/unenroll logic to CourseEnrollment model, add is_active and mode.

Browse Source 
Features coming down the pipe will want to be able to:
* Refer to enrollments before they are actually activated (approval step).
* See what courses a user used to be enrolled in for when they re-enroll in
  the same course, or a different run of that course.
* Have different "modes" of enrolling in a course, representing things like
  honor certificate enrollment, auditing (no certs), etc.

This change adds an is_active flag and mode (with default being "honor").
The commit is only as large as it is because many parts of the codebase were
manipulating enrollments by adding and removing CourseEnrollment objects
directly. It was necessary to create classmethods on CourseEnrollment to
encapsulate this functionality and then port everything over to using them.

The migration to add columns has been tested on a prod replica, and seems to be
fine for running on a live system with single digit millions of rows of
enrollments.
This commit is contained in:
David Ormsbee
2013-08-12 09:20:13 -04:00
parent cd838e2652
commit 3ce87583ab
32 changed files with 733 additions and 143 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
lms/djangoapps/instructor/enrollment.py
View File

@@ -14,7 +14,11 @@ class EmailEnrollmentState(object):
""" Store the complete enrollment state of an email in a class """
def __init__(self, course_id, email):
exists_user = User.objects.filter(email=email).exists()
exists_ce = CourseEnrollment.objects.filter(course_id=course_id, user__email=email).exists()
if exists_user:
user = User.objects.get(email=email)
exists_ce = CourseEnrollment.is_enrolled(user, course_id)
else:
exists_ce = False
ceas = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=email).all()
exists_allowed = len(ceas) > 0
state_auto_enroll = exists_allowed and ceas[0].auto_enroll
@@ -66,8 +70,7 @@ def enroll_email(course_id, student_email, auto_enroll=False):
previous_state = EmailEnrollmentState(course_id, student_email)
if previous_state.user:
user = User.objects.get(email=student_email)
CourseEnrollment.objects.get_or_create(course_id=course_id, user=user)
CourseEnrollment.enroll_by_email(student_email, course_id)
else:
cea, _ = CourseEnrollmentAllowed.objects.get_or_create(course_id=course_id, email=student_email)
cea.auto_enroll = auto_enroll
@@ -91,7 +94,7 @@ def unenroll_email(course_id, student_email):
previous_state = EmailEnrollmentState(course_id, student_email)
if previous_state.enrollment:
CourseEnrollment.objects.get(course_id=course_id, user__email=student_email).delete()
CourseEnrollment.unenroll_by_email(student_email, course_id)
if previous_state.allowed:
CourseEnrollmentAllowed.objects.get(course_id=course_id, email=student_email).delete()

5
lms/djangoapps/instructor/offline_gradecalc.py
View File

@@ -31,7 +31,10 @@ def offline_grade_calculation(course_id):
'''
tstart = time.time()
enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).prefetch_related("groups").order_by('username')
enrolled_students = User.objects.filter(
courseenrollment__course_id=course_id,
courseenrollment__is_active=1
).prefetch_related("groups").order_by('username')
enc = MyEncoder()

17
lms/djangoapps/instructor/tests/test_api.py
View File

@@ -98,7 +98,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
def setUp(self):
self.user = UserFactory.create()
self.course = CourseFactory.create()
CourseEnrollment.objects.create(user=self.user, course_id=self.course.id)
CourseEnrollment.enroll(self.user, self.course.id)
self.client.login(username=self.user.username, password='test')
def test_deny_students_update_enrollment(self):
@@ -161,9 +161,9 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.client.login(username=self.instructor.username, password='test')
self.enrolled_student = UserFactory()
CourseEnrollment.objects.create(
user=self.enrolled_student,
course_id=self.course.id
CourseEnrollment.enroll(
self.enrolled_student,
self.course.id
)
self.notenrolled_student = UserFactory()
@@ -237,7 +237,8 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.assertEqual(
self.enrolled_student.courseenrollment_set.filter(
course_id=self.course.id
course_id=self.course.id,
is_active=1,
).count(),
0
)
@@ -425,7 +426,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
self.students = [UserFactory() for _ in xrange(6)]
for student in self.students:
CourseEnrollment.objects.create(user=student, course_id=self.course.id)
CourseEnrollment.enroll(student, self.course.id)
def test_get_students_features(self):
"""
@@ -535,7 +536,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
self.client.login(username=self.instructor.username, password='test')
self.student = UserFactory()
CourseEnrollment.objects.create(course_id=self.course.id, user=self.student)
CourseEnrollment.enroll(self.student, self.course.id)
self.problem_urlname = 'robot-some-problem-urlname'
self.module_to_reset = StudentModule.objects.create(
@@ -678,7 +679,7 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.client.login(username=self.instructor.username, password='test')
self.student = UserFactory()
CourseEnrollment.objects.create(course_id=self.course.id, user=self.student)
CourseEnrollment.enroll(self.student, self.course.id)
self.problem_urlname = 'robot-some-problem-urlname'
self.module = StudentModule.objects.create(

5
lms/djangoapps/instructor/tests/test_enrollment.py
View File

@@ -353,10 +353,7 @@ class SettableEnrollmentState(EmailEnrollmentState):
user = UserFactory()
email = user.email
if self.enrollment:
cenr = CourseEnrollment.objects.create(
user=user,
course_id=course_id
)
cenr = CourseEnrollment.enroll(user, course_id)
return EnrollmentObjects(email, user, cenr, None)
else:
return EnrollmentObjects(email, user, None, None)

31
lms/djangoapps/instructor/tests/test_legacy_enrollment.py
View File

@@ -51,7 +51,13 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
# Run the Un-enroll students command
url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student0@test.com student1@test.com'})
response = self.client.post(
url,
{
'action': 'Unenroll multiple students',
'multiple_students': 'student0@test.com student1@test.com'
}
)
# Check the page output
self.assertContains(response, '<td>student0@test.com</td>')
@@ -60,12 +66,10 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
# Check the enrollment table
user = User.objects.get(email='student0@test.com')
ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
self.assertEqual(0, len(ce))
self.assertFalse(CourseEnrollment.is_enrolled(user, course.id))
user = User.objects.get(email='student1@test.com')
ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
self.assertEqual(0, len(ce))
self.assertFalse(CourseEnrollment.is_enrolled(user, course.id))
# Check the outbox
self.assertEqual(len(mail.outbox), 0)
@@ -96,7 +100,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
self.assertEqual(1, cea[0].auto_enroll)
# Check there is no enrollment db entry other than for the other students
ce = CourseEnrollment.objects.filter(course_id=course.id)
ce = CourseEnrollment.objects.filter(course_id=course.id, is_active=1)
self.assertEqual(4, len(ce))
# Create and activate student accounts with same email
@@ -111,12 +115,10 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
# Check students are enrolled
user = User.objects.get(email='student1_1@test.com')
ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
self.assertEqual(1, len(ce))
self.assertTrue(CourseEnrollment.is_enrolled(user, course.id))
user = User.objects.get(email='student1_2@test.com')
ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
self.assertEqual(1, len(ce))
self.assertTrue(CourseEnrollment.is_enrolled(user, course.id))
def test_repeat_enroll(self):
"""
@@ -156,7 +158,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
self.assertEqual(0, cea[0].auto_enroll)
# Check there is no enrollment db entry other than for the setup instructor and students
ce = CourseEnrollment.objects.filter(course_id=course.id)
ce = CourseEnrollment.objects.filter(course_id=course.id, is_active=1)
self.assertEqual(4, len(ce))
# Create and activate student accounts with same email
@@ -171,11 +173,10 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
# Check students are not enrolled
user = User.objects.get(email='student2_1@test.com')
ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
self.assertEqual(0, len(ce))
self.assertFalse(CourseEnrollment.is_enrolled(user, course.id))
user = User.objects.get(email='student2_2@test.com')
ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
self.assertEqual(0, len(ce))
self.assertFalse(CourseEnrollment.is_enrolled(user, course.id))
def test_get_and_clean_student_list(self):
"""

29
lms/djangoapps/instructor/views/legacy.py
View File

@@ -93,7 +93,7 @@ def instructor_dashboard(request, course_id):
datatable = {'header': ['Statistic', 'Value'],
'title': 'Course Statistics At A Glance',
}
data = [['# Enrolled', CourseEnrollment.objects.filter(course_id=course_id).count()]]
data = [['# Enrolled', CourseEnrollment.objects.filter(course_id=course_id, is_active=1).count()]]
data += [['Date', timezone.now().isoformat()]]
data += compute_course_stats(course).items()
if request.user.is_staff:
@@ -530,7 +530,10 @@ def instructor_dashboard(request, course_id):
# DataDump
elif 'Download CSV of all student profile data' in action:
enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).order_by('username').select_related("profile")
enrolled_students = User.objects.filter(
courseenrollment__course_id=course_id,
courseenrollment__is_active=1,
).order_by('username').select_related("profile")
profkeys = ['name', 'language', 'location', 'year_of_birth', 'gender', 'level_of_education',
'mailing_address', 'goals']
datatable = {'header': ['username', 'email'] + profkeys}
@@ -1002,7 +1005,10 @@ def get_student_grade_summary_data(request, course, course_id, get_grades=True,
If get_raw_scores=True, then instead of grade summaries, the raw grades for all graded modules are returned.
'''
enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).prefetch_related("groups").order_by('username')
enrolled_students = User.objects.filter(
courseenrollment__course_id=course_id,
courseenrollment__is_active=1,
).prefetch_related("groups").order_by('username')
header = ['ID', 'Username', 'Full Name', 'edX email', 'External email']
assignments = []
@@ -1053,7 +1059,10 @@ def gradebook(request, course_id):
"""
course = get_course_with_access(request.user, course_id, 'staff', depth=None)
enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).order_by('username').select_related("profile")
enrolled_students = User.objects.filter(
courseenrollment__course_id=course_id,
courseenrollment__is_active=1
).order_by('username').select_related("profile")
# TODO (vshnayder): implement pagination.
enrolled_students = enrolled_students[:1000] # HACK!
@@ -1110,7 +1119,7 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
for ce in todelete:
if not has_access(ce.user, course, 'staff') and ce.user.email.lower() not in new_students_lc:
status[ce.user.email] = 'deleted'
ce.delete()
ce.deactivate()
else:
status[ce.user.email] = 'is staff'
ceaset = CourseEnrollmentAllowed.objects.filter(course_id=course_id)
@@ -1162,14 +1171,13 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
continue
#Student has already registered
if CourseEnrollment.objects.filter(user=user, course_id=course_id):
if CourseEnrollment.is_enrolled(user, course_id):
status[student] = 'already enrolled'
continue
try:
#Not enrolled yet
ce = CourseEnrollment(user=user, course_id=course_id)
ce.save()
ce = CourseEnrollment.enroll(user, course_id)
status[student] = 'added'
if email_students:
@@ -1239,11 +1247,10 @@ def _do_unenroll_students(course_id, students, email_students=False):
continue
ce = CourseEnrollment.objects.filter(user=user, course_id=course_id)
#Will be 0 or 1 records as there is a unique key on user + course_id
if ce:
if CourseEnrollment.is_enrolled(user, course_id):
try:
ce[0].delete()
CourseEnrollment.unenroll(user, course_id)
status[student] = "un-enrolled"
if email_students:
#User was enrolled