Add DARK_LAUNCH functionality

* pass user to check_course * if dark launch feature enabled, users with staff access to course can see courseware before start date. Students still can't. * tests. * Remaining: enrollment view has custom access control. Need to check it.
2012-08-12 16:23:31 -04:00
parent 622eebc473
commit d7f94a05b7
7 changed files with 178 additions and 37 deletions
--- a/lms/djangoapps/courseware/courses.py
+++ b/lms/djangoapps/courseware/courses.py
@@ -15,11 +15,11 @@ from static_replace import replace_urls, try_staticfiles_lookup
 log = logging.getLogger(__name__)


-def check_course(course_id, course_must_be_open=True, course_required=True):
+def check_course(user, course_id, course_must_be_open=True, course_required=True):
    """
-    Given a course_id, this returns the course object. By default,
-    if the course is not found or the course is not open yet, this
-    method will raise a 404.
+    Given a django user and a course_id, this returns the course
+    object. By default, if the course is not found or the course is
+    not open yet, this method will raise a 404.

    If course_must_be_open is False, the course will be returned
    without a 404 even if it is not open.
@@ -27,6 +27,10 @@ def check_course(course_id, course_must_be_open=True, course_required=True):
    If course_required is False, a course_id of None is acceptable. The
    course returned will be None. Even if the course is not required,
    if a course_id is given that does not exist a 404 will be raised.
+
+    This behavior is modified by MITX_FEATURES['DARK_LAUNCH']:
+       if dark launch is enabled, course_must_be_open is ignored for
+    users that have staff access.
    """
    course = None
    if course_required or course_id:
@@ -38,7 +42,13 @@ def check_course(course_id, course_must_be_open=True, course_required=True):
            raise Http404("Course not found.")

        started = course.has_started() or settings.MITX_FEATURES['DISABLE_START_DATES']
-        if course_must_be_open and not started:
+
+        must_be_open = course_must_be_open
+        if (settings.MITX_FEATURES['DARK_LAUNCH'] and
+            has_staff_access_to_course(user, course)):
+            must_be_open = False
+
+        if must_be_open and not started:
            raise Http404("This course has not yet started.")

    return course
--- a/lms/djangoapps/courseware/tests/tests.py
+++ b/lms/djangoapps/courseware/tests/tests.py
@@ -1,11 +1,14 @@
 import copy
 import json
-from path import path
 import os
+import sys
+import time

-from pprint import pprint
 from nose import SkipTest
+from path import path
+from pprint import pprint

+from django.contrib.auth.models import User, Group
 from django.test import TestCase
 from django.test.client import Client
 from django.conf import settings
@@ -13,12 +16,11 @@ from django.core.urlresolvers import reverse
 from mock import patch, Mock
 from override_settings import override_settings

-from django.contrib.auth.models import User, Group
+import xmodule.modulestore.django
+
 from student.models import Registration
 from courseware.courses import course_staff_group_name
-
 from xmodule.modulestore.django import modulestore
-import xmodule.modulestore.django
 from xmodule.modulestore import Location
 from xmodule.modulestore.xml_importer import import_from_xml

@@ -206,13 +208,14 @@ class TestCoursesLoadTestCase(PageLoader):


@override_settings(MODULESTORE=TEST_DATA_MODULESTORE)
-class TestInstructorAuth(PageLoader):
-    """Check that authentication works properly"""
+class TestViewAuth(PageLoader):
+    """Check that view authentication works properly"""

    # NOTE: setUpClass() runs before override_settings takes effect, so
    # 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'])
@@ -237,12 +240,16 @@ class TestInstructorAuth(PageLoader):
        # TODO (vshnayder): once we're returning 404s, get rid of this if.
        if code != 404:
            self.assertEqual(resp.status_code, code)
+            # And 'page not found' shouldn't be in the returned page
+            self.assertTrue(resp.content.lower().find('page not found') == -1)
        else:
            # look for "page not found" instead of the status code
+            #print resp.content
            self.assertTrue(resp.content.lower().find('page not found') != -1)

-    def test_instructor_page(self):
-        "Make sure only instructors can load it"
+    def test_instructor_pages(self):
+        """Make sure only instructors for the course or staff can load the instructor
+        dashboard, the grade views, and student profile pages"""

        # First, try with an enrolled student
        self.login(self.student, self.password)
