Commit 57a80635 by David Ormsbee

Merge pull request #651 from edx/ormsbee/enrollment_modes

Add mode and is_active to CourseEnrollment, shift enrollment logic to model
parents 6416a21a 3ce87583
......@@ -11,6 +11,15 @@ LMS: Enable beta instructor dashboard. The beta dashboard is a rearchitecture
of the existing instructor dashboard and is available by clicking a link at
the top right of the existing dashboard.
Common: CourseEnrollment has new fields `is_active` and `mode`. The mode will be
used to differentiate different kinds of enrollments (currently, all enrollments
are honor certificate enrollments). The `is_active` flag will be used to
deactivate enrollments without deleting them, so that we know what course you
*were* enrolled in. Because of the latter change, enrollment and unenrollment
logic has been consolidated into the model -- you should use new class methods
to `enroll()`, `unenroll()`, and to check `is_enrolled()`, instead of creating
CourseEnrollment objects or querying them directly.
Studio: Email will be sent to admin address when a user requests course creator
privileges for Studio (edge only).
......
......@@ -49,7 +49,7 @@ import datetime
from pytz import UTC
from uuid import uuid4
from pymongo import MongoClient
from student.views import is_enrolled_in_course
from student.models import CourseEnrollment
TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex
......@@ -1168,7 +1168,7 @@ class ContentStoreTest(ModuleStoreTestCase):
self.assertNotIn('ErrMsg', data)
self.assertEqual(data['id'], 'i4x://MITx/{0}/course/2013_Spring'.format(test_course_data['number']))
# Verify that the creator is now registered in the course.
self.assertTrue(is_enrolled_in_course(self.user, self._get_course_id(test_course_data)))
self.assertTrue(CourseEnrollment.is_enrolled(self.user, self._get_course_id(test_course_data)))
return test_course_data
def test_create_course_check_forum_seeding(self):
......@@ -1190,14 +1190,14 @@ class ContentStoreTest(ModuleStoreTestCase):
Checks that the course did not get created
"""
course_id = self._get_course_id(self.course_data)
initially_enrolled = is_enrolled_in_course(self.user, course_id)
initially_enrolled = CourseEnrollment.is_enrolled(self.user, course_id)
resp = self.client.post(reverse('create_new_course'), self.course_data)
self.assertEqual(resp.status_code, 200)
data = parse_json(resp)
self.assertEqual(data['ErrMsg'], error_message)
# One test case involves trying to create the same course twice. Hence for that course,
# the user will be enrolled. In the other cases, initially_enrolled will be False.
self.assertEqual(initially_enrolled, is_enrolled_in_course(self.user, course_id))
self.assertEqual(initially_enrolled, CourseEnrollment.is_enrolled(self.user, course_id))
def test_create_course_duplicate_number(self):
"""Test new course creation - error path"""
......
......@@ -6,7 +6,7 @@ from .utils import CourseTestCase
from django.contrib.auth.models import User, Group
from django.core.urlresolvers import reverse
from auth.authz import get_course_groupname_for_role
from student.views import is_enrolled_in_course
from student.models import CourseEnrollment
class UsersTestCase(CourseTestCase):
......@@ -372,13 +372,13 @@ class UsersTestCase(CourseTestCase):
def assert_not_enrolled(self):
""" Asserts that self.ext_user is not enrolled in self.course. """
self.assertFalse(
is_enrolled_in_course(self.ext_user, self.course.location.course_id),
CourseEnrollment.is_enrolled(self.ext_user, self.course.location.course_id),
'Did not expect ext_user to be enrolled in course'
)
def assert_enrolled(self):
""" Asserts that self.ext_user is enrolled in self.course. """
self.assertTrue(
is_enrolled_in_course(self.ext_user, self.course.location.course_id),
CourseEnrollment.is_enrolled(self.ext_user, self.course.location.course_id),
'User ext_user should have been enrolled in the course'
)
......@@ -44,7 +44,7 @@ from .component import (
from django_comment_common.utils import seed_permissions_roles
from student.views import enroll_in_course
from student.models import CourseEnrollment
from xmodule.html_module import AboutDescriptor
__all__ = ['course_index', 'create_new_course', 'course_info',
......@@ -165,7 +165,7 @@ def create_new_course(request):
seed_permissions_roles(new_course.location.course_id)
# auto-enroll the course creator in the course so that "View Live" will work.
enroll_in_course(request.user, new_course.location.course_id)
CourseEnrollment.enroll(request.user, new_course.location.course_id)
return JsonResponse({'id': new_course.location.url()})
......
......@@ -24,7 +24,7 @@ from course_creators.views import (
from .access import has_access
from student.views import enroll_in_course
from student.models import CourseEnrollment
@login_required
......@@ -208,7 +208,7 @@ def course_team_user(request, org, course, name, email):
user.groups.add(groups["instructor"])
user.save()
# auto-enroll the course creator in the course so that "View Live" will work.
enroll_in_course(user, location.course_id)
CourseEnrollment.enroll(user, location.course_id)
elif role == "staff":
# if we're trying to downgrade a user from "instructor" to "staff",
# make sure we have at least one other instructor in the course team.
......@@ -223,7 +223,7 @@ def course_team_user(request, org, course, name, email):
user.groups.add(groups["staff"])
user.save()
# auto-enroll the course creator in the course so that "View Live" will work.
enroll_in_course(user, location.course_id)
CourseEnrollment.enroll(user, location.course_id)
return JsonResponse()
......
......@@ -19,6 +19,20 @@ FORUM_ROLE_STUDENT = 'Student'
@receiver(post_save, sender=CourseEnrollment)
def assign_default_role(sender, instance, **kwargs):
# The code below would remove all forum Roles from a user when they unenroll
# from a course. Concerns were raised that it should apply only to students,
# or that even the history of student roles is important for research
# purposes. Since this was new functionality being added in this release,
# I'm just going to comment it out for now and let the forums team deal with
# implementing the right behavior.
#
# # We've unenrolled the student, so remove all roles for this course
# if not instance.is_active:
# course_roles = list(Role.objects.filter(course_id=instance.course_id))
# instance.user.roles.remove(*course_roles)
# return
# We've enrolled the student, so make sure they have a default role
if instance.user.is_staff:
role = Role.objects.get_or_create(course_id=instance.course_id, name="Moderator")[0]
else:
......
from django.test import TestCase
from django_comment_common.models import Role
from student.models import CourseEnrollment, User
class RoleAssignmentTest(TestCase):
"""
Basic checks to make sure our Roles get assigned and unassigned as students
are enrolled and unenrolled from a course.
"""
def setUp(self):
self.staff_user = User.objects.create_user(
"patty",
"patty@fake.edx.org",
)
self.staff_user.is_staff = True
self.student_user = User.objects.create_user(
"hacky",
"hacky@fake.edx.org"
)
self.course_id = "edX/Fake101/2012"
CourseEnrollment.enroll(self.staff_user, self.course_id)
CourseEnrollment.enroll(self.student_user, self.course_id)
def test_enrollment_auto_role_creation(self):
moderator_role = Role.objects.get(
course_id=self.course_id,
name="Moderator"
)
student_role = Role.objects.get(
course_id=self.course_id,
name="Student"
)
self.assertIn(moderator_role, self.staff_user.roles.all())
self.assertIn(student_role, self.student_user.roles.all())
self.assertNotIn(moderator_role, self.student_user.roles.all())
# The following was written on the assumption that unenrolling from a course
# should remove all forum Roles for that student for that course. This is
# not necessarily the case -- please see comments at the top of
# django_comment_client.models.assign_default_role(). Leaving it for the
# forums team to sort out.
#
# def test_unenrollment_auto_role_removal(self):
# another_student = User.objects.create_user("sol", "sol@fake.edx.org")
# CourseEnrollment.enroll(another_student, self.course_id)
#
# CourseEnrollment.unenroll(self.student_user, self.course_id)
# # Make sure we didn't delete the actual Role
# student_role = Role.objects.get(
# course_id=self.course_id,
# name="Student"
# )
# self.assertNotIn(student_role, self.student_user.roles.all())
# self.assertIn(student_role, another_student.roles.all())
......@@ -431,12 +431,12 @@ class ShibSPTest(ModuleStoreTestCase):
# If course is not limited or student has correct shib extauth then enrollment should be allowed
if course is open_enroll_course or student is shib_student:
self.assertEqual(response.status_code, 200)
self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 1)
self.assertTrue(CourseEnrollment.is_enrolled(student, course.id))
# Clean up
CourseEnrollment.objects.filter(user=student, course_id=course.id).delete()
CourseEnrollment.unenroll(student, course.id)
else:
self.assertEqual(response.status_code, 400)
self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 0)
self.assertFalse(CourseEnrollment.is_enrolled(student, course.id))
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
def test_shib_login_enrollment(self):
......@@ -462,7 +462,7 @@ class ShibSPTest(ModuleStoreTestCase):
# use django test client for sessions and url processing
# no enrollment before trying
self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 0)
self.assertFalse(CourseEnrollment.is_enrolled(student, course.id))
self.client.logout()
request_kwargs = {'path': '/shib-login/',
'data': {'enrollment_action': 'enroll', 'course_id': course.id},
......@@ -474,4 +474,4 @@ class ShibSPTest(ModuleStoreTestCase):
self.assertEqual(response.status_code, 302)
self.assertEqual(response['location'], 'http://testserver/')
# now there is enrollment
self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 1)
self.assertTrue(CourseEnrollment.is_enrolled(student, course.id))
......@@ -12,7 +12,7 @@ def create(n, course_id):
for i in range(n):
(user, user_profile, _) = _do_create_account(get_random_post_override())
if course_id is not None:
CourseEnrollment.objects.create(user=user, course_id=course_id)
CourseEnrollment.enroll(user, course_id)
class Command(BaseCommand):
......
......@@ -11,11 +11,11 @@ file and check it in at the same time as your model changes. To do that,
3. Add the migration file created in edx-platform/common/djangoapps/student/migrations/
"""
from datetime import datetime
from random import randint
import hashlib
import json
import logging
import uuid
from random import randint
from django.conf import settings
......@@ -645,16 +645,223 @@ class PendingEmailChange(models.Model):
class CourseEnrollment(models.Model):
"""
Represents a Student's Enrollment record for a single Course. You should
generally not manipulate CourseEnrollment objects directly, but use the
classmethods provided to enroll, unenroll, or check on the enrollment status
of a given student.
We're starting to consolidate course enrollment logic in this class, but
more should be brought in (such as checking against CourseEnrollmentAllowed,
checking course dates, user permissions, etc.) This logic is currently
scattered across our views.
"""
user = models.ForeignKey(User)
course_id = models.CharField(max_length=255, db_index=True)
created = models.DateTimeField(auto_now_add=True, null=True, db_index=True)
# If is_active is False, then the student is not considered to be enrolled
# in the course (is_enrolled() will return False)
is_active = models.BooleanField(default=True)
# Represents the modes that are possible. We'll update this later with a
# list of possible values.
mode = models.CharField(default="honor", max_length=100)
class Meta:
unique_together = (('user', 'course_id'),)
ordering = ('user', 'course_id')
def __unicode__(self):
return "[CourseEnrollment] %s: %s (%s)" % (self.user, self.course_id, self.created)
return (
"[CourseEnrollment] {}: {} ({}); active: ({})"
).format(self.user, self.course_id, self.created, self.is_active)
@classmethod
def create_enrollment(cls, user, course_id, mode="honor", is_active=False):
"""
Create an enrollment for a user in a class. By default *this enrollment
is not active*. This is useful for when an enrollment needs to go
through some sort of approval process before being activated. If you
don't need this functionality, just call `enroll()` instead.
Returns a CoursewareEnrollment object.
`user` is a Django User object. If it hasn't been saved yet (no `.id`
attribute), this method will automatically save it before
adding an enrollment for it.
`course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
`mode` is a string specifying what kind of enrollment this is. The
default is "honor", meaning honor certificate. Future options
may include "audit", "verified_id", etc. Please don't use it
until we have these mapped out.
`is_active` is a boolean. If the CourseEnrollment object has
`is_active=False`, then calling
`CourseEnrollment.is_enrolled()` for that user/course_id
will return False.
It is expected that this method is called from a method which has already
verified the user authentication and access.
"""
# If we're passing in a newly constructed (i.e. not yet persisted) User,
# save it to the database so that it can have an ID that we can throw
# into our CourseEnrollment object. Otherwise, we'll get an
# IntegrityError for having a null user_id.
if user.id is None:
user.save()
enrollment, _ = CourseEnrollment.objects.get_or_create(
user=user,
course_id=course_id,
)
# In case we're reactivating a deactivated enrollment, or changing the
# enrollment mode.
if enrollment.mode != mode or enrollment.is_active != is_active:
enrollment.mode = mode
enrollment.is_active = is_active
enrollment.save()
return enrollment
@classmethod
def enroll(cls, user, course_id, mode="honor"):
"""
Enroll a user in a course. This saves immediately.
Returns a CoursewareEnrollment object.
`user` is a Django User object. If it hasn't been saved yet (no `.id`
attribute), this method will automatically save it before
adding an enrollment for it.
`course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
`mode` is a string specifying what kind of enrollment this is. The
default is "honor", meaning honor certificate. Future options
may include "audit", "verified_id", etc. Please don't use it
until we have these mapped out.
It is expected that this method is called from a method which has already
verified the user authentication and access.
"""
return cls.create_enrollment(user, course_id, mode, is_active=True)
@classmethod
def enroll_by_email(cls, email, course_id, mode="honor", ignore_errors=True):
"""
Enroll a user in a course given their email. This saves immediately.
Note that enrolling by email is generally done in big batches and the
error rate is high. For that reason, we supress User lookup errors by
default.
Returns a CoursewareEnrollment object. If the User does not exist and
`ignore_errors` is set to `True`, it will return None.
`email` Email address of the User to add to enroll in the course.
`course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
`mode` is a string specifying what kind of enrollment this is. The
default is "honor", meaning honor certificate. Future options
may include "audit", "verified_id", etc. Please don't use it
until we have these mapped out.
`ignore_errors` is a boolean indicating whether we should suppress
`User.DoesNotExist` errors (returning None) or let it
bubble up.
It is expected that this method is called from a method which has already
verified the user authentication and access.
"""
try:
user = User.objects.get(email=email)
return cls.enroll(user, course_id, mode)
except User.DoesNotExist:
err_msg = u"Tried to enroll email {} into course {}, but user not found"
log.error(err_msg.format(email, course_id))
if ignore_errors:
return None
raise
@classmethod
def unenroll(cls, user, course_id):
"""
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.
`user` is a Django User object. If it hasn't been saved yet (no `.id`
attribute), this method will automatically save it before
adding an enrollment for it.
`course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
"""
try:
record = CourseEnrollment.objects.get(user=user, course_id=course_id)
record.is_active = False
record.save()
except cls.DoesNotExist:
log.error("Tried to unenroll student {} from {} but they were not enrolled")
@classmethod
def unenroll_by_email(cls, email, course_id):
"""
Unenroll a user from a course given their email. This saves immediately.
User lookup errors are logged but will not throw an exception.
`email` Email address of the User to unenroll from the course.
`course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
"""
try:
user = User.objects.get(email=email)
return cls.unenroll(user, course_id)
except User.DoesNotExist:
err_msg = u"Tried to unenroll email {} from course {}, but user not found"
log.error(err_msg.format(email, course_id))
@classmethod
def is_enrolled(cls, user, course_id):
"""
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.
Returns True if the user is enrolled in the course (the entry must exist
and it must have `is_active=True`). Otherwise, returns False.
`user` is a Django User object. If it hasn't been saved yet (no `.id`
attribute), this method will automatically save it before
adding an enrollment for it.
`course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
"""
try:
record = CourseEnrollment.objects.get(user=user, course_id=course_id)
return record.is_active
except cls.DoesNotExist:
return False
@classmethod
def enrollments_for_user(cls, user):
return CourseEnrollment.objects.filter(user=user, is_active=1)
def activate(self):
"""Makes this `CourseEnrollment` record active. Saves immediately."""
if not self.is_active:
self.is_active = True
self.save()
def deactivate(self):
"""Makes this `CourseEnrollment` record inactive. Saves immediately. An
inactive record means that the student is not enrolled in this course.
"""
if self.is_active:
self.is_active = False
self.save()
class CourseEnrollmentAllowed(models.Model):
......
......@@ -21,9 +21,8 @@ from django.utils.http import int_to_base36
from mock import Mock, patch
from textwrap import dedent
from student.models import unique_id_for_user
from student.models import unique_id_for_user, CourseEnrollment
from student.views import process_survey_link, _cert_info, password_reset, password_reset_confirm_wrapper
from student.views import enroll_in_course, is_enrolled_in_course
from student.tests.factories import UserFactory
from student.tests.test_email import mock_render_to_string
COURSE_1 = 'edX/toy/2012_Fall'
......@@ -209,12 +208,127 @@ class CourseEndingTest(TestCase):
class EnrollInCourseTest(TestCase):
""" Tests the helper method for enrolling a user in a class """
"""Tests enrolling and unenrolling in courses."""
def test_enroll_in_course(self):
def test_enrollment(self):
user = User.objects.create_user("joe", "joe@joe.com", "password")
user.save()
course_id = "course_id"
self.assertFalse(is_enrolled_in_course(user, course_id))
enroll_in_course(user, course_id)
self.assertTrue(is_enrolled_in_course(user, course_id))
course_id = "edX/Test101/2013"
# Test basic enrollment
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
CourseEnrollment.enroll(user, course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
# Enrolling them again should be harmless
CourseEnrollment.enroll(user, course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
# Now unenroll the user
CourseEnrollment.unenroll(user, course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# Unenrolling them again should also be harmless
CourseEnrollment.unenroll(user, course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# The enrollment record should still exist, just be inactive
enrollment_record = CourseEnrollment.objects.get(
user=user,
course_id=course_id
)
self.assertFalse(enrollment_record.is_active)
def test_enrollment_non_existent_user(self):
# Testing enrollment of newly unsaved user (i.e. no database entry)
user = User(username="rusty", email="rusty@fake.edx.org")
course_id = "edX/Test101/2013"
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# Unenroll does nothing
CourseEnrollment.unenroll(user, course_id)
# Implicit save() happens on new User object when enrolling, so this
# should still work
CourseEnrollment.enroll(user, course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
def test_enrollment_by_email(self):
user = User.objects.create(username="jack", email="jack@fake.edx.org")
course_id = "edX/Test101/2013"
CourseEnrollment.enroll_by_email("jack@fake.edx.org", course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
# This won't throw an exception, even though the user is not found
self.assertIsNone(
CourseEnrollment.enroll_by_email("not_jack@fake.edx.org", course_id)
)
self.assertRaises(
User.DoesNotExist,
CourseEnrollment.enroll_by_email,
"not_jack@fake.edx.org",
course_id,
ignore_errors=False
)
# Now unenroll them by email
CourseEnrollment.unenroll_by_email("jack@fake.edx.org", course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# Harmless second unenroll
CourseEnrollment.unenroll_by_email("jack@fake.edx.org", course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# Unenroll on non-existent user shouldn't throw an error
CourseEnrollment.unenroll_by_email("not_jack@fake.edx.org", course_id)
def test_enrollment_multiple_classes(self):
user = User(username="rusty", email="rusty@fake.edx.org")
course_id1 = "edX/Test101/2013"
course_id2 = "MITx/6.003z/2012"
CourseEnrollment.enroll(user, course_id1)
CourseEnrollment.enroll(user, course_id2)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id1))
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id2))
CourseEnrollment.unenroll(user, course_id1)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id1))
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id2))
CourseEnrollment.unenroll(user, course_id2)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id1))
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id2))
def test_activation(self):
user = User.objects.create(username="jack", email="jack@fake.edx.org")
course_id = "edX/Test101/2013"
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# Creating an enrollment doesn't actually enroll a student
# (calling CourseEnrollment.enroll() would have)
enrollment = CourseEnrollment.create_enrollment(user, course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# Until you explicitly activate it
enrollment.activate()
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
# Activating something that's already active does nothing
enrollment.activate()
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
# Now deactive
enrollment.deactivate()
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# Deactivating something that's already inactive does nothing
enrollment.deactivate()
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
# A deactivated enrollment should be activated if enroll() is called
# for that user/course_id combination
CourseEnrollment.enroll(user, course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
......@@ -254,13 +254,12 @@ def register_user(request, extra_context=None):
@ensure_csrf_cookie
def dashboard(request):
user = request.user
enrollments = CourseEnrollment.objects.filter(user=user)
# Build our courses list for the user, but ignore any courses that no longer
# exist (because the course IDs have changed). Still, we don't delete those
# enrollments, because it could have been a data push snafu.
courses = []
for enrollment in enrollments:
for enrollment in CourseEnrollment.enrollments_for_user(user):
try:
courses.append(course_from_id(enrollment.course_id))
except ItemNotFoundError:
......@@ -377,18 +376,13 @@ def change_enrollment(request):
"course:{0}".format(course_num),
"run:{0}".format(run)])
try:
enroll_in_course(user, course.id)
except IntegrityError:
# If we've already created this enrollment in a separate transaction,
# then just continue
pass
CourseEnrollment.enroll(user, course.id)
return HttpResponse()
elif action == "unenroll":
try:
enrollment = CourseEnrollment.objects.get(user=user, course_id=course_id)
enrollment.delete()
CourseEnrollment.unenroll(user, course_id)
org, course_num, run = course_id.split("/")
statsd.increment("common.student.unenrollment",
......@@ -402,30 +396,10 @@ def change_enrollment(request):
else:
return HttpResponseBadRequest(_("Enrollment action is invalid"))
def enroll_in_course(user, course_id):
"""
Helper method to enroll a user in a particular class.
It is expected that this method is called from a method which has already
verified the user authentication and access.
"""
CourseEnrollment.objects.get_or_create(user=user, course_id=course_id)
def is_enrolled_in_course(user, course_id):
"""
Helper method that returns whether or not the user is enrolled in a particular course.
"""
return CourseEnrollment.objects.filter(user=user, course_id=course_id).count() > 0
@ensure_csrf_cookie
def accounts_login(request, error=""):
return render_to_response('login.html', {'error': error})
# Need different levels of logging
@ensure_csrf_cookie
def login_user(request, error=""):
......@@ -1008,13 +982,21 @@ def activate_account(request, key):
ceas = CourseEnrollmentAllowed.objects.filter(email=student[0].email)
for cea in ceas:
if cea.auto_enroll:
course_id = cea.course_id
_enrollment, _created = CourseEnrollment.objects.get_or_create(user_id=student[0].id, course_id=course_id)
resp = render_to_response("registration/activation_complete.html", {'user_logged_in': user_logged_in, 'already_active': already_active})
CourseEnrollment.enroll(student[0], cea.course_id)
resp = render_to_response(
"registration/activation_complete.html",
{
'user_logged_in': user_logged_in,
'already_active': already_active
}
)
return resp
if len(r) == 0:
return render_to_response("registration/activation_invalid.html", {'csrf': csrf(request)['csrf_token']})
return render_to_response(
"registration/activation_invalid.html",
{'csrf': csrf(request)['csrf_token']}
)
return HttpResponse(_("Unknown error. Please e-mail us to let us know how it happened."))
......
......@@ -54,7 +54,7 @@ def register_by_course_id(course_id, is_staff=False):
if is_staff:
u.is_staff = True
u.save()
CourseEnrollment.objects.get_or_create(user=u, course_id=course_id)
CourseEnrollment.enroll(u, course_id)
@world.absorb
......
......@@ -347,7 +347,7 @@ There is an important split in demographic data gathered for the students who si
`student_courseenrollment`
==========================
A row in this table represents a student's enrollment for a particular course run. If they decide to unenroll in the course, we delete their entry in this table, but we still leave all their state in `courseware_studentmodule` untouched.
A row in this table represents a student's enrollment for a particular course run. If they decide to unenroll in the course, we set `is_active` to `False`. We still leave all their state in `courseware_studentmodule` untouched, so they will not lose courseware state if they unenroll and reenroll.
`id`
----
......@@ -365,6 +365,13 @@ A row in this table represents a student's enrollment for a particular course ru
---------
Datetime of enrollment, UTC.
`is_active`
-----------
Boolean indicating whether this enrollment is active. If an enrollment is not active, a student is not enrolled in that course. This lets us unenroll students without losing a record of what courses they were enrolled in previously. This was introduced in the 2013-08-20 release. Before this release, unenrolling a student simply deleted the row in `student_courseenrollment`.
`mode`
------
String indicating what kind of enrollment this was. The default is "honor" (honor certificate) and all enrollments prior to 2013-08-20 will be of that type. Other types being considered are "audit" and "verified_id".
*******************
......
......@@ -25,8 +25,10 @@ def enrolled_students_features(course_id, features):
{'username': 'username3', 'first_name': 'firstname3'}
]
"""
students = User.objects.filter(courseenrollment__course_id=course_id)\
.order_by('username').select_related('profile')
students = User.objects.filter(
courseenrollment__course_id=course_id,
courseenrollment__is_active=1,
).order_by('username').select_related('profile')
def extract_student(student, features):
""" convert student to dictionary """
......
......@@ -15,7 +15,8 @@ class TestAnalyticsBasic(TestCase):
def setUp(self):
self.course_id = 'some/robot/course/id'
self.users = tuple(UserFactory() for _ in xrange(30))
self.ces = tuple(CourseEnrollment.objects.create(course_id=self.course_id, user=user) for user in self.users)
self.ces = tuple(CourseEnrollment.enroll(user, self.course_id)
for user in self.users)
def test_enrolled_students_features_username(self):
self.assertIn('username', AVAILABLE_FEATURES)
......
......@@ -19,10 +19,8 @@ class TestAnalyticsDistributions(TestCase):
profile__year_of_birth=i + 1930
) for i in xrange(30)]
self.ces = [CourseEnrollment.objects.create(
course_id=self.course_id,
user=user
) for user in self.users]
self.ces = [CourseEnrollment.enroll(user, self.course_id)
for user in self.users]
@raises(ValueError)
def test_profile_distribution_bad_feature(self):
......@@ -68,7 +66,8 @@ class TestAnalyticsDistributionsNoData(TestCase):
self.users += self.nodata_users
self.ces = tuple(CourseEnrollment.objects.create(course_id=self.course_id, user=user) for user in self.users)
self.ces = tuple(CourseEnrollment.enroll(user, self.course_id)
for user in self.users)
def test_profile_distribution_easy_choice_nodata(self):
feature = 'gender'
......
......@@ -53,7 +53,7 @@ def i_am_registered_for_the_course(step, course):
# If the user is not already enrolled, enroll the user.
# TODO: change to factory
CourseEnrollment.objects.get_or_create(user=u, course_id=course_id(course))
CourseEnrollment.enroll(u, course_id(course))
world.log_in(username='robot', password='test')
......
......@@ -149,7 +149,7 @@ def create_user_and_visit_course():
world.create_user('robot', 'test')
u = User.objects.get(username='robot')
CourseEnrollment.objects.get_or_create(user=u, course_id=course_id(world.scenario_dict['COURSE'].number))
CourseEnrollment.enroll(u, course_id(world.scenario_dict['COURSE'].number))
world.log_in(username='robot', password='test')
chapter_name = (TEST_SECTION_NAME + "1").replace(" ", "_")
......
......@@ -67,9 +67,9 @@ class ViewsTestCase(TestCase):
email='test@mit.edu')
self.date = datetime.datetime(2013, 1, 22, tzinfo=UTC)
self.course_id = 'edX/toy/2012_Fall'
self.enrollment = CourseEnrollment.objects.get_or_create(user=self.user,
course_id=self.course_id,
created=self.date)[0]
self.enrollment = CourseEnrollment.enroll(self.user, self.course_id)
self.enrollment.created = self.date
self.enrollment.save()
self.location = ['tag', 'org', 'course', 'category', 'name']
self._MODULESTORES = {}
# This is a CourseDescriptor object
......
......@@ -568,12 +568,12 @@ def syllabus(request, course_id):
def registered_for_course(course, user):
"""
Return CourseEnrollment if user is registered for course, else False
Return True if user is registered for course, else False
"""
if user is None:
return False
if user.is_authenticated():
return CourseEnrollment.objects.filter(user=user, course_id=course.id).exists()
return CourseEnrollment.is_enrolled(user, course.id)
else:
return False
......
......@@ -47,7 +47,7 @@ def dashboard(request):
results["scalars"]["Activated Usernames"]=User.objects.filter(is_active=1).count()
# count how many enrollments we have
results["scalars"]["Total Enrollments Across All Courses"]=CourseEnrollment.objects.count()
results["scalars"]["Total Enrollments Across All Courses"] = CourseEnrollment.objects.filter(is_active=1).count()
# establish a direct connection to the database (for executing raw SQL)
cursor = connection.cursor()
......@@ -56,20 +56,22 @@ def dashboard(request):
# table queries need not take the form of raw SQL, but do in this case since
# the MySQL backend for django isn't very friendly with group by or distinct
table_queries = {}
table_queries["course enrollments"]= \
"select "+ \
"course_id as Course, "+ \
"count(user_id) as Students " + \
"from student_courseenrollment "+ \
"group by course_id "+ \
"order by students desc;"
table_queries["number of students in each number of classes"]= \
"select registrations as 'Registered for __ Classes' , "+ \
"count(registrations) as Users "+ \
"from (select count(user_id) as registrations "+ \
"from student_courseenrollment "+ \
"group by user_id) as registrations_per_user "+ \
"group by registrations;"
table_queries["course enrollments"] = """
select
course_id as Course,
count(user_id) as Students
from student_courseenrollment
where is_active=1
group by course_id
order by students desc;"""
table_queries["number of students in each number of classes"] = """
select registrations as 'Registered for __ Classes' ,
count(registrations) as Users
from (select count(user_id) as registrations
from student_courseenrollment
where is_active=1
group by user_id) as registrations_per_user
group by registrations;"""
# add the result for each of the table_queries to the results object
for query in table_queries.keys():
......
......@@ -22,7 +22,7 @@ class Command(BaseCommand):
course_id = args[0]
print "Updated roles for ",
for i, enrollment in enumerate(CourseEnrollment.objects.filter(course_id=course_id), start=1):
for i, enrollment in enumerate(CourseEnrollment.objects.filter(course_id=course_id, is_active=1), start=1):
assign_default_role(None, enrollment)
if i % 1000 == 0:
print "{0}...".format(i),
......
......@@ -19,7 +19,7 @@ class Command(BaseCommand):
raise CommandError("This Command takes no arguments")
print "Updated roles for ",
for i, enrollment in enumerate(CourseEnrollment.objects.all(), start=1):
for i, enrollment in enumerate(CourseEnrollment.objects.filter(is_active=1), start=1):
assign_default_role(None, enrollment)
if i % 1000 == 0:
print "{0}...".format(i),
......
......@@ -26,8 +26,8 @@ class PermissionsTestCase(TestCase):
password="123456", email="staff@edx.org")
self.moderator.is_staff = True
self.moderator.save()
self.student_enrollment = CourseEnrollment.objects.create(user=self.student, course_id=self.course_id)
self.moderator_enrollment = CourseEnrollment.objects.create(user=self.moderator, course_id=self.course_id)
self.student_enrollment = CourseEnrollment.enroll(self.student, self.course_id)
self.moderator_enrollment = CourseEnrollment.enroll(self.moderator, self.course_id)
def tearDown(self):
self.student_enrollment.delete()
......
......@@ -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()
......
......@@ -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()
......
......@@ -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(
......
......@@ -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)
......
......@@ -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):
"""
......
......@@ -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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment