Merge pull request #4418 from Stanford-Online/dcadams/make_registration_button_conditional

Make text on register button on About page conditional.

CHANGELOG.rst

@@ -5,6 +5,12 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio: New advanced setting "invitation_only" for courses. This setting overrides the enrollment start/end dates
+  if set. LMS-2670
+
+LMS: Register button on About page was active even when greyed out. Now made inactive when appropriate and
+displays appropriate context sensitive message to student. LMS-2717
+
 Blades: Redirect Chinese students to a Chinese CDN for video. BLD-1052.
 
 Studio: Show display names and help text in Advanced Settings. Also hide deprecated settings

common/lib/xmodule/xmodule/course_module.py

@@ -540,6 +540,11 @@ class CourseFields(object):
                                        default=False,
                                        scope=Scope.settings)
 
+    invitation_only = Boolean(display_name=_("Invitation Only"),
+                              help="Whether to restrict enrollment to invitation by the course staff.",
+                              default=False,
+                              scope=Scope.settings)
+
 class CourseDescriptor(CourseFields, SequenceDescriptor):
     module_class = SequenceModule
 

lms/djangoapps/courseware/access.py

@@ -3,6 +3,7 @@ Ideally, it will be the only place that needs to know about any special settings
 like DISABLE_START_DATES"""
 import logging
 from datetime import datetime, timedelta
+import pytz
 
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
@@ -139,12 +140,14 @@ def _has_access_course_desc(user, action, course):
         If it is, then the user must pass the criterion set by the course, e.g. that ExternalAuthMap
             was set by 'shib:https://idp.stanford.edu/", in addition to requirements below.
         Rest of requirements:
-        Enrollment can only happen in the course enrollment period, if one exists.
-            or
-
         (CourseEnrollmentAllowed always overrides)
+          or
         (staff can always enroll)
+          or
+        Enrollment can only happen in the course enrollment period, if one exists, and
+        course is not invitation only.
         """
+
         # if using registration method to restrict (say shibboleth)
         if settings.FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain:
             if user is not None and user.is_authenticated() and \
@@ -154,16 +157,11 @@ def _has_access_course_desc(user, action, course):
             else:
                 reg_method_ok = False
         else:
-            reg_method_ok = True #if not using this access check, it's always OK.
+            reg_method_ok = True  # if not using this access check, it's always OK.
 
         now = datetime.now(UTC())
-        start = course.enrollment_start
-        end = course.enrollment_end
-
-        if reg_method_ok and (start is None or now > start) and (end is None or now < end):
-            # in enrollment period, so any user is allowed to enroll.
-            debug("Allow: in enrollment period")
-            return True
+        start = course.enrollment_start or datetime.min.replace(tzinfo=pytz.UTC)
+        end = course.enrollment_end or datetime.max.replace(tzinfo=pytz.UTC)
 
         # if user is in CourseEnrollmentAllowed with right course key then can also enroll
         # (note that course.id actually points to a CourseKey)
@@ -173,8 +171,17 @@ def _has_access_course_desc(user, action, course):
             if CourseEnrollmentAllowed.objects.filter(email=user.email, course_id=course.id):
                 return True
 
-        # otherwise, need staff access
-        return _has_staff_access_to_descriptor(user, course, course.id)
+        if _has_staff_access_to_descriptor(user, course, course.id):
+            return True
+
+        # Invitation_only doesn't apply to CourseEnrollmentAllowed or has_staff_access_access
+        if course.invitation_only:
+            debug("Deny: invitation only")
+            return False
+
+        if reg_method_ok and start < now < end:
+            debug("Allow: in enrollment period")
+            return True
 
     def see_exists():
         """

lms/djangoapps/courseware/tests/test_about.py

@@ -2,6 +2,8 @@
 Test the about xblock
 """
 import mock
+import pytz
+import datetime
 from django.test.utils import override_settings
 from django.core.urlresolvers import reverse
 
@@ -10,6 +12,10 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from student.tests.factories import UserFactory, CourseEnrollmentAllowedFactory
+
+# HTML for registration button
+REG_STR = "<form id=\"class_enroll_form\" method=\"post\" data-remote=\"true\" action=\"/change_enrollment\">"
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -34,6 +40,9 @@ class AboutTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
         self.assertEqual(resp.status_code, 200)
         self.assertIn("OOGIE BLOOGIE", resp.content)
 