@@ -297,7 +304,125 @@ class TestInstructorAuth(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)
+        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()
+        finally:
+            settings.MITX_FEATURES['DISABLE_START_DATES'] = oldDSD
+            settings.MITX_FEATURES['DARK_LAUNCH'] = oldDL
+
+
+    def _do_test_dark_launch(self):
+        """Actually do the test, relying on settings to be right."""
+
+        # Make courses start in the future
+        tomorrow = time.time() + 24*3600
+        self.toy.start = self.toy.metadata['start'] = time.gmtime(tomorrow)
+        self.full.start = self.full.metadata['start'] = time.gmtime(tomorrow)
+
+        self.assertFalse(self.toy.has_started())
+        self.assertFalse(self.full.has_started())
+        self.assertFalse(settings.MITX_FEATURES['DISABLE_START_DATES'])
+        self.assertTrue(settings.MITX_FEATURES['DARK_LAUNCH'])
+
+        def reverse_urls(names, course):
+            return [reverse(name, kwargs={'course_id': course.id}) for name in names]
+
+        def dark_student_urls(course):
+            """
+            list of urls that students should be able to see only
+            after launch, but staff should see before
+            """
+            urls = reverse_urls(['info', 'book', 'courseware', 'profile'], course)
+            return urls
+
+        def light_student_urls(course):
+            """
+            list of urls that students should be able to see before
+            launch.
+            """
+            urls = reverse_urls(['about_course'], course)
+            urls.append(reverse('courses'))
+            # Need separate test for change_enrollment, since it's a POST view
+            #urls.append(reverse('change_enrollment'))
+
+            return urls
+
+        def instructor_urls(course):
+            """list of urls that only instructors/staff should be able to see"""
+            urls = reverse_urls(['instructor_dashboard','gradebook','grade_summary'],
+                                course)
+            urls.append(reverse('student_profile', kwargs={'course_id': course.id,
+                                                     'student_id': user(self.student).id}))
+            return urls
+
+        def check_non_staff(course):
+            """Check that access is right for non-staff in course"""
+            print '=== Checking non-staff access for {}'.format(course.id)
+            for url in instructor_urls(course) + dark_student_urls(course):
+                print 'checking for 404 on {}'.format(url)
+                self.check_for_get_code(404, url)
+
+            for url in light_student_urls(course):
+                print 'checking for 200 on {}'.format(url)
+                self.check_for_get_code(200, url)
+
+        def check_staff(course):
+            """Check that access is right for staff in course"""
+            print '=== Checking staff access for {}'.format(course.id)
+            for url in (instructor_urls(course) +
+                        dark_student_urls(course) +
+                        light_student_urls(course)):
+                print 'checking for 200 on {}'.format(url)
+                self.check_for_get_code(200, url)
+
+        # First, try with an enrolled student
+        print '=== Testing student access....'
+        self.login(self.student, self.password)
+        self.enroll(self.toy)
+        self.enroll(self.full)
+
+        # shouldn't be able to get to anything except the light pages
+        check_non_staff(self.toy)
+        check_non_staff(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))
+
+        self.logout()
+        self.login(self.instructor, self.password)
+        # Enroll in the classes---can't see courseware otherwise.
+        self.enroll(self.toy)
+        self.enroll(self.full)
+
+        # should now be able to get to everything for toy course
+        check_non_staff(self.full)
+        check_staff(self.toy)
+
+        print '=== Testing staff access....'
+        # now also make the instructor staff
+        u = user(self.instructor)
+        u.is_staff = True
+        u.save()
+
+        # and now should be able to load both
+        check_staff(self.toy)
+        check_staff(self.full)


