diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 1951426ea7..d9720903d3 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1,13 +1,14 @@ import datetime +import feedparser +import itertools import json import logging import random import string import sys -import uuid -import feedparser +import time import urllib -import itertools +import uuid from django.conf import settings from django.contrib.auth import logout, authenticate, login @@ -26,17 +27,19 @@ from bs4 import BeautifulSoup from django.core.cache import cache from django_future.csrf import ensure_csrf_cookie -from student.models import Registration, UserProfile, PendingNameChange, PendingEmailChange, CourseEnrollment +from student.models import (Registration, UserProfile, + PendingNameChange, PendingEmailChange, + CourseEnrollment) from util.cache import cache_if_anonymous from xmodule.course_module import CourseDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError -from models import Registration, UserProfile, PendingNameChange, PendingEmailChange, CourseEnrollment from datetime import date from collections import namedtuple -from courseware.courses import course_staff_group_name, has_staff_access_to_course, get_courses_by_university +from courseware.courses import (course_staff_group_name, has_staff_access_to_course, + get_courses_by_university) log = logging.getLogger("mitx.student") Article = namedtuple('Article', 'title url author image deck publication publish_date') @@ -47,7 +50,8 @@ def csrf_token(context): csrf_token = context.get('csrf_token', '') if csrf_token == 'NOTPROVIDED': return '' - return u'
' % (csrf_token) + return (u'
' % (csrf_token)) @ensure_csrf_cookie @@ -162,6 +166,26 @@ def change_enrollment_view(request): """Delegate to change_enrollment to actually do the work.""" return HttpResponse(json.dumps(change_enrollment(request))) +def enrollment_allowed(user, course): + """If the course has an enrollment period, check whether we are in it. + Also respects the DARK_LAUNCH setting""" + now = time.gmtime() + start = course.enrollment_start + end = course.enrollment_end + + if (start is None or now > start) and (end is None or now < end): + # in enrollment period. + print "allowing enrollment in {}: start {}, end {}, now {}".format( + course.location.url(), start, end, now) + return True + + if settings.MITX_FEATURES['DARK_LAUNCH']: + if has_staff_access_to_course(user, course): + # if dark launch, staff can enroll outside enrollment window + return True + return False + + def change_enrollment(request): if request.method != "POST": raise Http404 @@ -174,7 +198,8 @@ def change_enrollment(request): course_id = request.POST.get("course_id", None) if course_id == None: - return HttpResponse(json.dumps({'success': False, 'error': 'There was an error receiving the course id.'})) + return HttpResponse(json.dumps({'success': False, + 'error': 'There was an error receiving the course id.'})) if action == "enroll": # Make sure the course exists @@ -187,12 +212,20 @@ def change_enrollment(request): return {'success': False, 'error': 'The course requested does not exist.'} if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): - # require that user be in the staff_* group (or be an overall admin) to be able to enroll - # eg staff_6.002x or staff_6.00x + # require that user be in the staff_* group (or be an + # overall admin) to be able to enroll eg staff_6.002x or + # staff_6.00x if not has_staff_access_to_course(user, course): staff_group = course_staff_group_name(course) - log.debug('user %s denied enrollment to %s ; not in %s' % (user,course.location.url(),staff_group)) - return {'success': False, 'error' : '%s membership required to access course.' % staff_group} + log.debug('user %s denied enrollment to %s ; not in %s' % ( + user, course.location.url(), staff_group)) + return {'success': False, + 'error' : '%s membership required to access course.' % staff_group} + + if not enrollment_allowed(user, course): + return {'success': False, + 'error': 'enrollment in {} not allowed at this time' + .format(course.display_name)} enrollment, created = CourseEnrollment.objects.get_or_create(user=user, course_id=course.id) return {'success': True} diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 7ec9f54cd2..e7d480f4e9 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -21,18 +21,35 @@ class CourseDescriptor(SequenceDescriptor): try: self.start = time.strptime(self.metadata["start"], "%Y-%m-%dT%H:%M") except KeyError: - self.start = time.gmtime(0) #The epoch msg = "Course loaded without a start date. id = %s" % self.id - log.critical(msg) except ValueError as e: - self.start = time.gmtime(0) #The epoch msg = "Course loaded with a bad start date. %s '%s'" % (self.id, e) - log.critical(msg) # Don't call the tracker from the exception handler. if msg is not None: + self.start = time.gmtime(0) # The epoch + log.critical(msg) system.error_tracker(msg) + def try_parse_time(key): + """ + Parse an optional metadata key: if present, must be valid. + Return None if not present. + """ + if key in self.metadata: + try: + return time.strptime(self.metadata[key], "%Y-%m-%dT%H:%M") + except ValueError as e: + msg = "Course %s loaded with a bad metadata key %s '%s'" % ( + self.id, self.metadata[key], e) + log.warning(msg) + return None + + self.enrollment_start = try_parse_time("enrollment_start") + self.enrollment_end = try_parse_time("enrollment_end") + + + def has_started(self): return time.gmtime() > self.start @@ -100,7 +117,7 @@ class CourseDescriptor(SequenceDescriptor): for s in c.get_children(): if s.metadata.get('graded', False): xmoduledescriptors = list(yield_descriptor_descendents(s)) - + # The xmoduledescriptors included here are only the ones that have scores. section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : filter(lambda child: child.has_score, xmoduledescriptors) } diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 0fb5c9983e..daffa44d2a 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -58,8 +58,22 @@ def mongo_store_config(data_dir): } } +def xml_store_config(data_dir): + return { + 'default': { + 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', + 'OPTIONS': { + 'data_dir': data_dir, + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + 'eager': True, + } + } +} + + TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT -TEST_DATA_MODULESTORE = mongo_store_config(TEST_DATA_DIR) +TEST_DATA_MONGO_MODULESTORE = mongo_store_config(TEST_DATA_DIR) +TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR) REAL_DATA_DIR = settings.GITHUB_REPO_ROOT REAL_DATA_MODULESTORE = mongo_store_config(REAL_DATA_DIR) @@ -149,8 +163,27 @@ class ActivateLoginTestCase(TestCase): class PageLoader(ActivateLoginTestCase): ''' Base class that adds a function to load all pages in a modulestore ''' + def _enroll(self, course): + """Post to the enrollment view, and return the parsed json response""" + resp = self.client.post('/change_enrollment', { + 'enrollment_action': 'enroll', + 'course_id': course.id, + }) + return parse_json(resp) + + def try_enroll(self, course): + """Try to enroll. Return bool success instead of asserting it.""" + data = self._enroll(course) + print 'Enrollment in {} result: {}'.format(course.location.url(), data) + return data['success'] + def enroll(self, course): """Enroll the currently logged-in user, and check that it worked.""" + data = self._enroll(course) + self.assertTrue(data['success']) + + def unenroll(self, course): + """Unenroll the currently logged-in user, and check that it worked.""" resp = self.client.post('/change_enrollment', { 'enrollment_action': 'enroll', 'course_id': course.id, @@ -159,6 +192,7 @@ class PageLoader(ActivateLoginTestCase): self.assertTrue(data['success']) def check_pages_load(self, course_name, data_dir, modstore): + """Make all locations in course load""" print "Checking course {0} in {1}".format(course_name, data_dir) import_from_xml(modstore, data_dir, [course_name]) @@ -191,7 +225,7 @@ class PageLoader(ActivateLoginTestCase): self.assertTrue(all_ok) -@override_settings(MODULESTORE=TEST_DATA_MODULESTORE) +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) class TestCoursesLoadTestCase(PageLoader): '''Check that all pages in test courses load properly''' @@ -207,7 +241,7 @@ class TestCoursesLoadTestCase(PageLoader): self.check_pages_load('full', TEST_DATA_DIR, modulestore()) -@override_settings(MODULESTORE=TEST_DATA_MODULESTORE) +@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestViewAuth(PageLoader): """Check that view authentication works properly""" @@ -215,15 +249,15 @@ class TestViewAuth(PageLoader): # can't do imports there without manually hacking settings. def setUp(self): - print "sys.path: {}".format(sys.path) xmodule.modulestore.django._MODULESTORES = {} - modulestore().collection.drop() - import_from_xml(modulestore(), TEST_DATA_DIR, ['toy']) - import_from_xml(modulestore(), TEST_DATA_DIR, ['full']) courses = modulestore().get_courses() - # get the two courses sorted out - courses.sort(key=lambda c: c.location.course) - [self.full, self.toy] = courses + + def find_course(name): + """Assumes the course is present""" + return [c for c in courses if c.location.course==name][0] + + self.full = find_course("full") + self.toy = find_course("toy") # Create two accounts self.student = 'view@test.com' @@ -304,26 +338,35 @@ class TestViewAuth(PageLoader): self.check_for_get_code(200, url) - def test_dark_launch(self): - """Make sure that when dark launch is on, students can't access course - pages, but instructors can""" - - # test.py turns off start dates, enable them and set them correctly. - # Because settings is global, be careful not to mess it up for other tests - # (Can't use override_settings because we're only changing part of the - # MITX_FEATURES dict) + def run_wrapped(self, test): + """ + test.py turns off start dates. Enable them and DARK_LAUNCH. + Because settings is global, be careful not to mess it up for other tests + (Can't use override_settings because we're only changing part of the + MITX_FEATURES dict) + """ oldDSD = settings.MITX_FEATURES['DISABLE_START_DATES'] oldDL = settings.MITX_FEATURES['DARK_LAUNCH'] try: settings.MITX_FEATURES['DISABLE_START_DATES'] = False settings.MITX_FEATURES['DARK_LAUNCH'] = True - self._do_test_dark_launch() + test() finally: settings.MITX_FEATURES['DISABLE_START_DATES'] = oldDSD settings.MITX_FEATURES['DARK_LAUNCH'] = oldDL + def test_dark_launch(self): + """Make sure that when dark launch is on, students can't access course + pages, but instructors can""" + self.run_wrapped(self._do_test_dark_launch) + + def test_enrollment_period(self): + """Check that enrollment periods work""" + self.run_wrapped(self._do_test_enrollment_period) + + def _do_test_dark_launch(self): """Actually do the test, relying on settings to be right.""" @@ -338,6 +381,7 @@ class TestViewAuth(PageLoader): self.assertTrue(settings.MITX_FEATURES['DARK_LAUNCH']) def reverse_urls(names, course): + """Reverse a list of course urls""" return [reverse(name, kwargs={'course_id': course.id}) for name in names] def dark_student_urls(course): @@ -424,6 +468,53 @@ class TestViewAuth(PageLoader): check_staff(self.toy) check_staff(self.full) + def _do_test_enrollment_period(self): + """Actually do the test, relying on settings to be right.""" + + # Make courses start in the future + tomorrow = time.time() + 24 * 3600 + nextday = tomorrow + 24 * 3600 + yesterday = time.time() - 24 * 3600 + + print "changing" + # toy course's enrollment period hasn't started + self.toy.enrollment_start = time.gmtime(tomorrow) + self.toy.enrollment_end = time.gmtime(nextday) + + # full course's has + self.full.enrollment_start = time.gmtime(yesterday) + self.full.enrollment_end = time.gmtime(tomorrow) + + print "login" + # First, try with an enrolled student + print '=== Testing student access....' + self.login(self.student, self.password) + self.assertFalse(self.try_enroll(self.toy)) + self.assertTrue(self.try_enroll(self.full)) + + print '=== Testing course instructor access....' + # Make the instructor staff in the toy course + group_name = course_staff_group_name(self.toy) + g = Group.objects.create(name=group_name) + g.user_set.add(user(self.instructor)) + + print "logout/login" + self.logout() + self.login(self.instructor, self.password) + print "Instructor should be able to enroll in toy course" + self.assertTrue(self.try_enroll(self.toy)) + + print '=== Testing staff access....' + # now make the instructor global staff, but not in the instructor group + g.user_set.remove(user(self.instructor)) + u = user(self.instructor) + u.is_staff = True + u.save() + + # unenroll and try again + self.unenroll(self.toy) + self.assertTrue(self.try_enroll(self.toy)) + @override_settings(MODULESTORE=REAL_DATA_MODULESTORE) class RealCoursesLoadTestCase(PageLoader):