diff --git a/common/lib/xmodule/xmodule/videoannotation_module.py b/common/lib/xmodule/xmodule/videoannotation_module.py
index 03fc2f75e3..6a8584505b 100644
--- a/common/lib/xmodule/xmodule/videoannotation_module.py
+++ b/common/lib/xmodule/xmodule/videoannotation_module.py
@@ -19,7 +19,9 @@ _ = lambda text: text
class AnnotatableFields(object):
""" Fields for `VideoModule` and `VideoDescriptor`. """
- data = String(help=_("XML data for the annotation"), scope=Scope.content, default=textwrap.dedent("""\
+ data = String(help=_("XML data for the annotation"),
+ scope=Scope.content,
+ default=textwrap.dedent("""\
From f6ea16223b3db02eaaa414c750b02731a7380406 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Wed, 11 Jun 2014 12:06:37 -0400
Subject: [PATCH 6/6] Allow users that have unregistered from a verified course
to re-register.
Fixes LMS-2830
---
.../course_modes/tests/factories.py | 4 +-
.../course_modes/tests/test_views.py | 100 ++++++++++++++++++
common/djangoapps/course_modes/views.py | 14 +--
common/djangoapps/student/models.py | 13 +--
common/djangoapps/student/tests/factories.py | 5 +-
lms/djangoapps/certificates/queue.py | 2 +-
lms/djangoapps/verify_student/views.py | 6 +-
7 files changed, 123 insertions(+), 21 deletions(-)
create mode 100644 common/djangoapps/course_modes/tests/test_views.py
diff --git a/common/djangoapps/course_modes/tests/factories.py b/common/djangoapps/course_modes/tests/factories.py
index 4c32dea10f..0cfb439700 100644
--- a/common/djangoapps/course_modes/tests/factories.py
+++ b/common/djangoapps/course_modes/tests/factories.py
@@ -1,5 +1,6 @@
from course_modes.models import CourseMode
from factory.django import DjangoModelFactory
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
# Factories don't have __init__ methods, and are self documenting
@@ -7,9 +8,10 @@ from factory.django import DjangoModelFactory
class CourseModeFactory(DjangoModelFactory):
FACTORY_FOR = CourseMode
- course_id = u'MITx/999/Robot_Super_Course'
+ course_id = SlashSeparatedCourseKey('MITx', '999', 'Robot_Super_Course')
mode_slug = 'audit'
mode_display_name = 'audit course'
min_price = 0
currency = 'usd'
expiration_datetime = None
+ suggested_prices = ''
diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py
new file mode 100644
index 0000000000..9de6f3c09f
--- /dev/null
+++ b/common/djangoapps/course_modes/tests/test_views.py
@@ -0,0 +1,100 @@
+import ddt
+import unittest
+from django.test import TestCase
+from django.conf import settings
+from django.core.urlresolvers import reverse
+from mock import patch, Mock
+
+from course_modes.tests.factories import CourseModeFactory
+from student.tests.factories import CourseEnrollmentFactory, UserFactory
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
+
+
+@ddt.ddt
+class CourseModeViewTest(TestCase):
+
+ def setUp(self):
+ self.course_id = SlashSeparatedCourseKey('org', 'course', 'run')
+
+ for mode in ('audit', 'verified', 'honor'):
+ CourseModeFactory(mode_slug=mode, course_id=self.course_id)
+
+ @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
+ @ddt.data(
+ # is_active?, enrollment_mode, upgrade?, redirect?
+ (True, 'verified', True, True), # User is already verified
+ (True, 'verified', False, True), # User is already verified
+ (True, 'honor', True, False), # User isn't trying to upgrade
+ (True, 'honor', False, True), # User is trying to upgrade
+ (True, 'audit', True, False), # User isn't trying to upgrade
+ (True, 'audit', False, True), # User is trying to upgrade
+ (False, 'verified', True, False), # User isn't active
+ (False, 'verified', False, False), # User isn't active
+ (False, 'honor', True, False), # User isn't active
+ (False, 'honor', False, False), # User isn't active
+ (False, 'audit', True, False), # User isn't active
+ (False, 'audit', False, False), # User isn't active
+ )
+ @ddt.unpack
+ @patch('course_modes.views.modulestore', Mock())
+ def test_reregister_redirect(self, is_active, enrollment_mode, upgrade, redirect):
+ enrollment = CourseEnrollmentFactory(
+ is_active=is_active,
+ mode=enrollment_mode,
+ course_id=self.course_id
+ )
+
+ self.client.login(
+ username=enrollment.user.username,
+ password='test'
+ )
+
+ if upgrade:
+ get_params = {'upgrade': True}
+ else:
+ get_params = {}
+
+ response = self.client.get(
+ reverse('course_modes_choose', args=[self.course_id.to_deprecated_string()]),
+ get_params,
+ follow=False,
+ )
+
+ if redirect:
+ self.assertEquals(response.status_code, 302)
+ self.assertTrue(response['Location'].endswith(reverse('dashboard')))
+ else:
+ self.assertEquals(response.status_code, 200)
+ # TODO: Fix it so that response.templates works w/ mako templates, and then assert
+ # that the right template rendered
+
+ @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
+ @ddt.data(
+ '',
+ '1,,2',
+ '1, ,2',
+ '1, 2, 3'
+ )
+ @patch('course_modes.views.modulestore', Mock())
+ def test_suggested_prices(self, price_list):
+ course_id = SlashSeparatedCourseKey('org', 'course', 'price_course')
+ user = UserFactory()
+
+ for mode in ('audit', 'honor'):
+ CourseModeFactory(mode_slug=mode, course_id=course_id)
+
+ CourseModeFactory(mode_slug='verified', course_id=course_id, suggested_prices=price_list)
+
+ self.client.login(
+ username=user.username,
+ password='test'
+ )
+
+ response = self.client.get(
+ reverse('course_modes_choose', args=[self.course_id.to_deprecated_string()]),
+ follow=False,
+ )
+
+ self.assertEquals(response.status_code, 200)
+ # TODO: Fix it so that response.templates works w/ mako templates, and then assert
+ # that the right template rendered
diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py
index 68b44e61cb..62855f65f6 100644
--- a/common/djangoapps/course_modes/views.py
+++ b/common/djangoapps/course_modes/views.py
@@ -36,16 +36,14 @@ class ChooseModeView(View):
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
- enrollment_mode = CourseEnrollment.enrollment_mode_for_user(request.user, course_key)
+ enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(request.user, course_key)
upgrade = request.GET.get('upgrade', False)
request.session['attempting_upgrade'] = upgrade
+ # Inactive users always need to re-register
# verified users do not need to register or upgrade
- if enrollment_mode == 'verified':
- return redirect(reverse('dashboard'))
-
# registered users who are not trying to upgrade do not need to re-register
- if enrollment_mode is not None and upgrade is False:
+ if is_active and (upgrade is False or enrollment_mode == 'verified'):
return redirect(reverse('dashboard'))
modes = CourseMode.modes_for_course_dict(course_key)
@@ -64,7 +62,11 @@ class ChooseModeView(View):
"upgrade": upgrade,
}
if "verified" in modes:
- context["suggested_prices"] = [decimal.Decimal(x) for x in modes["verified"].suggested_prices.split(",")]
+ context["suggested_prices"] = [
+ decimal.Decimal(x.strip())
+ for x in modes["verified"].suggested_prices.split(",")
+ if x.strip()
+ ]
context["currency"] = modes["verified"].currency.upper()
context["min_price"] = modes["verified"].min_price
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index 6faec29c0e..8654a32110 100644
--- a/common/djangoapps/student/models.py
+++ b/common/djangoapps/student/models.py
@@ -880,18 +880,15 @@ class CourseEnrollment(models.Model):
`user` is a Django User object
`course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
- Returns the mode for both inactive and active users.
- Returns None if the courseenrollment record does not exist.
+ Returns (mode, is_active) where mode is the enrollment mode of the student
+ and is_active is whether the enrollment is active.
+ Returns (None, None) if the courseenrollment record does not exist.
"""
try:
record = CourseEnrollment.objects.get(user=user, course_id=course_id)
-
- if hasattr(record, 'mode'):
- return record.mode
- else:
- return None
+ return (record.mode, record.is_active)
except cls.DoesNotExist:
- return None
+ return (None, None)
@classmethod
def enrollments_for_user(cls, user):
diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py
index d44ee95b80..6ad1962c4e 100644
--- a/common/djangoapps/student/tests/factories.py
+++ b/common/djangoapps/student/tests/factories.py
@@ -10,6 +10,7 @@ import factory
from factory.django import DjangoModelFactory
from uuid import uuid4
from pytz import UTC
+from xmodule.modulestore.locations import SlashSeparatedCourseKey
# Factories don't have __init__ methods, and are self documenting
# pylint: disable=W0232, C0111
@@ -109,14 +110,14 @@ class CourseEnrollmentFactory(DjangoModelFactory):
FACTORY_FOR = CourseEnrollment
user = factory.SubFactory(UserFactory)
- course_id = u'edX/toy/2012_Fall'
+ course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
class CourseEnrollmentAllowedFactory(DjangoModelFactory):
FACTORY_FOR = CourseEnrollmentAllowed
email = 'test@edx.org'
- course_id = 'edX/test/2012_Fall'
+ course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
class PendingEmailChangeFactory(DjangoModelFactory):
diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py
index 105eab04e4..cbcad9ea2b 100644
--- a/lms/djangoapps/certificates/queue.py
+++ b/lms/djangoapps/certificates/queue.py
@@ -181,7 +181,7 @@ class XQueueCertInterface(object):
course_name = course.display_name or course_id.to_deprecated_string()
is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists()
grade = grades.grade(student, self.request, course)
- enrollment_mode = CourseEnrollment.enrollment_mode_for_user(student, course_id)
+ enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id)
mode_is_verified = (enrollment_mode == GeneratedCertificate.MODES.verified)
user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student)
user_is_reverified = SoftwareSecurePhotoVerification.user_is_reverified_for_all(course_id, student)
diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py
index 17d1513667..0d2754ff7c 100644
--- a/lms/djangoapps/verify_student/views.py
+++ b/lms/djangoapps/verify_student/views.py
@@ -65,7 +65,7 @@ class VerifyView(View):
reverse('verify_student_verified',
kwargs={'course_id': course_id.to_deprecated_string()}) + "?upgrade={}".format(upgrade)
)
- elif CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == 'verified':
+ elif CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == ('verified', True):
return redirect(reverse('dashboard'))
else:
# If they haven't completed a verification attempt, we have to
@@ -119,7 +119,7 @@ class VerifiedView(View):
"""
upgrade = request.GET.get('upgrade', False)
course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
- if CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == 'verified':
+ if CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == ('verified', True):
return redirect(reverse('dashboard'))
verify_mode = CourseMode.mode_for_course(course_id, "verified")
@@ -284,7 +284,7 @@ def show_requirements(request, course_id):
Show the requirements necessary for the verification flow.
"""
course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
- if CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == 'verified':
+ if CourseEnrollment.enrollment_mode_for_user(request.user, course_id) == ('verified', True):
return redirect(reverse('dashboard'))
upgrade = request.GET.get('upgrade', False)