@override_settings(MODULESTORE=REAL_DATA_MODULESTORE)
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -110,9 +110,10 @@ def index(request, course_id, chapter=None, section=None,

     - HTTPresponse
    '''
-    course = check_course(course_id)
+    course = check_course(request.user, course_id)
    registered = registered_for_course(course, request.user)
    if not registered:
+        # TODO (vshnayder): do course instructors need to be registered to see course?
        log.debug('User %s tried to view course %s but is not enrolled' % (request.user,course.location.url()))
        return redirect(reverse('about_course', args=[course.id]))

@@ -203,7 +204,7 @@ def course_info(request, course_id):

    Assumes the course_id is in a valid format.
    """
-    course = check_course(course_id)
+    course = check_course(request.user, course_id)

    return render_to_response('info.html', {'course': course})

@@ -220,7 +221,7 @@ def registered_for_course(course, user):
@ensure_csrf_cookie
@cache_if_anonymous
 def course_about(request, course_id):
-    course = check_course(course_id, course_must_be_open=False)
+    course = check_course(request.user, course_id, course_must_be_open=False)
    registered = registered_for_course(course, request.user)
    return render_to_response('portal/course_about.html', {'course': course, 'registered': registered})

@@ -252,7 +253,7 @@ def profile(request, course_id, student_id=None):

    Course staff are allowed to see the profiles of students in their class.
    """
-    course = check_course(course_id)
+    course = check_course(request.user, course_id)

    if student_id is None or student_id == request.user.id:
        # always allowed to see your own profile
@@ -299,7 +300,7 @@ def gradebook(request, course_id):
    if not has_staff_access_to_course_id(request.user, course_id):
        raise Http404

-    course = check_course(course_id)
+    course = check_course(request.user, course_id)

    enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).order_by('username')

@@ -324,7 +325,7 @@ def grade_summary(request, course_id):
    if not has_staff_access_to_course_id(request.user, course_id):
        raise Http404

-    course = check_course(course_id)
+    course = check_course(request.user, course_id)

    # For now, just a static page
    context = {'course': course }
@@ -337,7 +338,7 @@ def instructor_dashboard(request, course_id):
    if not has_staff_access_to_course_id(request.user, course_id):
        raise Http404

-    course = check_course(course_id)
+    course = check_course(request.user, course_id)

    # For now, just a static page
    context = {'course': course }
--- a/lms/djangoapps/simplewiki/views.py
+++ b/lms/djangoapps/simplewiki/views.py
@@ -51,7 +51,7 @@ def update_template_dictionary(dictionary, request=None, course=None, article=No


 def view(request, article_path, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    (article, err) = get_article(request, article_path, course)
    if err:
@@ -67,7 +67,7 @@ def view(request, article_path, course_id=None):


 def view_revision(request, revision_number, article_path, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    (article, err) = get_article(request, article_path, course)
    if err:
@@ -91,7 +91,7 @@ def view_revision(request, revision_number, article_path, course_id=None):


 def root_redirect(request, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    #TODO: Add a default namespace to settings.
    namespace = course.wiki_namespace if course else "edX"
@@ -109,7 +109,7 @@ def root_redirect(request, course_id=None):


 def create(request, article_path, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    article_path_components = article_path.split('/')

@@ -170,7 +170,7 @@ def create(request, article_path, course_id=None):


 def edit(request, article_path, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    (article, err) = get_article(request, article_path, course)
    if err:
@@ -218,7 +218,7 @@ def edit(request, article_path, course_id=None):


 def history(request, article_path, page=1, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    (article, err) = get_article(request, article_path, course)
    if err:
@@ -300,7 +300,7 @@ def history(request, article_path, page=1, course_id=None):


 def revision_feed(request, page=1, namespace=None, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    page_size = 10

@@ -333,7 +333,7 @@ def revision_feed(request, page=1, namespace=None, course_id=None):


 def search_articles(request, namespace=None, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    # blampe: We should check for the presence of other popular django search
    # apps and use those if possible. Only fall back on this as a last resort.
@@ -382,7 +382,7 @@ def search_articles(request, namespace=None, course_id=None):


 def search_add_related(request, course_id, slug, namespace):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    (article, err) = get_article(request, slug, namespace if namespace else course_id)
    if err:
@@ -415,7 +415,7 @@ def search_add_related(request, course_id, slug, namespace):


 def add_related(request, course_id, slug, namespace):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    (article, err) = get_article(request, slug, namespace if namespace else course_id)
    if err:
@@ -439,7 +439,7 @@ def add_related(request, course_id, slug, namespace):


 def remove_related(request, course_id, namespace, slug, related_id):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    (article, err) = get_article(request, slug, namespace if namespace else course_id)

@@ -462,7 +462,7 @@ def remove_related(request, course_id, namespace, slug, related_id):


 def random_article(request, course_id=None):
-    course = check_course(course_id, course_required=False)
+    course = check_course(request.user, course_id, course_required=False)

    from random import randint
    num_arts = Article.objects.count()
--- a/lms/djangoapps/staticbook/views.py
+++ b/lms/djangoapps/staticbook/views.py
@@ -6,7 +6,7 @@ from lxml import etree

@login_required
 def index(request, course_id, page=0):
-    course = check_course(course_id)
+    course = check_course(request.user, course_id)
    raw_table_of_contents = open('lms/templates/book_toc.xml', 'r') # TODO: This will need to come from S3
    table_of_contents = etree.parse(raw_table_of_contents).getroot()
    return render_to_response('staticbook.html', {'page': int(page), 'course': course, 'table_of_contents': table_of_contents})
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -48,6 +48,7 @@ MITX_FEATURES = {
    ## DO NOT SET TO True IN THIS FILE
    ## Doing so will cause all courses to be released on production
    'DISABLE_START_DATES': False,  # When True, all courses will be active, regardless of start date
+    'DARK_LAUNCH': False,  # When True, courses will be active for staff only

    'ENABLE_TEXTBOOK' : True,
    'ENABLE_DISCUSSION' : True,
--- a/lms/urls.py
+++ b/lms/urls.py
@@ -97,12 +97,16 @@ if settings.PERFSTATS:

 if settings.COURSEWARE_ENABLED:
    urlpatterns += (
+        # Hook django-masquerade, allowing staff to view site as other users
        url(r'^masquerade/', include('masquerade.urls')),
        url(r'^jump_to/(?P<location>.*)$', 'courseware.views.jump_to', name="jump_to"),

-        url(r'^modx/(?P<id>.*?)/(?P<dispatch>[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'),
-        url(r'^xqueue/(?P<userid>[^/]*)/(?P<id>.*?)/(?P<dispatch>[^/]*)$', 'courseware.module_render.xqueue_callback'),
-        url(r'^change_setting$', 'student.views.change_setting'),
+        url(r'^modx/(?P<id>.*?)/(?P<dispatch>[^/]*)$',
+            'courseware.module_render.modx_dispatch', name='modx_dispatch'),
+        url(r'^xqueue/(?P<userid>[^/]*)/(?P<id>.*?)/(?P<dispatch>[^/]*)$',
+            'courseware.module_render.xqueue_callback', name='xqueue_callback'),
+        url(r'^change_setting$', 'student.views.change_setting',
+            name='change_setting'),

        # TODO: These views need to be updated before they work
        # url(r'^calculate$', 'util.views.calculate'),