Changed check_course from a decorator to a regular function that throws…

Changed check_course from a decorator to a regular function that throws exceptions. Much easier to understand now.

common/djangoapps/student/views.py

@@ -20,7 +20,7 @@ from django.shortcuts import redirect
 from mitxmako.shortcuts import render_to_response, render_to_string
 from django.core.urlresolvers import reverse
 
-from courseware.decorators import check_course
+from courseware.courses import check_course
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.django import modulestore
 from django_future.csrf import ensure_csrf_cookie
@@ -495,8 +495,8 @@ def accept_name_change(request):
 
 
 @ensure_csrf_cookie
-@check_course(course_must_be_open=False)
-def course_info(request, course):
+def course_info(request, course_id):
+    course = check_course(course_id, course_must_be_open=False)
     # This is the advertising page for a student to look at the course before signing up
     csrf_token = csrf(request)['csrf_token']
     # TODO: Couse should be a model
@@ -519,9 +519,10 @@ def help(request):
 
 
 @login_required
-@check_course(course_must_be_open=False)
 @ensure_csrf_cookie
-def enroll(request, course):
+def enroll(request, course_id):
+    course = check_course(course_id, course_must_be_open=False)
+    
     user = request.user
     enrollment = CourseEnrollment(user=user,
         course_id=course.id)

lms/djangoapps/courseware/courses.py

-from collections import namedtuple
-import logging
-import os
+from functools import wraps
 
-from path import path
-import yaml
+from django.http import Http404
 
-log = logging.getLogger('mitx.courseware.courses')
+from xmodule.course_module import CourseDescriptor
+from xmodule.modulestore.django import modulestore
 
-_FIELDS = ['number', # 6.002x
-           'title', # Circuits and Electronics
-           'short_title', # Circuits
-           'run_id', # Spring 2012
-           'path', # /some/absolute/filepath/6.002x --> course.xml is in here.
-           'instructors', # ['Anant Agarwal']
-           'institution', # "MIT"
-           'wiki_namespace',
-           'grader', # a courseware.graders.CourseGrader object
 
-           #'start', # These should be datetime fields
-           #'end'
-           ]
-
-class CourseInfoLoadError(Exception):
-    pass
-
-class Course(namedtuple('Course', _FIELDS)):
-    """Course objects encapsulate general information about a given run of a
-    course. This includes things like name, grading policy, etc.
+def check_course(course_id, course_must_be_open=True, course_required=True):
     """
-
-def load_courses(courses_path):
-    """Given a directory of courses, returns a list of Course objects. For the
-    sake of backwards compatibility, if you point it at the top level of a
-    specific course, it will return a list with one Course object in it.
+    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.
+    
+    If course_must_be_open is False, the course will be returned
+    without a 404 even if it is not open.
+    
+    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.
     """
-    courses_path = path(courses_path)
-    def _is_course_path(p):
-        return os.path.exists(p / "course_info.yaml")
-
-    log.info("Loading courses from {0}".format(courses_path))
-
-    # Compatibility: courses_path is the path for a single course
-    if _is_course_path(courses_path):
-        log.warning("course_info.yaml found in top-level ({0})"
-                    .format(courses_path) +
-                    " -- assuming there is only a single course.")
-        return [Course.load_from_path(courses_path)]
-
-    # Default: Each dir in courses_path is a separate course
-    courses = []
-    log.info("Reading courses from {0}".format(courses_path))
-    for course_dir_name in os.listdir(courses_path):
-        course_path = courses_path / course_dir_name
-        if _is_course_path(course_path):
-            log.info("Initializing course {0}".format(course_path))
-            courses.append(Course.load_from_path(course_path))
-
-    return courses
-
-def create_lookup_table(courses):
-    return dict((c.id, c) for c in courses)
+    course = None
+    if course_required or course_id:
+        try:
+            course_loc = CourseDescriptor.id_to_location(course_id)
+            course = modulestore().get_item(course_loc)
+        except KeyError:
+            raise Http404("Course not found.")
+        
+        if course_must_be_open and not course.has_started():
+            raise Http404("This course has not yet started.")
+    
+    return course

lms/djangoapps/courseware/decorators.py