+        # Check that registration button is present
+        self.assertIn(REG_STR, resp.content)
+
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase):
@@ -107,3 +116,88 @@ class AboutWithCappedEnrollmentsTestCase(LoginEnrollmentTestCase, ModuleStoreTes
         # Try to enroll as well
         result = self.enroll(self.course)
         self.assertFalse(result)
+
+        # Check that registration button is not present
+        self.assertNotIn(REG_STR, resp.content)
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class AboutWithInvitationOnly(ModuleStoreTestCase):
+    """
+    This test case will check the About page when a course is invitation only.
+    """
+    def setUp(self):
+
+        self.course = CourseFactory.create(metadata={"invitation_only": True})
+
+        self.about = ItemFactory.create(
+            category="about", parent_location=self.course.location,
+            display_name="overview"
+        )
+
+    def test_invitation_only(self):
+        """
+        Test for user not logged in, invitation only course.
+        """
+
+        url = reverse('about_course', args=[self.course.id.to_deprecated_string()])
+        resp = self.client.get(url)
+        self.assertEqual(resp.status_code, 200)
+        self.assertIn("Enrollment in this course is by invitation only", resp.content)
+
+        # Check that registration button is not present
+        self.assertNotIn(REG_STR, resp.content)
+
+    def test_invitation_only_but_allowed(self):
+        """
+        Test for user logged in and allowed to enroll in invitation only course.
+        """
+
+        # Course is invitation only, student is allowed to enroll and logged in
+        user = UserFactory.create(username='allowed_student', password='test', email='allowed_student@test.com')
+        CourseEnrollmentAllowedFactory(email=user.email, course_id=self.course.id)
+        self.client.login(username=user.username, password='test')
+
+        url = reverse('about_course', args=[self.course.id.to_deprecated_string()])
+        resp = self.client.get(url)
+        self.assertEqual(resp.status_code, 200)
+        self.assertIn("Register for 999", resp.content)
+
+        # Check that registration button is present
+        self.assertIn(REG_STR, resp.content)
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class AboutWithClosedEnrollment(ModuleStoreTestCase):
+    """
+    This test case will check the About page for a course that has enrollment start/end
+    set but it is currently outside of that period.
+    """
+    def setUp(self):
+
+        super(AboutWithClosedEnrollment, self).setUp()
+        self.course = CourseFactory.create(metadata={"invitation_only": False})
+
+        # Setup enrollment period to be in future
+        now = datetime.datetime.now(pytz.UTC)
+        tomorrow = now + datetime.timedelta(days=1)
+        nextday = tomorrow + datetime.timedelta(days=1)
+
+        self.course.enrollment_start = tomorrow
+        self.course.enrollment_end = nextday
+        self.course = self.update_course(self.course, self.user.id)
+
+        self.about = ItemFactory.create(
+            category="about", parent_location=self.course.location,
+            display_name="overview"
+        )
+
+    def test_closed_enrollmement(self):
+
+        url = reverse('about_course', args=[self.course.id.to_deprecated_string()])
+        resp = self.client.get(url)
+        self.assertEqual(resp.status_code, 200)
+        self.assertIn("Enrollment is Closed", resp.content)
+
+        # Check that registration button is not present
+        self.assertNotIn(REG_STR, resp.content)

lms/djangoapps/courseware/tests/test_access.py

@@ -140,40 +140,48 @@ class AccessTestCase(TestCase):
         verify_access(False)
 
     def test__has_access_course_desc_can_enroll(self):
-        user = Mock()
         yesterday = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1)
         tomorrow = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1)
-        course = Mock(enrollment_start=yesterday, enrollment_end=tomorrow, enrollment_domain='')
-
-        # User can enroll if it is between the start and end dates
-        self.assertTrue(access._has_access_course_desc(user, 'enroll', course))
 
-        # User can enroll if authenticated and specifically allowed for that course
+        # Non-staff can enroll if authenticated and specifically allowed for that course
         # even outside the open enrollment period
-        user = Mock(email='test@edx.org', is_staff=False)
-        user.is_authenticated.return_value = True
-
+        user = UserFactory.create()
         course = Mock(
             enrollment_start=tomorrow, enrollment_end=tomorrow,
             id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'), enrollment_domain=''
         )
-
         CourseEnrollmentAllowedFactory(email=user.email, course_id=course.id)
-
         self.assertTrue(access._has_access_course_desc(user, 'enroll', course))
 
         # Staff can always enroll even outside the open enrollment period
