From 458ed0a64ee357ec791b6c7307e7b3dc07061148 Mon Sep 17 00:00:00 2001 From: Carlos de la Guardia Date: Mon, 13 Jul 2015 06:16:15 -0500 Subject: [PATCH] Use the standard enrollment table instead of a custom CCX table for CCX enrollments. The goal for this PR is to have a single mechanism for registering users and reducing the number of places where special-casing for ccx courses is needed. The migration at this point is purposefully limited to convert ccx memberships into student enrollments when moving forward. No backward migration is in place at the moment. The ccx membership tables are not removed at this time. It is possible to go backwards and forwards multiple times with no errors or data loss. --- common/djangoapps/student/models.py | 6 - common/djangoapps/student/views.py | 18 - .../migrations/0002_convert_memberships.py | 98 +++ lms/djangoapps/ccx/models.py | 47 -- lms/djangoapps/ccx/tests/factories.py | 11 - .../tests/test_field_override_performance.py | 21 +- lms/djangoapps/ccx/tests/test_models.py | 115 ---- lms/djangoapps/ccx/tests/test_utils.py | 562 +----------------- lms/djangoapps/ccx/tests/test_views.py | 61 +- lms/djangoapps/ccx/utils.py | 286 +-------- lms/djangoapps/ccx/views.py | 48 +- lms/templates/ccx/_dashboard_ccx_listing.html | 80 --- lms/templates/ccx/enrollment.html | 4 +- lms/templates/dashboard.html | 8 - .../content/course_overviews/models.py | 19 +- 15 files changed, 210 insertions(+), 1174 deletions(-) create mode 100644 lms/djangoapps/ccx/migrations/0002_convert_memberships.py delete mode 100644 lms/templates/ccx/_dashboard_ccx_listing.html diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index ff9a863c33..0528eb6865 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1196,12 +1196,6 @@ class CourseEnrollment(models.Model): if not user.is_authenticated(): return False - # unwrap CCXLocators so that we use the course as the access control - # source - from ccx_keys.locator import CCXLocator - if isinstance(course_key, CCXLocator): - course_key = course_key.to_course_locator() - try: record = CourseEnrollment.objects.get(user=user, course_id=course_key) return record.is_active diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index eb8d00526c..4a02405154 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -655,13 +655,6 @@ def dashboard(request): ) courses_requirements_not_met = get_pre_requisite_courses_not_completed(user, courses_having_prerequisites) - ccx_membership_triplets = [] - if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): - from ccx.utils import get_ccx_membership_triplets - ccx_membership_triplets = get_ccx_membership_triplets( - user, course_org_filter, org_filter_out_set - ) - if 'notlive' in request.GET: redirect_message = _("The course you are looking for does not start until {date}.").format( date=request.GET['notlive'] @@ -697,7 +690,6 @@ def dashboard(request): 'provider_states': [], 'order_history_list': order_history_list, 'courses_requirements_not_met': courses_requirements_not_met, - 'ccx_membership_triplets': ccx_membership_triplets, 'nav_hidden': True, } @@ -1904,16 +1896,6 @@ def activate_account(request, key): manual_enrollment_audit.reason, enrollment ) - # enroll student in any pending CCXs he/she may have if auto_enroll flag is set - if settings.FEATURES.get('CUSTOM_COURSES_EDX'): - from ccx.models import CcxMembership, CcxFutureMembership - ccxfms = CcxFutureMembership.objects.filter( - email=student[0].email - ) - for ccxfm in ccxfms: - if ccxfm.auto_enroll: - CcxMembership.auto_enroll(student[0], ccxfm) - resp = render_to_response( "registration/activation_complete.html", { diff --git a/lms/djangoapps/ccx/migrations/0002_convert_memberships.py b/lms/djangoapps/ccx/migrations/0002_convert_memberships.py new file mode 100644 index 0000000000..3030c463b0 --- /dev/null +++ b/lms/djangoapps/ccx/migrations/0002_convert_memberships.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import DataMigration +from django.db import models + +class Migration(DataMigration): + + def forwards(self, orm): + "Convert CCX Memberships to Course Enrollments." + from ccx_keys.locator import CCXLocator + memberships = orm['ccx.CcxMembership'].objects.select_related('ccx', 'student').all() + for membership in memberships: + ccx = membership.ccx + course_key = CCXLocator.from_course_locator(ccx.course_id, ccx.id) + enrollment, created = orm['student.CourseEnrollment'].objects.get_or_create( + user=membership.student, + course_id=course_key, + ) + + + def backwards(self, orm): + """In the future, here we will convert back CCX Course Enrollments to CCX + Memberships. + """ + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'ccx.ccxfieldoverride': { + 'Meta': {'unique_together': "(('ccx', 'location', 'field'),)", 'object_name': 'CcxFieldOverride'}, + 'ccx': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['ccx.CustomCourseForEdX']"}), + 'field': ('django.db.models.fields.CharField', [], {'max_length': '255'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'location': ('xmodule_django.models.LocationKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'value': ('django.db.models.fields.TextField', [], {'default': "'null'"}) + }, + 'ccx.customcourseforedx': { + 'Meta': {'object_name': 'CustomCourseForEdX'}, + 'coach': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'display_name': ('django.db.models.fields.CharField', [], {'max_length': '255'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'ccx.ccxmembership': { + 'Meta': {'object_name': 'CcxMembership'}, + 'active': ('django.db.models.fields.BooleanField', [], {'default': False}), + 'ccx': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['ccx.CustomCourseForEdX']"}), + 'student': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'student.courseenrollment': { + 'Meta': {'object_name': 'CourseEnrollment'}, + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}), + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': True}), + 'mode': ('django.db.models.fields.CharField', [], {'max_length': '100', 'default': '"honor"'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + } + } + + complete_apps = ['ccx', 'ccx'] + symmetrical = True diff --git a/lms/djangoapps/ccx/models.py b/lms/djangoapps/ccx/models.py index 9639923946..439962b0e6 100644 --- a/lms/djangoapps/ccx/models.py +++ b/lms/djangoapps/ccx/models.py @@ -9,7 +9,6 @@ from django.db import models from django.utils.timezone import UTC from lazy import lazy -from student.models import CourseEnrollment, AlreadyEnrolledError # pylint: disable=import-error from xmodule_django.models import CourseKeyField, LocationKeyField # pylint: disable=import-error from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore @@ -96,52 +95,6 @@ class CustomCourseForEdX(models.Model): return value -class CcxMembership(models.Model): - """ - Which students are in a CCX? - """ - ccx = models.ForeignKey(CustomCourseForEdX, db_index=True) - student = models.ForeignKey(User, db_index=True) - active = models.BooleanField(default=False) - - @classmethod - def auto_enroll(cls, student, future_membership): - """convert future_membership to an active membership - """ - if not future_membership.auto_enroll: - msg = "auto enrollment not allowed for {}" - raise ValueError(msg.format(future_membership)) - membership = cls( - ccx=future_membership.ccx, student=student, active=True - ) - try: - CourseEnrollment.enroll( - student, future_membership.ccx.course_id, check_access=True - ) - except AlreadyEnrolledError: - # if the user is already enrolled in the course, great! - pass - - membership.save() - future_membership.delete() - - @classmethod - def memberships_for_user(cls, user, active=True): - """ - active memberships for a user - """ - return cls.objects.filter(student=user, active__exact=active) - - -class CcxFutureMembership(models.Model): - """ - Which emails for non-users are waiting to be added to CCX on registration - """ - ccx = models.ForeignKey(CustomCourseForEdX, db_index=True) - email = models.CharField(max_length=255) - auto_enroll = models.BooleanField(default=0) - - class CcxFieldOverride(models.Model): """ Field overrides for custom courses. diff --git a/lms/djangoapps/ccx/tests/factories.py b/lms/djangoapps/ccx/tests/factories.py index 507df6b44a..b2a99215c1 100644 --- a/lms/djangoapps/ccx/tests/factories.py +++ b/lms/djangoapps/ccx/tests/factories.py @@ -5,8 +5,6 @@ from factory import SubFactory from factory.django import DjangoModelFactory from student.tests.factories import UserFactory from ccx.models import CustomCourseForEdX # pylint: disable=import-error -from ccx.models import CcxMembership # pylint: disable=import-error -from ccx.models import CcxFutureMembership # pylint: disable=import-error class CcxFactory(DjangoModelFactory): # pylint: disable=missing-docstring @@ -14,12 +12,3 @@ class CcxFactory(DjangoModelFactory): # pylint: disable=missing-docstring display_name = "Test CCX" id = None # pylint: disable=redefined-builtin, invalid-name coach = SubFactory(UserFactory) - - -class CcxMembershipFactory(DjangoModelFactory): # pylint: disable=missing-docstring - FACTORY_FOR = CcxMembership - active = False - - -class CcxFutureMembershipFactory(DjangoModelFactory): # pylint: disable=missing-docstring - FACTORY_FOR = CcxFutureMembership diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 77247650fb..77f9a061ef 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -27,7 +27,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \ from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, check_sum_of_calls from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin from ccx_keys.locator import CCXLocator -from ccx.tests.factories import CcxFactory, CcxMembershipFactory +from ccx.tests.factories import CcxFactory @attr('shard_1') @@ -65,7 +65,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, MakoMiddleware().process_request(self.request) - def setup_course(self, size, enable_ccx): + def setup_course(self, size, enable_ccx, view_as_ccx): """ Build a gradable course where each node has `size` children. """ @@ -112,18 +112,17 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, ) self.populate_course(size) + course_key = self.course.id + if enable_ccx: + self.ccx = CcxFactory.create(course_id=self.course.id) + if view_as_ccx: + course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id) + CourseEnrollment.enroll( self.student, - self.course.id + course_key ) - if enable_ccx: - self.ccx = CcxFactory.create() - CcxMembershipFactory.create( - student=self.student, - ccx=self.ccx - ) - def grade_course(self, course, view_as_ccx): """ Renders the progress page for the given course. @@ -156,7 +155,7 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin, """ Renders the progress page, instrumenting Mongo reads and SQL queries. """ - self.setup_course(course_width, enable_ccx) + self.setup_course(course_width, enable_ccx, view_as_ccx) # Switch to published-only mode to simulate the LMS with self.settings(MODULESTORE_BRANCH='published-only'): diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py index 4162a8811e..fd60a53725 100644 --- a/lms/djangoapps/ccx/tests/test_models.py +++ b/lms/djangoapps/ccx/tests/test_models.py @@ -5,12 +5,9 @@ from datetime import datetime, timedelta from django.utils.timezone import UTC from mock import patch from nose.plugins.attrib import attr -from student.models import CourseEnrollment # pylint: disable=import-error from student.roles import CourseCcxCoachRole # pylint: disable=import-error from student.tests.factories import ( # pylint: disable=import-error AdminFactory, - CourseEnrollmentFactory, - UserFactory, ) from util.tests.test_date_utils import fake_ugettext from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -21,122 +18,10 @@ from xmodule.modulestore.tests.factories import ( from .factories import ( CcxFactory, - CcxFutureMembershipFactory, -) -from ..models import ( - CcxMembership, - CcxFutureMembership, ) from ..overrides import override_field_for_ccx -@attr('shard_1') -class TestCcxMembership(ModuleStoreTestCase): - """Unit tests for the CcxMembership model - """ - - def setUp(self): - """common setup for all tests""" - super(TestCcxMembership, self).setUp() - self.course = course = CourseFactory.create() - coach = AdminFactory.create() - role = CourseCcxCoachRole(course.id) - role.add_users(coach) - self.ccx = CcxFactory(course_id=course.id, coach=coach) - enrollment = CourseEnrollmentFactory.create(course_id=course.id) - self.enrolled_user = enrollment.user - self.unenrolled_user = UserFactory.create() - - def create_future_enrollment(self, user, auto_enroll=True): - """ - utility method to create future enrollment - """ - pfm = CcxFutureMembershipFactory.create( - ccx=self.ccx, - email=user.email, - auto_enroll=auto_enroll - ) - return pfm - - def has_course_enrollment(self, user): - """ - utility method to create future enrollment - """ - enrollment = CourseEnrollment.objects.filter( - user=user, course_id=self.course.id - ) - return enrollment.exists() - - def has_ccx_membership(self, user): - """ - verify ccx membership - """ - membership = CcxMembership.objects.filter( - student=user, ccx=self.ccx, active=True - ) - return membership.exists() - - def has_ccx_future_membership(self, user): - """ - verify future ccx membership - """ - future_membership = CcxFutureMembership.objects.filter( - email=user.email, ccx=self.ccx - ) - return future_membership.exists() - - def call_mut(self, student, future_membership): - """ - Call the method undser test - """ - CcxMembership.auto_enroll(student, future_membership) - - def test_ccx_auto_enroll_unregistered_user(self): - """verify auto_enroll works when user is not enrolled in the MOOC - - n.b. After auto_enroll, user will have both a MOOC enrollment and a - CCX membership - """ - user = self.unenrolled_user - pfm = self.create_future_enrollment(user) - self.assertTrue(self.has_ccx_future_membership(user)) - self.assertFalse(self.has_course_enrollment(user)) - # auto_enroll user - self.call_mut(user, pfm) - - self.assertTrue(self.has_course_enrollment(user)) - self.assertTrue(self.has_ccx_membership(user)) - self.assertFalse(self.has_ccx_future_membership(user)) - - def test_ccx_auto_enroll_registered_user(self): - """verify auto_enroll works when user is enrolled in the MOOC - """ - user = self.enrolled_user - pfm = self.create_future_enrollment(user) - self.assertTrue(self.has_ccx_future_membership(user)) - self.assertTrue(self.has_course_enrollment(user)) - - self.call_mut(user, pfm) - - self.assertTrue(self.has_course_enrollment(user)) - self.assertTrue(self.has_ccx_membership(user)) - self.assertFalse(self.has_ccx_future_membership(user)) - - def test_future_membership_disallows_auto_enroll(self): - """verify that the CcxFutureMembership can veto auto_enroll - """ - user = self.unenrolled_user - pfm = self.create_future_enrollment(user, auto_enroll=False) - self.assertTrue(self.has_ccx_future_membership(user)) - self.assertFalse(self.has_course_enrollment(user)) - - self.assertRaises(ValueError, self.call_mut, user, pfm) - - self.assertFalse(self.has_course_enrollment(user)) - self.assertFalse(self.has_ccx_membership(user)) - self.assertTrue(self.has_ccx_future_membership(user)) - - @attr('shard_1') class TestCCX(ModuleStoreTestCase): """Unit tests for the CustomCourseForEdX model diff --git a/lms/djangoapps/ccx/tests/test_utils.py b/lms/djangoapps/ccx/tests/test_utils.py index 7fa024c924..01c0bc7cc0 100644 --- a/lms/djangoapps/ccx/tests/test_utils.py +++ b/lms/djangoapps/ccx/tests/test_utils.py @@ -3,575 +3,49 @@ test utils """ from nose.plugins.attrib import attr -from ccx.models import ( # pylint: disable=import-error - CcxMembership, - CcxFutureMembership, -) from ccx.tests.factories import ( # pylint: disable=import-error CcxFactory, - CcxMembershipFactory, - CcxFutureMembershipFactory, ) from student.roles import CourseCcxCoachRole # pylint: disable=import-error from student.tests.factories import ( # pylint: disable=import-error AdminFactory, - UserFactory, ) from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE) from xmodule.modulestore.tests.factories import CourseFactory + from ccx_keys.locator import CCXLocator @attr('shard_1') -class TestEmailEnrollmentState(ModuleStoreTestCase): - """unit tests for the EmailEnrollmentState class - """ - - def setUp(self): - """ - Set up tests - """ - super(TestEmailEnrollmentState, self).setUp() - # remove user provided by the parent test case so we can make our own - # when needed. - self.user = None - course = CourseFactory.create() - coach = AdminFactory.create() - role = CourseCcxCoachRole(course.id) - role.add_users(coach) - self.ccx = CcxFactory(course_id=course.id, coach=coach) - - def create_user(self): - """provide a legitimate django user for testing - """ - if getattr(self, 'user', None) is None: - self.user = UserFactory() - - def register_user_in_ccx(self): - """create registration of self.user in self.ccx - - registration will be inactive - """ - self.create_user() - CcxMembershipFactory(ccx=self.ccx, student=self.user) - - def create_one(self, email=None): - """Create a single EmailEnrollmentState object and return it - """ - from ccx.utils import EmailEnrollmentState # pylint: disable=import-error - if email is None: - email = self.user.email - return EmailEnrollmentState(self.ccx, email) - - def test_enrollment_state_for_non_user(self): - """verify behavior for non-user email address - """ - ee_state = self.create_one(email='nobody@nowhere.com') - for attribute in ['user', 'member', 'full_name', 'in_ccx']: - value = getattr(ee_state, attribute, 'missing attribute') - self.assertFalse(value, "{}: {}".format(value, attribute)) - - def test_enrollment_state_for_non_member_user(self): - """verify behavior for email address of user who is not a ccx memeber - """ - self.create_user() - ee_state = self.create_one() - self.assertTrue(ee_state.user) - self.assertFalse(ee_state.in_ccx) - self.assertEqual(ee_state.member, self.user) - self.assertEqual(ee_state.full_name, self.user.profile.name) - - def test_enrollment_state_for_member_user(self): - """verify behavior for email address of user who is a ccx member - """ - self.create_user() - self.register_user_in_ccx() - ee_state = self.create_one() - for attribute in ['user', 'in_ccx']: - self.assertTrue( - getattr(ee_state, attribute, False), - "attribute {} is missing or False".format(attribute) - ) - self.assertEqual(ee_state.member, self.user) - self.assertEqual(ee_state.full_name, self.user.profile.name) - - def test_enrollment_state_to_dict(self): - """verify dict representation of EmailEnrollmentState - """ - self.create_user() - self.register_user_in_ccx() - ee_state = self.create_one() - ee_dict = ee_state.to_dict() - expected = { - 'user': True, - 'member': self.user, - 'in_ccx': True, - } - for expected_key, expected_value in expected.iteritems(): - self.assertTrue(expected_key in ee_dict) - self.assertEqual(expected_value, ee_dict[expected_key]) - - def test_enrollment_state_repr(self): - self.create_user() - self.register_user_in_ccx() - ee_state = self.create_one() - representation = repr(ee_state) - self.assertTrue('user=True' in representation) - self.assertTrue('in_ccx=True' in representation) - member = 'member={}'.format(self.user) - self.assertTrue(member in representation) - - -@attr('shard_1') -# TODO: deal with changes in behavior for auto_enroll -class TestGetEmailParams(ModuleStoreTestCase): - """tests for ccx.utils.get_email_params - """ - MODULESTORE = TEST_DATA_SPLIT_MODULESTORE - - def setUp(self): - """ - Set up tests - """ - super(TestGetEmailParams, self).setUp() - course = CourseFactory.create() - coach = AdminFactory.create() - role = CourseCcxCoachRole(course.id) - role.add_users(coach) - self.ccx = CcxFactory(course_id=course.id, coach=coach) - self.all_keys = [ - 'site_name', 'course', 'course_url', 'registration_url', - 'course_about_url', 'auto_enroll' - ] - self.url_keys = [k for k in self.all_keys if 'url' in k] - self.course_keys = [k for k in self.url_keys if 'course' in k] - - def call_fut(self, auto_enroll=False, secure=False): - """ - call function under test - """ - from ccx.utils import get_email_params # pylint: disable=import-error - return get_email_params(self.ccx, auto_enroll, secure) - - def test_params_have_expected_keys(self): - params = self.call_fut() - self.assertFalse(set(params.keys()) - set(self.all_keys)) - - def test_ccx_id_in_params(self): - expected_course_id = unicode(CCXLocator.from_course_locator(self.ccx.course_id, self.ccx.id)) - params = self.call_fut() - self.assertEqual(params['course'], self.ccx) - for url_key in self.url_keys: - self.assertTrue('http://' in params[url_key]) - for url_key in self.course_keys: - self.assertTrue(expected_course_id in params[url_key]) - - def test_security_respected(self): - secure = self.call_fut(secure=True) - for url_key in self.url_keys: - self.assertTrue('https://' in secure[url_key]) - insecure = self.call_fut(secure=False) - for url_key in self.url_keys: - self.assertTrue('http://' in insecure[url_key]) - - def test_auto_enroll_passed_correctly(self): - not_auto = self.call_fut(auto_enroll=False) - self.assertFalse(not_auto['auto_enroll']) - auto = self.call_fut(auto_enroll=True) - self.assertTrue(auto['auto_enroll']) - - -@attr('shard_1') -# TODO: deal with changes in behavior for auto_enroll -class TestEnrollEmail(ModuleStoreTestCase): - """tests for the enroll_email function from ccx.utils - """ - MODULESTORE = TEST_DATA_SPLIT_MODULESTORE - - def setUp(self): - super(TestEnrollEmail, self).setUp() - # unbind the user created by the parent, so we can create our own when - # needed. - self.user = None - course = CourseFactory.create() - coach = AdminFactory.create() - role = CourseCcxCoachRole(course.id) - role.add_users(coach) - self.ccx = CcxFactory(course_id=course.id, coach=coach) - self.outbox = self.get_outbox() - - def create_user(self): - """provide a legitimate django user for testing - """ - if getattr(self, 'user', None) is None: - self.user = UserFactory() - - def register_user_in_ccx(self): - """create registration of self.user in self.ccx - - registration will be inactive - """ - self.create_user() - CcxMembershipFactory(ccx=self.ccx, student=self.user) - - def get_outbox(self): - """Return the django mail outbox""" - from django.core import mail - return mail.outbox - - def check_membership(self, email=None, user=None, future=False): - """Verify tjat an appropriate CCX Membership exists""" - if not email and not user: - self.fail( - "must provide user or email address to check CCX Membership" - ) - if future and email: - membership = CcxFutureMembership.objects.filter( - ccx=self.ccx, email=email - ) - elif not future: - if not user: - user = self.user - membership = CcxMembership.objects.filter( - ccx=self.ccx, student=user - ) - self.assertTrue(membership.exists()) - - def check_enrollment_state(self, state, in_ccx, member, user): - """Verify an enrollment state object against provided arguments - - state.in_ccx will always be a boolean - state.user will always be a boolean - state.member will be a Django user object or None - """ - self.assertEqual(in_ccx, state.in_ccx) - self.assertEqual(member, state.member) - self.assertEqual(user, state.user) - - def call_fut( - self, - student_email=None, - auto_enroll=False, - email_students=False, - email_params=None - ): - """Call function under test""" - from ccx.utils import enroll_email # pylint: disable=import-error - if student_email is None: - student_email = self.user.email - before, after = enroll_email( - self.ccx, student_email, auto_enroll, email_students, email_params - ) - return before, after - - def test_enroll_non_user_sending_email(self): - """enroll a non-user email and send an enrollment email to them - """ - # ensure no emails are in the outbox now - self.assertEqual(self.outbox, []) - test_email = "nobody@nowhere.com" - before, after = self.call_fut( - student_email=test_email, email_students=True - ) - - # there should be a future membership set for this email address now - self.check_membership(email=test_email, future=True) - for state in [before, after]: - self.check_enrollment_state(state, False, None, False) - # mail was sent and to the right person - self.assertEqual(len(self.outbox), 1) - msg = self.outbox[0] - self.assertTrue(test_email in msg.recipients()) - - def test_enroll_non_member_sending_email(self): - """register a non-member and send an enrollment email to them - """ - self.create_user() - # ensure no emails are in the outbox now - self.assertEqual(self.outbox, []) - before, after = self.call_fut(email_students=True) - - # there should be a membership set for this email address now - self.check_membership(email=self.user.email) - self.check_enrollment_state(before, False, self.user, True) - self.check_enrollment_state(after, True, self.user, True) - # mail was sent and to the right person - self.assertEqual(len(self.outbox), 1) - msg = self.outbox[0] - self.assertTrue(self.user.email in msg.recipients()) - - def test_enroll_member_sending_email(self): - """register a member and send an enrollment email to them - """ - self.register_user_in_ccx() - # ensure no emails are in the outbox now - self.assertEqual(self.outbox, []) - before, after = self.call_fut(email_students=True) - - # there should be a membership set for this email address now - self.check_membership(email=self.user.email) - for state in [before, after]: - self.check_enrollment_state(state, True, self.user, True) - # mail was sent and to the right person - self.assertEqual(len(self.outbox), 1) - msg = self.outbox[0] - self.assertTrue(self.user.email in msg.recipients()) - - def test_enroll_non_user_no_email(self): - """register a non-user via email address but send no email - """ - # ensure no emails are in the outbox now - self.assertEqual(self.outbox, []) - test_email = "nobody@nowhere.com" - before, after = self.call_fut( - student_email=test_email, email_students=False - ) - - # there should be a future membership set for this email address now - self.check_membership(email=test_email, future=True) - for state in [before, after]: - self.check_enrollment_state(state, False, None, False) - # ensure there are still no emails in the outbox now - self.assertEqual(self.outbox, []) - - def test_enroll_non_member_no_email(self): - """register a non-member but send no email""" - self.create_user() - # ensure no emails are in the outbox now - self.assertEqual(self.outbox, []) - before, after = self.call_fut(email_students=False) - - # there should be a membership set for this email address now - self.check_membership(email=self.user.email) - self.check_enrollment_state(before, False, self.user, True) - self.check_enrollment_state(after, True, self.user, True) - # ensure there are still no emails in the outbox now - self.assertEqual(self.outbox, []) - - def test_enroll_member_no_email(self): - """enroll a member but send no email - """ - self.register_user_in_ccx() - # ensure no emails are in the outbox now - self.assertEqual(self.outbox, []) - before, after = self.call_fut(email_students=False) - - # there should be a membership set for this email address now - self.check_membership(email=self.user.email) - for state in [before, after]: - self.check_enrollment_state(state, True, self.user, True) - # ensure there are still no emails in the outbox now - self.assertEqual(self.outbox, []) - - -@attr('shard_1') -# TODO: deal with changes in behavior for auto_enroll -class TestUnenrollEmail(ModuleStoreTestCase): - """Tests for the unenroll_email function from ccx.utils""" - MODULESTORE = TEST_DATA_SPLIT_MODULESTORE - - def setUp(self): - super(TestUnenrollEmail, self).setUp() - # unbind the user created by the parent, so we can create our own when - # needed. - self.user = None - course = CourseFactory.create() - coach = AdminFactory.create() - role = CourseCcxCoachRole(course.id) - role.add_users(coach) - self.ccx = CcxFactory(course_id=course.id, coach=coach) - self.outbox = self.get_outbox() - self.email = "nobody@nowhere.com" - - def get_outbox(self): - """Return the django mail outbox""" - from django.core import mail - return mail.outbox - - def create_user(self): - """provide a legitimate django user for testing - """ - if getattr(self, 'user', None) is None: - self.user = UserFactory() - - def make_ccx_membership(self): - """create registration of self.user in self.ccx - - registration will be inactive - """ - self.create_user() - CcxMembershipFactory.create(ccx=self.ccx, student=self.user) - - def make_ccx_future_membership(self): - """create future registration for email in self.ccx""" - CcxFutureMembershipFactory.create( - ccx=self.ccx, email=self.email - ) - - def check_enrollment_state(self, state, in_ccx, member, user): - """Verify an enrollment state object against provided arguments - - state.in_ccx will always be a boolean - state.user will always be a boolean - state.member will be a Django user object or None - """ - self.assertEqual(in_ccx, state.in_ccx) - self.assertEqual(member, state.member) - self.assertEqual(user, state.user) - - def check_membership(self, future=False): - """ - check membership - """ - if future: - membership = CcxFutureMembership.objects.filter( - ccx=self.ccx, email=self.email - ) - else: - membership = CcxMembership.objects.filter( - ccx=self.ccx, student=self.user - ) - return membership.exists() - - def call_fut(self, email_students=False): - """call function under test""" - from ccx.utils import unenroll_email # pylint: disable=import-error - email = getattr(self, 'user', None) and self.user.email or self.email - return unenroll_email(self.ccx, email, email_students=email_students) - - def test_unenroll_future_member_with_email(self): - """unenroll a future member and send an email - """ - self.make_ccx_future_membership() - # assert that a membership exists and that no emails have been sent - self.assertTrue(self.check_membership(future=True)) - self.assertEqual(self.outbox, []) - # unenroll the student - before, after = self.call_fut(email_students=True) - - # assert that membership is now gone - self.assertFalse(self.check_membership(future=True)) - # validate the before and after enrollment states - for state in [before, after]: - self.check_enrollment_state(state, False, None, False) - # check that mail was sent and to the right person - self.assertEqual(len(self.outbox), 1) - msg = self.outbox[0] - self.assertTrue(self.email in msg.recipients()) - - def test_unenroll_member_with_email(self): - """unenroll a current member and send an email""" - self.make_ccx_membership() - # assert that a membership exists and that no emails have been sent - self.assertTrue(self.check_membership()) - self.assertEqual(self.outbox, []) - # unenroll the student - before, after = self.call_fut(email_students=True) - - # assert that membership is now gone - self.assertFalse(self.check_membership()) - # validate the before and after enrollment state - self.check_enrollment_state(after, False, self.user, True) - self.check_enrollment_state(before, True, self.user, True) - # check that mail was sent and to the right person - self.assertEqual(len(self.outbox), 1) - msg = self.outbox[0] - self.assertTrue(self.user.email in msg.recipients()) - - def test_unenroll_future_member_no_email(self): - """unenroll a future member but send no email - """ - self.make_ccx_future_membership() - # assert that a membership exists and that no emails have been sent - self.assertTrue(self.check_membership(future=True)) - self.assertEqual(self.outbox, []) - # unenroll the student - before, after = self.call_fut() - - # assert that membership is now gone - self.assertFalse(self.check_membership(future=True)) - # validate the before and after enrollment states - for state in [before, after]: - self.check_enrollment_state(state, False, None, False) - # no email was sent to the student - self.assertEqual(self.outbox, []) - - def test_unenroll_member_no_email(self): - """unenroll a current member but send no email - """ - self.make_ccx_membership() - # assert that a membership exists and that no emails have been sent - self.assertTrue(self.check_membership()) - self.assertEqual(self.outbox, []) - # unenroll the student - before, after = self.call_fut() - - # assert that membership is now gone - self.assertFalse(self.check_membership()) - # validate the before and after enrollment state - self.check_enrollment_state(after, False, self.user, True) - self.check_enrollment_state(before, True, self.user, True) - # no email was sent to the student - self.assertEqual(self.outbox, []) - - -@attr('shard_1') -class TestGetMembershipTriplets(ModuleStoreTestCase): - """Verify that get_ccx_membership_triplets functions properly""" +class TestGetCCXFromCCXLocator(ModuleStoreTestCase): + """Verify that get_ccx_from_ccx_locator functions properly""" MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): """Set up a course, coach, ccx and user""" - super(TestGetMembershipTriplets, self).setUp() + super(TestGetCCXFromCCXLocator, self).setUp() self.course = CourseFactory.create() - coach = AdminFactory.create() + coach = self.coach = AdminFactory.create() role = CourseCcxCoachRole(self.course.id) role.add_users(coach) - self.ccx = CcxFactory(course_id=self.course.id, coach=coach) - def make_ccx_membership(self, active=True): - """create registration of self.user in self.ccx - - registration will be inactive - """ - CcxMembershipFactory.create(ccx=self.ccx, student=self.user, active=active) - - def call_fut(self, org_filter=None, org_filter_out=()): + def call_fut(self, course_id): """call the function under test in this test case""" - from ccx.utils import get_ccx_membership_triplets - return list( - get_ccx_membership_triplets(self.user, org_filter, org_filter_out) - ) + from ccx.utils import get_ccx_from_ccx_locator + return get_ccx_from_ccx_locator(course_id) - def test_no_membership(self): - """verify that no triplets are returned if there are no memberships + def test_non_ccx_locator(self): + """verify that nothing is returned if locator is not a ccx locator """ - triplets = self.call_fut() - self.assertEqual(len(triplets), 0) + result = self.call_fut(self.course.id) + self.assertEqual(result, None) - def test_has_membership(self): - """verify that a triplet is returned when a membership exists + def test_ccx_locator(self): + """verify that the ccx is retuned if using a ccx locator """ - self.make_ccx_membership() - triplets = self.call_fut() - self.assertEqual(len(triplets), 1) - ccx, membership, course_overview = triplets[0] - self.assertEqual(ccx.id, self.ccx.id) - self.assertEqual(unicode(course_overview.id), unicode(self.course.id)) - self.assertEqual(membership.student, self.user) - - def test_has_membership_org_filtered(self): - """verify that microsite org filter prevents seeing microsite ccx""" - self.make_ccx_membership() - bad_org = self.course.location.org + 'foo' - triplets = self.call_fut(org_filter=bad_org) - self.assertEqual(len(triplets), 0) - - def test_has_membership_org_filtered_out(self): - """verify that microsite ccxs not seen in non-microsite view""" - self.make_ccx_membership() - filter_list = [self.course.location.org] - triplets = self.call_fut(org_filter_out=filter_list) - self.assertEqual(len(triplets), 0) + ccx = CcxFactory(course_id=self.course.id, coach=self.coach) + course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) + result = self.call_fut(course_key) + self.assertEqual(result, ccx) diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index d9a8182017..919cd1f52a 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -20,9 +20,12 @@ from django.utils.timezone import UTC from django.test.utils import override_settings from django.test import RequestFactory from edxmako.shortcuts import render_to_response # pylint: disable=import-error -from student.models import CourseEnrollment from request_cache.middleware import RequestCache from student.roles import CourseCcxCoachRole # pylint: disable=import-error +from student.models import ( + CourseEnrollment, + CourseEnrollmentAllowed, +) from student.tests.factories import ( # pylint: disable=import-error AdminFactory, CourseEnrollmentFactory, @@ -42,14 +45,10 @@ from ccx_keys.locator import CCXLocator from ..models import ( CustomCourseForEdX, - CcxMembership, - CcxFutureMembership, ) from ..overrides import get_override_for_ccx, override_field_for_ccx from .factories import ( CcxFactory, - CcxMembershipFactory, - CcxFutureMembershipFactory, ) @@ -280,7 +279,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertTrue(student.email in outbox[0].recipients()) # pylint: disable=no-member # a CcxMembership exists for this student self.assertTrue( - CcxMembership.objects.filter(ccx=ccx, student=student).exists() + CourseEnrollment.objects.filter(course_id=self.course.id, user=student).exists() ) def test_unenroll_member_student(self): @@ -288,16 +287,15 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): """ self.make_coach() ccx = self.make_ccx() - enrollment = CourseEnrollmentFactory(course_id=self.course.id) + course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) + enrollment = CourseEnrollmentFactory(course_id=course_key) student = enrollment.user outbox = self.get_outbox() self.assertEqual(outbox, []) - # student is member of CCX: - CcxMembershipFactory(ccx=ccx, student=student) url = reverse( 'ccx_invite', - kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)} + kwargs={'course_id': course_key} ) data = { 'enrollment-button': 'Unenroll', @@ -311,10 +309,6 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertTrue(302 in response.redirect_chain[0]) self.assertEqual(len(outbox), 1) self.assertTrue(student.email in outbox[0].recipients()) # pylint: disable=no-member - # the membership for this student is gone - self.assertFalse( - CcxMembership.objects.filter(ccx=ccx, student=student).exists() - ) def test_enroll_non_user_student(self): """enroll a list of students who are not users yet @@ -322,12 +316,13 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): test_email = "nobody@nowhere.com" self.make_coach() ccx = self.make_ccx() + course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) outbox = self.get_outbox() self.assertEqual(outbox, []) url = reverse( 'ccx_invite', - kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)} + kwargs={'course_id': course_key} ) data = { 'enrollment-button': 'Enroll', @@ -342,8 +337,8 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(len(outbox), 1) self.assertTrue(test_email in outbox[0].recipients()) self.assertTrue( - CcxFutureMembership.objects.filter( - ccx=ccx, email=test_email + CourseEnrollmentAllowed.objects.filter( + course_id=course_key, email=test_email ).exists() ) @@ -352,14 +347,16 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): """ test_email = "nobody@nowhere.com" self.make_coach() + course = CourseFactory.create() ccx = self.make_ccx() + course_key = CCXLocator.from_course_locator(course.id, ccx.id) outbox = self.get_outbox() - CcxFutureMembershipFactory(ccx=ccx, email=test_email) + CourseEnrollmentAllowed(course_id=course_key, email=test_email) self.assertEqual(outbox, []) url = reverse( 'ccx_invite', - kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)} + kwargs={'course_id': course_key} ) data = { 'enrollment-button': 'Unenroll', @@ -371,11 +368,9 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): # we were redirected to our current location self.assertEqual(len(response.redirect_chain), 1) self.assertTrue(302 in response.redirect_chain[0]) - self.assertEqual(len(outbox), 1) - self.assertTrue(test_email in outbox[0].recipients()) self.assertFalse( - CcxFutureMembership.objects.filter( - ccx=ccx, email=test_email + CourseEnrollmentAllowed.objects.filter( + course_id=course_key, email=test_email ).exists() ) @@ -384,7 +379,8 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): """ self.make_coach() ccx = self.make_ccx() - enrollment = CourseEnrollmentFactory(course_id=self.course.id) + course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) + enrollment = CourseEnrollmentFactory(course_id=course_key) student = enrollment.user # no emails have been sent so far outbox = self.get_outbox() @@ -392,7 +388,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): url = reverse( 'ccx_manage_student', - kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)} + kwargs={'course_id': course_key} ) data = { 'student-action': 'add', @@ -406,7 +402,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(outbox, []) # a CcxMembership exists for this student self.assertTrue( - CcxMembership.objects.filter(ccx=ccx, student=student).exists() + CourseEnrollment.objects.filter(course_id=course_key, user=student).exists() ) def test_manage_remove_single_student(self): @@ -414,9 +410,9 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): """ self.make_coach() ccx = self.make_ccx() - enrollment = CourseEnrollmentFactory(course_id=self.course.id) + course_key = CCXLocator.from_course_locator(self.course.id, ccx.id) + enrollment = CourseEnrollmentFactory(course_id=course_key) student = enrollment.user - CcxMembershipFactory(ccx=ccx, student=student) # no emails have been sent so far outbox = self.get_outbox() self.assertEqual(outbox, []) @@ -435,10 +431,6 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertEqual(len(response.redirect_chain), 1) self.assertTrue(302 in response.redirect_chain[0]) self.assertEqual(outbox, []) - # a CcxMembership exists for this student - self.assertFalse( - CcxMembership.objects.filter(ccx=ccx, student=student).exists() - ) GET_CHILDREN = XModuleMixin.get_children @@ -525,7 +517,6 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): self.student = student = UserFactory.create() CourseEnrollmentFactory.create(user=student, course_id=self.course.id) - CcxMembershipFactory(ccx=ccx, student=student, active=True) # create grades for self.student as if they'd submitted the ccx for chapter in self.course.get_children(): @@ -674,12 +665,14 @@ class TestStudentDashboardWithCCX(ModuleStoreTestCase): self.ccx = CcxFactory(course_id=self.split_course.id, coach=self.coach) last_week = datetime.datetime.now(UTC()) - datetime.timedelta(days=7) override_field_for_ccx(self.ccx, self.split_course, 'start', last_week) # Required by self.ccx.has_started(). - CcxMembershipFactory(ccx=self.ccx, student=self.student, active=True) + course_key = CCXLocator.from_course_locator(self.split_course.id, self.ccx.id) + CourseEnrollment.enroll(self.student, course_key) def test_load_student_dashboard(self): self.client.login(username=self.student.username, password=self.student_password) response = self.client.get(reverse('dashboard')) self.assertEqual(response.status_code, 200) + self.assertTrue(re.search('Test CCX', response.content)) def flatten(seq): diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 5b69cd794b..8cbcb8ece1 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -4,283 +4,23 @@ CCX Enrollment operations for use by Coach APIs. Does not include any access control, be sure to check access before calling. """ import logging -from django.contrib.auth.models import User -from django.conf import settings -from django.core.urlresolvers import reverse -from django.core.mail import send_mail -from edxmako.shortcuts import render_to_string # pylint: disable=import-error -from microsite_configuration import microsite # pylint: disable=import-error -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from xmodule.modulestore.django import modulestore -from xmodule.error_module import ErrorDescriptor -from ccx_keys.locator import CCXLocator -from .models import ( - CcxMembership, - CcxFutureMembership, -) +from .models import CustomCourseForEdX log = logging.getLogger("edx.ccx") -class EmailEnrollmentState(object): - """ Store the complete enrollment state of an email in a class """ - def __init__(self, ccx, email): - exists_user = User.objects.filter(email=email).exists() - if exists_user: - user = User.objects.get(email=email) - ccx_member = CcxMembership.objects.filter(ccx=ccx, student=user) - in_ccx = ccx_member.exists() - full_name = user.profile.name - else: - user = None - in_ccx = False - full_name = None - self.user = exists_user - self.member = user - self.full_name = full_name - self.in_ccx = in_ccx - - def __repr__(self): - return "{}(user={}, member={}, in_ccx={})".format( - self.__class__.__name__, - self.user, - self.member, - self.in_ccx, +def get_ccx_from_ccx_locator(course_id): + """ helper function to allow querying ccx fields from templates """ + ccx_id = getattr(course_id, 'ccx', None) + ccx = None + if ccx_id: + ccx = CustomCourseForEdX.objects.filter(id=ccx_id) + if not ccx: + log.warning( + "CCX does not exist for course with id %s", + course_id ) - - def to_dict(self): - """ return dict with membership and ccx info """ - return { - 'user': self.user, - 'member': self.member, - 'in_ccx': self.in_ccx, - } - - -def enroll_email(ccx, student_email, auto_enroll=False, email_students=False, email_params=None): - """ - Send email to newly enrolled student - """ - if email_params is None: - email_params = get_email_params(ccx, True) - previous_state = EmailEnrollmentState(ccx, student_email) - - if previous_state.user: - user = User.objects.get(email=student_email) - if not previous_state.in_ccx: - membership = CcxMembership( - ccx=ccx, student=user, active=True - ) - membership.save() - elif auto_enroll: - # activate existing memberships - membership = CcxMembership.objects.get(student=user, ccx=ccx) - membership.active = True - membership.save() - if email_students: - email_params['message'] = 'enrolled_enroll' - email_params['email_address'] = student_email - email_params['full_name'] = previous_state.full_name - send_mail_to_student(student_email, email_params) - else: - membership = CcxFutureMembership( - ccx=ccx, auto_enroll=auto_enroll, email=student_email - ) - membership.save() - if email_students: - email_params['message'] = 'allowed_enroll' - email_params['email_address'] = student_email - send_mail_to_student(student_email, email_params) - - after_state = EmailEnrollmentState(ccx, student_email) - - return previous_state, after_state - - -def unenroll_email(ccx, student_email, email_students=False, email_params=None): - """ - send email to unenrolled students - """ - if email_params is None: - email_params = get_email_params(ccx, True) - previous_state = EmailEnrollmentState(ccx, student_email) - - if previous_state.in_ccx: - CcxMembership.objects.get( - ccx=ccx, student=previous_state.member - ).delete() - if email_students: - email_params['message'] = 'enrolled_unenroll' - email_params['email_address'] = student_email - email_params['full_name'] = previous_state.full_name - send_mail_to_student(student_email, email_params) - else: - if CcxFutureMembership.objects.filter( - ccx=ccx, email=student_email).exists(): - CcxFutureMembership.objects.get( - ccx=ccx, email=student_email - ).delete() - if email_students: - email_params['message'] = 'allowed_unenroll' - email_params['email_address'] = student_email - send_mail_to_student(student_email, email_params) - - after_state = EmailEnrollmentState(ccx, student_email) - - return previous_state, after_state - - -def get_email_params(ccx, auto_enroll, secure=True): - """ - get parameters for enrollment emails - """ - protocol = 'https' if secure else 'http' - - stripped_site_name = microsite.get_value( - 'SITE_NAME', - settings.SITE_NAME - ) - registration_url = u'{proto}://{site}{path}'.format( - proto=protocol, - site=stripped_site_name, - path=reverse('register_user') - ) - course_url = u'{proto}://{site}{path}'.format( - proto=protocol, - site=stripped_site_name, - path=reverse( - 'course_root', - kwargs={'course_id': CCXLocator.from_course_locator(ccx.course_id, ccx.id)} - ) - ) - - course_about_url = None - if not settings.FEATURES.get('ENABLE_MKTG_SITE', False): - course_about_url = u'{proto}://{site}{path}'.format( - proto=protocol, - site=stripped_site_name, - path=reverse( - 'about_course', - kwargs={'course_id': CCXLocator.from_course_locator(ccx.course_id, ccx.id)} - ) - ) - - email_params = { - 'site_name': stripped_site_name, - 'registration_url': registration_url, - 'course': ccx, - 'auto_enroll': auto_enroll, - 'course_url': course_url, - 'course_about_url': course_about_url, - } - return email_params - - -def send_mail_to_student(student, param_dict): - """ - Check parameters, set text template and send email to student - """ - if 'course' in param_dict: - param_dict['course_name'] = param_dict['course'].display_name - - param_dict['site_name'] = microsite.get_value( - 'SITE_NAME', - param_dict['site_name'] - ) - - subject = None - message = None - - message_type = param_dict['message'] - - email_template_dict = { - 'allowed_enroll': ( - 'ccx/enroll_email_allowedsubject.txt', - 'ccx/enroll_email_allowedmessage.txt' - ), - 'enrolled_enroll': ( - 'ccx/enroll_email_enrolledsubject.txt', - 'ccx/enroll_email_enrolledmessage.txt' - ), - 'allowed_unenroll': ( - 'ccx/unenroll_email_subject.txt', - 'ccx/unenroll_email_allowedmessage.txt' - ), - 'enrolled_unenroll': ( - 'ccx/unenroll_email_subject.txt', - 'ccx/unenroll_email_enrolledmessage.txt' - ), - } - - subject_template, message_template = email_template_dict.get( - message_type, (None, None) - ) - if subject_template is not None and message_template is not None: - subject = render_to_string(subject_template, param_dict) - message = render_to_string(message_template, param_dict) - - if subject and message: - message = message.strip() - - subject = ''.join(subject.splitlines()) - from_address = microsite.get_value( - 'email_from_address', - settings.DEFAULT_FROM_EMAIL - ) - - send_mail( - subject, - message, - from_address, - [student], - fail_silently=False - ) - - -def get_ccx_membership_triplets(user, course_org_filter, org_filter_out_set): - """ - Get the relevant set of (CustomCourseForEdX, CcxMembership, CourseOverview) - triplets to be displayed on a student's dashboard. - """ - error_message_format = "User {0} enrolled in {2} course {1}" - - # only active memberships for now - for membership in CcxMembership.memberships_for_user(user): - ccx = membership.ccx - - try: - course_overview = CourseOverview.get_from_id(ccx.course_id) - except CourseOverview.DoesNotExist: - log.error(error_message_format.format( # pylint: disable=logging-format-interpolation - user.username, ccx.course_id, "non-existent" - )) - continue - except IOError: - log.error(error_message_format.format( # pylint: disable=logging-format-interpolation - user.username, ccx.course_id, "broken" - )) - continue - - # if we are in a Microsite, then filter out anything that is not - # attributed (by ORG) to that Microsite - if course_org_filter and course_org_filter != course_overview.location.org: - continue - # Conversely, if we are not in a Microsite, then let's filter out any enrollments - # with courses attributed (by ORG) to Microsites - elif course_overview.location.org in org_filter_out_set: - continue - - # If, somehow, we've got a ccx that has been created for a - # course with a deprecated ID, we must filter it out. Emit a - # warning to the log so we can clean up. - if course_overview.location.deprecated: - log.warning( - "CCX %s exists for course %s with deprecated id", - ccx, - ccx.course_id - ) - continue - - yield (ccx, membership, course_overview) + return None + return ccx[0] diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 554f0be9b3..931ba33b08 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -36,22 +36,24 @@ from courseware.module_render import get_module_for_descriptor from edxmako.shortcuts import render_to_response from opaque_keys.edx.keys import CourseKey from ccx_keys.locator import CCXLocator -from student.roles import CourseCcxCoachRole +from student.roles import CourseCcxCoachRole # pylint: disable=import-error +from student.models import CourseEnrollment -from instructor.offline_gradecalc import student_grades -from instructor.views.api import _split_input_list -from instructor.views.tools import get_student_from_identifier +from instructor.offline_gradecalc import student_grades # pylint: disable=import-error +from instructor.views.api import _split_input_list # pylint: disable=import-error +from instructor.views.tools import get_student_from_identifier # pylint: disable=import-error +from instructor.enrollment import ( + enroll_email, + unenroll_email, + get_email_params, +) -from .models import CustomCourseForEdX, CcxMembership +from .models import CustomCourseForEdX from .overrides import ( clear_override_for_ccx, get_override_for_ccx, override_field_for_ccx, ) -from .utils import ( - enroll_email, - unenroll_email, -) log = logging.getLogger(__name__) @@ -127,7 +129,7 @@ def dashboard(request, course, ccx=None): context['schedule'] = json.dumps(schedule, indent=4) context['save_url'] = reverse( 'save_ccx', kwargs={'course_id': ccx_locator}) - context['ccx_members'] = CcxMembership.objects.filter(ccx=ccx) + context['ccx_members'] = CourseEnrollment.objects.filter(course_id=ccx_locator) context['gradebook_url'] = reverse( 'ccx_gradebook', kwargs={'course_id': ccx_locator}) context['grades_csv_url'] = reverse( @@ -156,7 +158,7 @@ def create_ccx(request, course, ccx=None): "You cannot create a CCX from a course using a deprecated id. " "Please create a rerun of this course in the studio to allow " "this action.")) - url = reverse('ccx_coach_dashboard', kwargs={'course_id', course.id}) + url = reverse('ccx_coach_dashboard', kwargs={'course_id': course.id}) return redirect(url) ccx = CustomCourseForEdX( @@ -407,15 +409,18 @@ def ccx_invite(request, course, ccx=None): email = user.email try: validate_email(email) + course_key = CCXLocator.from_course_locator(course.id, ccx.id) + email_params = get_email_params(course, auto_enroll) if action == 'Enroll': enroll_email( - ccx, + course_key, email, auto_enroll=auto_enroll, - email_students=email_students + email_students=email_students, + email_params=email_params ) if action == "Unenroll": - unenroll_email(ccx, email, email_students=email_students) + unenroll_email(course_key, email, email_students=email_students, email_params=email_params) except ValidationError: log.info('Invalid user name or email when trying to invite students: %s', email) url = reverse( @@ -444,20 +449,21 @@ def ccx_student_management(request, course, ccx=None): else: email = user.email + course_key = CCXLocator.from_course_locator(course.id, ccx.id) try: validate_email(email) if action == 'add': # by decree, no emails sent to students added this way # by decree, any students added this way are auto_enrolled - enroll_email(ccx, email, auto_enroll=True, email_students=False) + enroll_email(course_key, email, auto_enroll=True, email_students=False) elif action == 'revoke': - unenroll_email(ccx, email, email_students=False) + unenroll_email(course_key, email, email_students=False) except ValidationError: log.info('Invalid user name or email when trying to enroll student: %s', email) url = reverse( 'ccx_coach_dashboard', - kwargs={'course_id': CCXLocator.from_course_locator(course.id, ccx.id)} + kwargs={'course_id': course_key} ) return redirect(url) @@ -496,8 +502,8 @@ def ccx_gradebook(request, course, ccx=None): prep_course_for_grading(course, request) enrolled_students = User.objects.filter( - ccxmembership__ccx=ccx, - ccxmembership__active=1 + courseenrollment__course_id=ccx_key, + courseenrollment__is_active=1 ).order_by('username').select_related("profile") student_info = [ @@ -535,8 +541,8 @@ def ccx_grades_csv(request, course, ccx=None): prep_course_for_grading(course, request) enrolled_students = User.objects.filter( - ccxmembership__ccx=ccx, - ccxmembership__active=1 + courseenrollment__course_id=ccx_key, + courseenrollment__is_active=1 ).order_by('username').select_related("profile") grades = iterate_grades_for(course, enrolled_students) diff --git a/lms/templates/ccx/_dashboard_ccx_listing.html b/lms/templates/ccx/_dashboard_ccx_listing.html deleted file mode 100644 index 1eca7c6eb5..0000000000 --- a/lms/templates/ccx/_dashboard_ccx_listing.html +++ /dev/null @@ -1,80 +0,0 @@ -<%page args="ccx, membership, course_overview, show_courseware_link, is_course_blocked" /> - -<%! -from django.utils.translation import ugettext as _ -from django.core.urlresolvers import reverse -from courseware.courses import course_image_url, get_course_about_section -from ccx_keys.locator import CCXLocator -%> -<% - ccx_target = reverse('info', args=[CCXLocator.from_course_locator(course_overview.id, ccx.id)]) -%> -
  • -
    -
    - -
    -

    - % if show_courseware_link: - % if not is_course_blocked: - ${ccx.display_name} - % else: - ${ccx.display_name} - % endif - % else: - ${ccx.display_name} - % endif -

    -
    - ${get_course_about_section(course_overview, 'university')} - - ${course_overview.display_number_with_default | h} - - % if ccx.has_ended(): - ${_("Ended - {end_date}").format(end_date=ccx.end_datetime_text("SHORT_DATE"))} - % elif ccx.has_started(): - ${_("Started - {start_date}").format(start_date=ccx.start_datetime_text("SHORT_DATE"))} - % else: # hasn't started yet - ${_("Starts - {start_date}").format(start_date=ccx.start_datetime_text("SHORT_DATE"))} - % endif - -
    - % if show_courseware_link: -
    -
    - % if ccx.has_ended(): - % if not is_course_blocked: - ${_('View Archived Custom Course')} ${ccx.display_name} - % else: - ${_('View Archived Custom Course')} ${ccx.display_name} - % endif - % else: - % if not is_course_blocked: - ${_('View Custom Course')} ${ccx.display_name} - % else: - ${_('View Custom Course')} ${ccx.display_name} - % endif - % endif - -
    -
    - % endif -
    -
    -
    -
    • diff --git a/lms/templates/ccx/enrollment.html b/lms/templates/ccx/enrollment.html index f024ea3a68..15ea3d86a3 100644 --- a/lms/templates/ccx/enrollment.html +++ b/lms/templates/ccx/enrollment.html @@ -63,8 +63,8 @@ %for member in ccx_members: - ${member.student} - ${member.student.email} + ${member.user} + ${member.user.email}
    Revoke access
    %endfor diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index b3c53bb6a6..f03cb821d4 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -100,14 +100,6 @@ import json <%include file='dashboard/_dashboard_course_listing.html' args="course_overview=enrollment.course_overview, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option=show_refund_option, is_paid_course=is_paid_course, is_course_blocked=is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user" /> % endfor - % if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): - % for ccx, membership, course_overview in ccx_membership_triplets: - <% show_courseware_link = ccx.has_started() %> - <% is_course_blocked = False %> - <%include file='ccx/_dashboard_ccx_listing.html' args="ccx=ccx, membership=membership, course_overview=course_overview, show_courseware_link=show_courseware_link, is_course_blocked=is_course_blocked" /> - % endfor - % endif - % else:
    diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index a6db91fbcc..d98b77f1cb 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -15,6 +15,8 @@ from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule_django.models import CourseKeyField, UsageKeyField +from ccx_keys.locator import CCXLocator + class CourseOverview(TimeStampedModel): """ @@ -100,17 +102,26 @@ class CourseOverview(TimeStampedModel): except ValueError: lowest_passing_grade = None + display_name = course.display_name + start = course.start + end = course.end + if isinstance(course.id, CCXLocator): + from ccx.utils import get_ccx_from_ccx_locator # pylint: disable=import-error + ccx = get_ccx_from_ccx_locator(course.id) + display_name = ccx.display_name + start = ccx.start + end = ccx.due + return cls( version=cls.VERSION, - id=course.id, _location=course.location, - display_name=course.display_name, + display_name=display_name, display_number_with_default=course.display_number_with_default, display_org_with_default=course.display_org_with_default, - start=course.start, - end=course.end, + start=start, + end=end, advertised_start=course.advertised_start, course_image_url=course_image_url(course),