-from functools import wraps
-
-from django.http import Http404
-
-from xmodule.course_module import CourseDescriptor
-from xmodule.modulestore.django import modulestore
-
-
-def check_course(function=None, course_must_be_open=True, course_required=True):
-    """
-    This is a decorator for views that are within a course.
-    It converts the string passed to the view as 'course_id'
-    to a course, and then passes it to the view function as
-    a parameter named 'course'.
-    
-    This decorator also checks that the course has started.
-    If the course has not started, it raises a 404. This check
-    can be skipped by setting course_must_be_open to False.
-    
-    Usually, a 404 will be raised if a course is not found. This
-    behavior can be overrided by setting course_required to false.
-    When course_required is False, course_id is not required. If
-    course_id is still provided, but is None, course will be
-    set to None.
-    
-    Usage
-    This wrapper would be used on a function that has the following
-    entry in urls.py:
-    
-    url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/book$', 'staticbook.views.index'),
-     
-    Where staticbook.views.index has the following parameters:
-    
-    @check_course
-    def index(request, course):
-        # Notice that the parameter is course, not course_id
-    """
-    def inner_check_course(function):
-        @wraps(function)
-        def wrapped_function(*args, **kwargs):
-            if course_required or 'course_id' in kwargs:
-                course_id = kwargs['course_id']
-                course = None
-                
-                if course_required or course_id:
-                    try:
-                        course_loc = CourseDescriptor.id_to_location(course_id)
-                        course = modulestore().get_item(course_loc)
-                    except KeyError:
-                        raise Http404("Course not found.")
-                    
-                    if course_must_be_open and not course.has_started():
-                        raise Http404("This course has not yet started.")
-        
-                del kwargs['course_id']
-                kwargs['course'] = course
-        
-            return function(*args, **kwargs)
-        return wrapped_function
-        
-    if function is None:
-        return inner_check_course
-    else:
-        return inner_check_course(function)

lms/djangoapps/courseware/views.py

@@ -20,7 +20,7 @@ from multicourse import multicourse_settings
 from util.cache import cache
 from student.models import UserTestGroup
 from courseware import grades
-from courseware.decorators import check_course
+from courseware.courses import check_course
 from xmodule.modulestore.django import modulestore
 
 log = logging.getLogger("mitx.courseware")
@@ -57,11 +57,12 @@ def courses(request):
                'csrf': csrf_token}
     return render_to_response("courses.html", context)
 
-@check_course
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
-def gradebook(request, course):
+def gradebook(request, course_id):
     if 'course_admin' not in user_groups(request.user):
         raise Http404
+    course = check_course(course_id)
+    
     
     student_objects = User.objects.all()[:100]
     student_info = []
@@ -81,11 +82,11 @@ def gradebook(request, course):
 
 
 @login_required
-@check_course
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
-def profile(request, course, student_id=None):
+def profile(request, course_id, student_id=None):
     ''' User profile. Show username, location, etc, as well as grades .
         We need to allow the user to change some of these settings .'''
+    course = check_course(course_id)
 
     if student_id is None:
         student = request.user
@@ -139,9 +140,8 @@ def render_accordion(request, course, chapter, section):
 
 
 @ensure_csrf_cookie
