diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f5b7250683..6c71fb77fd 100644 --- a/CHANGELOG.rst +++ b/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 diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 7641a27627..24334bcc73 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/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 diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index f081862576..57b7a3543a 100644 --- a/lms/djangoapps/courseware/access.py +++ b/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(): """ diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 6b616ac333..824890e348 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/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 = "
" @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) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 1868779b35..74a03099c0 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/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""" diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index d5434406da..af5015c1d2 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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 diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 4aeded3a34..896342d793 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -171,6 +171,10 @@ ${_("Course is full")} + % elif invitation_only and not can_enroll: + ${_("Enrollment in this course is by invitation only")} + % elif not can_enroll: + ${_("Enrollment is Closed")} %else: ${_("Register for {course.display_number_with_default}").format(course=course) | h} @@ -335,7 +339,7 @@ -%if not registered: +%if active_reg_button: