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 e286ca1259..f7faaeb60d 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 3ebaa053bb..959d109029 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -889,18 +889,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/common/lib/xmodule/xmodule/imageannotation_module.py b/common/lib/xmodule/xmodule/imageannotation_module.py index ceee8661d1..4b2ad809ba 100644 --- a/common/lib/xmodule/xmodule/imageannotation_module.py +++ b/common/lib/xmodule/xmodule/imageannotation_module.py @@ -19,7 +19,9 @@ _ = lambda text: text class AnnotatableFields(object): """ Fields for `ImageModule` and `ImageDescriptor`. """ - 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("""\

diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 90969c1742..69e0629ef2 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -290,7 +290,7 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule): Record in the tracking logs which child was rendered """ # TODO: use publish instead, when publish is wired to the tracking logs - self.system.track_function('xblock.split_test.child_render', {'child-id': self.child.scope_ids.usage_id}) + self.system.track_function('xblock.split_test.child_render', {'child-id': self.child.scope_ids.usage_id.to_deprecated_string()}) return Response() def get_icon_class(self): diff --git a/common/lib/xmodule/xmodule/textannotation_module.py b/common/lib/xmodule/xmodule/textannotation_module.py index 6a06ee51b2..441deaaf8d 100644 --- a/common/lib/xmodule/xmodule/textannotation_module.py +++ b/common/lib/xmodule/xmodule/textannotation_module.py @@ -17,7 +17,9 @@ _ = lambda text: text class AnnotatableFields(object): """Fields for `TextModule` and `TextDescriptor`.""" - 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("""\

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("""\

diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 2845494b3c..790163b6f3 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -180,7 +180,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/notes/views.py b/lms/djangoapps/notes/views.py index 45db26cc1c..f121e2473a 100644 --- a/lms/djangoapps/notes/views.py +++ b/lms/djangoapps/notes/views.py @@ -11,7 +11,6 @@ from xmodule.annotator_token import retrieve_token @login_required def notes(request, course_id): ''' Displays the student's notes. ''' - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_with_access(request.user, 'load', course_key) if not notes_enabled_for_course(course): diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index bee594fb7b..015e4b4e22 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)