-@check_course
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
-def index(request, course, chapter=None, section=None,
+def index(request, course_id, chapter=None, section=None,
           position=None):
     ''' Displays courseware accordion, and any associated content.
     If course, chapter, and section aren't all specified, just returns
@@ -160,6 +160,8 @@ def index(request, course, chapter=None, section=None,
 
      - HTTPresponse
     '''
+    course = check_course(course_id)
+    
     def clean(s):
         ''' Fixes URLs -- we convert spaces to _ in URLs to prevent
         funny encoding characters and keep the URLs readable.  This undoes
@@ -244,8 +246,7 @@ def jump_to(request, probname=None):
 
 
 @ensure_csrf_cookie
-@check_course
-def course_info(request, course):
-    csrf_token = csrf(request)['csrf_token']
+def course_info(request, course_id):
+    course = check_course(course_id)
 
-    return render_to_response('info.html', {'csrf': csrf_token, 'course': course})
+    return render_to_response('info.html', {'course': course})

lms/djangoapps/simplewiki/views.py

@@ -9,7 +9,7 @@ from django.utils import simplejson
 from django.utils.translation import ugettext_lazy as _
 from mitxmako.shortcuts import render_to_response
 
-from courseware.decorators import check_course
+from courseware.courses import check_course
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.django import modulestore
 
@@ -47,8 +47,8 @@ def update_template_dictionary(dictionary, request = None, course = None, articl
     if request:
         dictionary.update(csrf(request))
         
-@check_course(course_required=False)
-def view(request, article_path, course=None):
+def view(request, article_path, course_id=None):
+    course = check_course(course_id, course_required=False)
             
     (article, err) = get_article(request, article_path, course )
     if err:
@@ -62,9 +62,9 @@ def view(request, article_path, course=None):
     update_template_dictionary(d, request, course, article, article.current_revision)
     return render_to_response('simplewiki/simplewiki_view.html', d)
     
-@check_course(course_required=False)
-def view_revision(request, revision_number, article_path, course=None):
-
+def view_revision(request, revision_number, article_path, course_id=None):
+    course = check_course(course_id, course_required=False)
+    
     (article, err) = get_article(request, article_path, course )
     if err:
         return err
@@ -85,8 +85,9 @@ def view_revision(request, revision_number, article_path, course=None):
     
     return render_to_response('simplewiki/simplewiki_view.html', d)
 
-@check_course(course_required=False)
-def root_redirect(request, course=None):
+def root_redirect(request, course_id=None):
+    course = check_course(course_id, course_required=False)
+    
     #TODO: Add a default namespace to settings.
     namespace = course.wiki_namespace if course else "edX"
     
@@ -101,8 +102,9 @@ def root_redirect(request, course=None):
         err = not_found(request, namespace + '/', course)
         return err
 
-@check_course(course_required=False)
-def create(request, article_path, course=None):    
+def create(request, article_path, course_id=None):   
+    course = check_course(course_id, course_required=False)
+     
     article_path_components = article_path.split('/')
 
     # Ensure the namespace exists
@@ -160,8 +162,9 @@ def create(request, article_path, course=None):
 
     return render_to_response('simplewiki/simplewiki_edit.html', d)
 
-@check_course(course_required=False)
-def edit(request, article_path, course=None):
+def edit(request, article_path, course_id=None):
+    course = check_course(course_id, course_required=False)
+    
     (article, err) = get_article(request, article_path, course )
     if err:
         return err
@@ -206,8 +209,9 @@ def edit(request, article_path, course=None):
     update_template_dictionary(d, request, course, article)
     return render_to_response('simplewiki/simplewiki_edit.html', d)
 
-@check_course(course_required=False)
-def history(request, article_path, page=1, course=None):
+def history(request, article_path, page=1, course_id=None):
+    course = check_course(course_id, course_required=False)
+    
     (article, err) = get_article(request, article_path, course )
     if err:
         return err
@@ -287,8 +291,9 @@ def history(request, article_path, page=1, course=None):
     
     return render_to_response('simplewiki/simplewiki_history.html', d)
     
-@check_course(course_required=False)   
-def revision_feed(request, page=1, namespace=None, course=None):
+def revision_feed(request, page=1, namespace=None, course_id=None):
+    course = check_course(course_id, course_required=False)
+    
     page_size = 10
     
     if page == None:
@@ -318,8 +323,9 @@ def revision_feed(request, page=1, namespace=None, course=None):
     
     return render_to_response('simplewiki/simplewiki_revision_feed.html', d)
 
-@check_course(course_required=False)
-def search_articles(request, namespace=None, course = None):
+def search_articles(request, namespace=None, course_id = None):
+    course = check_course(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.
     # Adding some context to results (eg where matches were) would also be nice.
@@ -366,8 +372,9 @@ def search_articles(request, namespace=None, course = None):
         update_template_dictionary(d, request, course)
         return render_to_response('simplewiki/simplewiki_searchresults.html', d)
         
-@check_course(course_required=False)
-def search_add_related(request, course, slug, namespace):
+def search_add_related(request, course_id, slug, namespace):
+    course = check_course(course_id, course_required=False)
+    
     (article, err) = get_article(request, slug, namespace if namespace else course_id )
     if err:
         return err
@@ -397,8 +404,9 @@ def search_add_related(request, course, slug, namespace):
     json = simplejson.dumps({'results': results})
     return HttpResponse(json, mimetype='application/json')
 
-@check_course(course_required=False)
-def add_related(request, course, slug, namespace):
+def add_related(request, course_id, slug, namespace):
+    course = check_course(course_id, course_required=False)
+    
     (article, err) = get_article(request, slug, namespace if namespace else course_id )
     if err:
         return err
@@ -419,8 +427,9 @@ def add_related(request, course, slug, namespace):
     finally:
         return HttpResponseRedirect(reverse('wiki_view', args=(article.get_url(),)))
 
-@check_course(course_required=False)
-def remove_related(request, course, namespace, slug, related_id):
+def remove_related(request, course_id, namespace, slug, related_id):
+    course = check_course(course_id, course_required=False)
+    
     (article, err) = get_article(request, slug, namespace if namespace else course_id )
     
     if err:
@@ -440,8 +449,9 @@ def remove_related(request, course, namespace, slug, related_id):
     finally:
         return HttpResponseRedirect(reverse('wiki_view', args=(article.get_url(),)))
 
-@check_course(course_required=False)
-def random_article(request, course=None):
+def random_article(request, course_id=None):
+    course = check_course(course_id, course_required=False)
+    
     from random import randint
     num_arts = Article.objects.count()
     article = Article.objects.all()[randint(0, num_arts-1)]

lms/djangoapps/staticbook/views.py

 from django.contrib.auth.decorators import login_required
 from mitxmako.shortcuts import render_to_response
 
-from courseware.decorators import check_course
+from courseware.courses import check_course
 
 @login_required
-@check_course
-def index(request, course, page=0): 
+def index(request, course_id, page=0): 
+    course = check_course(course_id)
     return render_to_response('staticbook.html',{'page':int(page), 'course': course})
 
 def index_shifted(request, course_id, page):