-        user = Mock(email='test@edx.org', is_staff=True)
-        user.is_authenticated.return_value = True
+        user = StaffFactory.create(course_key=course.id)
+        self.assertTrue(access._has_access_course_desc(user, 'enroll', course))
 
+        # Non-staff cannot enroll if it is between the start and end dates and invitation only
+        # and not specifically allowed
         course = Mock(
-            enrollment_start=tomorrow, enrollment_end=tomorrow,
-            id=SlashSeparatedCourseKey('edX', 'test', 'Whenever'), enrollment_domain='',
+            enrollment_start=yesterday, enrollment_end=tomorrow,
+            id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'), enrollment_domain='',
+            invitation_only=True
+        )
+        user = UserFactory.create()
+        self.assertFalse(access._has_access_course_desc(user, 'enroll', course))
+
+        # Non-staff can enroll if it is between the start and end dates and not invitation only
+        course = Mock(
+            enrollment_start=yesterday, enrollment_end=tomorrow,
+            id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'), enrollment_domain='',
+            invitation_only=False
         )
         self.assertTrue(access._has_access_course_desc(user, 'enroll', course))
 
-        # TODO:
         # Non-staff cannot enroll outside the open enrollment period if not specifically allowed
+        course = Mock(
+            enrollment_start=tomorrow, enrollment_end=tomorrow,
+            id=SlashSeparatedCourseKey('edX', 'test', '2012_Fall'), enrollment_domain='',
+            invitation_only=False
+        )
+        self.assertFalse(access._has_access_course_desc(user, 'enroll', course))
 
     def test__user_passed_as_none(self):
         """Ensure has_access handles a user being passed as null"""

lms/djangoapps/courseware/views.py

@@ -55,6 +55,7 @@ template_imports = {'urllib': urllib}
 
 CONTENT_DEPTH = 2
 
+
 def user_groups(user):
     """
     TODO (vshnayder): This is not used. When we have a new plan for groups, adjust appropriately.
@@ -643,9 +644,17 @@ def course_about(request, course_id):
         reg_then_add_to_cart_link = "{reg_url}?course_id={course_id}&enrollment_action=add_to_cart".format(
             reg_url=reverse('register_user'), course_id=course.id.to_deprecated_string())
 
-    # see if we have already filled up all allowed enrollments
+    # Used to provide context to message to student if enrollment not allowed
+    can_enroll = has_access(request.user, 'enroll', course)
+    invitation_only = course.invitation_only
     is_course_full = CourseEnrollment.is_course_full(course)
 
+    # Register button should be disabled if one of the following is true:
+    # - Student is already registered for course
+    # - Course is already full
+    # - Student cannot enroll in course
+    active_reg_button = not(registered or is_course_full or not can_enroll)
+
     return render_to_response('courseware/course_about.html', {
         'course': course,
         'staff_access': staff_access,
@@ -656,7 +665,10 @@ def course_about(request, course_id):
         'in_cart': in_cart,
         'reg_then_add_to_cart_link': reg_then_add_to_cart_link,
         'show_courseware_link': show_courseware_link,
-        'is_course_full': is_course_full
+        'is_course_full': is_course_full,
+        'can_enroll': can_enroll,
+        'invitation_only': invitation_only,
+        'active_reg_button': active_reg_button,
     })
 
 
@@ -882,7 +894,7 @@ def get_static_tab_contents(request, course, tab):
         except Exception:  # pylint: disable=broad-except
             html = render_to_string('courseware/error-message.html', None)
             log.exception(
-                u"Error rendering course={course}, tab={tab_url}".format(course=course,tab_url=tab['url_slug'])
+                u"Error rendering course={course}, tab={tab_url}".format(course=course, tab_url=tab['url_slug'])
             )
 
     return html

lms/templates/courseware/course_about.html

@@ -171,6 +171,10 @@
           <span class="register disabled">
             ${_("Course is full")}
           </span>
+        % elif invitation_only and not can_enroll:
+          <span class="register disabled">${_("Enrollment in this course is by invitation only")}</span>
+        % elif not can_enroll:
+          <span class="register disabled">${_("Enrollment is Closed")}</span>
         %else:
           <a href="#" class="register">
             ${_("Register for {course.display_number_with_default}").format(course=course) | h}
@@ -335,7 +339,7 @@
   </section>
 </section>
 
-%if not registered:
+%if active_reg_button:
   <div style="display: none;">
     <form id="class_enroll_form" method="post" data-remote="true" action="${reverse('change_enrollment')}">
       <fieldset class="enroll_fieldset">