Merge pull request #4058 from cpennington/hotfix/lms-2830

Allow users that have unregistered from a verified course to re-register.

common/djangoapps/course_modes/tests/factories.py

 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 = ''

common/djangoapps/course_modes/tests/test_views.py

+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

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
 

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):

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):

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)

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)