From ecc50314f771c22ba4bd3c96f4c8cf70e816d98b Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Fri, 13 Jul 2012 10:31:36 -0400 Subject: [PATCH] Added a decorator that prevents access to course pages until the course has started. --- common/lib/xmodule/xmodule/course_module.py | 11 ++- lms/djangoapps/courseware/decorators.py | 50 +++++++++++++ lms/djangoapps/courseware/views.py | 38 +++------- lms/djangoapps/simplewiki/views.py | 79 +++++++++------------ lms/djangoapps/staticbook/views.py | 13 ++-- lms/djangoapps/student/views.py | 11 +-- lms/envs/devplus.py | 23 ++++++ lms/urls.py | 13 ++-- 8 files changed, 145 insertions(+), 93 deletions(-) create mode 100644 lms/djangoapps/courseware/decorators.py diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 08a9553763..e2ee21a07f 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, date import dateutil.parser from fs.errors import ResourceNotFoundError import logging @@ -18,7 +18,14 @@ class CourseDescriptor(SequenceDescriptor): def __init__(self, system, definition=None, **kwargs): super(CourseDescriptor, self).__init__(system, definition, **kwargs) - self.start = dateutil.parser.parse(self.metadata["start"]) + try: + self.start = dateutil.parser.parse(self.metadata["start"]) + except KeyError: + self.start = date.fromtimestamp(0) #The epoch + log.critical("Course loaded without a start date. " + str(self.id)) + except ValueError, e: + self.start = date.fromtimestamp(0) #The epoch + log.critical("Course loaded with a bad start date. " + str(self.id) + " '" + str(e) + "'") def has_started(self): return datetime.now() > self.start diff --git a/lms/djangoapps/courseware/decorators.py b/lms/djangoapps/courseware/decorators.py new file mode 100644 index 0000000000..ec7654a050 --- /dev/null +++ b/lms/djangoapps/courseware/decorators.py @@ -0,0 +1,50 @@ +from django.http import Http404 + +from xmodule.course_module import CourseDescriptor +from xmodule.modulestore.django import modulestore + +def check_course(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. + + If course_required is False, course_id is not required. If + course_id is still provided, but is None, course will be + set to None. + """ + def inner_check_course(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 + + del kwargs['course_id'] + kwargs['course'] = course + + return function(*args, **kwargs) + return wrapped_function + + # If no arguments were passed to the decorator, the function itself + # will be in course_must_be_open + if hasattr(course_must_be_open, '__call__'): + function = course_must_be_open + course_must_be_open = True + return inner_check_course(function) + else: + return inner_check_course diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index d2516a7361..f2a40c1c11 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -16,37 +16,17 @@ from module_render import toc_for_course, get_module, get_section from models import StudentModuleCache from student.models import UserProfile from multicourse import multicourse_settings -from xmodule.modulestore.django import modulestore -from xmodule.course_module import CourseDescriptor -from util.cache import cache +#from util.cache import cache #TODO: Where did this go? lib/util/cache no longer exists from student.models import UserTestGroup from courseware import grades +from courseware.decorators import check_course +from xmodule.modulestore.django import modulestore log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} -def check_course(function, course_must_be_open=True): - def inner_function(*args, **kwargs): - course_id = kwargs['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 - - del kwargs['course_id'] - kwargs['course'] = course - - return function(*args, **kwargs) - return inner_function - - def user_groups(user): if not user.is_authenticated(): return [] @@ -77,20 +57,18 @@ 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_id): +def gradebook(request, course): if 'course_admin' not in user_groups(request.user): raise Http404 - - course_location = CourseDescriptor.id_to_location(course_id) - + student_objects = User.objects.all()[:100] student_info = [] for student in student_objects: - student_module_cache = StudentModuleCache(student, modulestore().get_item(course_location)) - course, _, _, _ = get_module(request.user, request, course_location, student_module_cache) + student_module_cache = StudentModuleCache(student, course) + course, _, _, _ = get_module(request.user, request, course.location, student_module_cache) student_info.append({ 'username': student.username, 'id': student.id, diff --git a/lms/djangoapps/simplewiki/views.py b/lms/djangoapps/simplewiki/views.py index 81ce8abf76..f0f2950fa3 100644 --- a/lms/djangoapps/simplewiki/views.py +++ b/lms/djangoapps/simplewiki/views.py @@ -9,20 +9,12 @@ 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 xmodule.course_module import CourseDescriptor from xmodule.modulestore.django import modulestore from models import Revision, Article, Namespace, CreateArticleForm, RevisionFormWithTitle, RevisionForm import wiki_settings - -def get_course(course_id): - if course_id == None: - return None - - course_loc = CourseDescriptor.id_to_location(course_id) - course = modulestore().get_item(course_loc) - # raise Http404("Course not found") - return course def wiki_reverse(wiki_page, article = None, course = None, namespace=None, args=[], kwargs={}): kwargs = dict(kwargs) # TODO: Figure out why if I don't do this kwargs sometimes contains {'article_path'} @@ -55,10 +47,9 @@ def update_template_dictionary(dictionary, request = None, course = None, articl if request: dictionary.update(csrf(request)) - -def view(request, article_path, course_id=None): - course = get_course(course_id) - +@check_course(course_required=False) +def view(request, article_path, course=None): + (article, err) = get_article(request, article_path, course ) if err: return err @@ -71,8 +62,9 @@ def view(request, article_path, course_id=None): update_template_dictionary(d, request, course, article, article.current_revision) return render_to_response('simplewiki/simplewiki_view.html', d) -def view_revision(request, revision_number, article_path, course_id=None): - course = get_course(course_id) +@check_course(course_required=False) +def view_revision(request, revision_number, article_path, course=None): + (article, err) = get_article(request, article_path, course ) if err: return err @@ -93,24 +85,24 @@ def view_revision(request, revision_number, article_path, course_id=None): return render_to_response('simplewiki/simplewiki_view.html', d) - -def root_redirect(request, course_id=None): - course = get_course(course_id) +@check_course(course_required=False) +def root_redirect(request, course=None): + #TODO: Add a default namespace to settings. + namespace = course.wiki_namespace if course else "edX" + try: - root = Article.get_root(course.wiki_namespace) + root = Article.get_root(namespace) + return HttpResponseRedirect(reverse('wiki_view', kwargs={'course_id' : course_id, 'article_path' : root.get_path()} )) except: # If the root is not found, we probably are loading this class for the first time # We should make sure the namespace exists so the root article can be created. - Namespace.ensure_namespace(course.wiki_namespace) + Namespace.ensure_namespace(namespace) - err = not_found(request, course.wiki_namespace + '/', course) + err = not_found(request, namespace + '/', course) return err - - return HttpResponseRedirect(reverse('wiki_view', kwargs={'course_id' : course_id, 'article_path' : root.get_path()} )) -def create(request, article_path, course_id=None): - course = get_course(course_id) - +@check_course(course_required=False) +def create(request, article_path, course=None): article_path_components = article_path.split('/') # Ensure the namespace exists @@ -168,8 +160,8 @@ def create(request, article_path, course_id=None): return render_to_response('simplewiki/simplewiki_edit.html', d) -def edit(request, article_path, course_id=None): - course = get_course(course_id) +@check_course(course_required=False) +def edit(request, article_path, course=None): (article, err) = get_article(request, article_path, course ) if err: return err @@ -214,8 +206,8 @@ def edit(request, article_path, course_id=None): update_template_dictionary(d, request, course, article) return render_to_response('simplewiki/simplewiki_edit.html', d) -def history(request, article_path, page=1, course_id=None): - course = get_course(course_id) +@check_course(course_required=False) +def history(request, article_path, page=1, course=None): (article, err) = get_article(request, article_path, course ) if err: return err @@ -295,10 +287,8 @@ def history(request, article_path, page=1, course_id=None): return render_to_response('simplewiki/simplewiki_history.html', d) - -def revision_feed(request, page=1, namespace=None, course_id=None): - course = get_course(course_id) - +@check_course(course_required=False) +def revision_feed(request, page=1, namespace=None, course=None): page_size = 10 if page == None: @@ -328,7 +318,8 @@ def revision_feed(request, page=1, namespace=None, course_id=None): return render_to_response('simplewiki/simplewiki_revision_feed.html', d) -def search_articles(request, namespace=None, course_id = None): +@check_course(course_required=False) +def search_articles(request, namespace=None, course = None): # 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. @@ -339,9 +330,7 @@ def search_articles(request, namespace=None, course_id = None): querystring = request.GET.get('value', '').strip() else: querystring = "" - - course = get_course(course_id) - + results = Article.objects.all() if namespace: results = results.filter(namespace__name__exact = namespace) @@ -377,8 +366,8 @@ def search_articles(request, namespace=None, course_id = None): update_template_dictionary(d, request, course) return render_to_response('simplewiki/simplewiki_searchresults.html', d) - -def search_add_related(request, course_id, slug, namespace): +@check_course(course_required=False) +def search_add_related(request, course, slug, namespace): (article, err) = get_article(request, slug, namespace if namespace else course_id ) if err: return err @@ -408,7 +397,8 @@ def search_add_related(request, course_id, slug, namespace): json = simplejson.dumps({'results': results}) return HttpResponse(json, mimetype='application/json') -def add_related(request, course_id, slug, namespace): +@check_course(course_required=False) +def add_related(request, course, slug, namespace): (article, err) = get_article(request, slug, namespace if namespace else course_id ) if err: return err @@ -429,7 +419,8 @@ def add_related(request, course_id, slug, namespace): finally: return HttpResponseRedirect(reverse('wiki_view', args=(article.get_url(),))) -def remove_related(request, course_id, namespace, slug, related_id): +@check_course(course_required=False) +def remove_related(request, course, namespace, slug, related_id): (article, err) = get_article(request, slug, namespace if namespace else course_id ) if err: @@ -449,8 +440,8 @@ def remove_related(request, course_id, namespace, slug, related_id): finally: return HttpResponseRedirect(reverse('wiki_view', args=(article.get_url(),))) -def random_article(request, course_id): - course = get_course(course_id) +@check_course(course_required=False) +def random_article(request, course=None): from random import randint num_arts = Article.objects.count() article = Article.objects.all()[randint(0, num_arts-1)] diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index 3bc62c2dbf..bf0c3dc015 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -1,13 +1,12 @@ from django.contrib.auth.decorators import login_required from mitxmako.shortcuts import render_to_response -from xmodule.modulestore.django import modulestore -from xmodule.course_module import CourseDescriptor + +from courseware.decorators import check_course @login_required -def index(request, course_id=None, page=0): - course_location = CourseDescriptor.id_to_location(course_id) - course = modulestore().get_item(course_location) +@check_course +def index(request, course, page=0): return render_to_response('staticbook.html',{'page':int(page), 'course': course}) -def index_shifted(request, page): - return index(request, int(page)+24) +def index_shifted(request, course_id, page): + return index(request, course_id=course_id, page=int(page)+24) diff --git a/lms/djangoapps/student/views.py b/lms/djangoapps/student/views.py index ae64549f30..ae5dde4a6c 100644 --- a/lms/djangoapps/student/views.py +++ b/lms/djangoapps/student/views.py @@ -20,6 +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 xmodule.course_module import CourseDescriptor from xmodule.modulestore.django import modulestore from django_future.csrf import ensure_csrf_cookie @@ -495,11 +496,10 @@ def accept_name_change(request): @ensure_csrf_cookie -def course_info(request, course_id): +@check_course(course_must_be_open=False) +def course_info(request, course): # This is the advertising page for a student to look at the course before signing up csrf_token = csrf(request)['csrf_token'] - course_loc = CourseDescriptor.id_to_location(course_id) - course = modulestore().get_item(course_loc) # TODO: Couse should be a model return render_to_response('portal/course_about.html', {'csrf': csrf_token, 'course': course}) @@ -520,10 +520,11 @@ def help(request): @login_required +@check_course(course_must_be_open=False) @ensure_csrf_cookie -def enroll(request, course_id): +def enroll(request, course): user = request.user enrollment = CourseEnrollment(user=user, - course_id=course_id) + course_id=course.id) enrollment.save() return redirect(reverse('dashboard')) diff --git a/lms/envs/devplus.py b/lms/envs/devplus.py index 32e0a7beb7..31c61b36db 100644 --- a/lms/envs/devplus.py +++ b/lms/envs/devplus.py @@ -42,3 +42,26 @@ CACHES = { } SESSION_ENGINE = 'django.contrib.sessions.backends.cache' + + +################################ DEBUG TOOLBAR ################################# +INSTALLED_APPS += ('debug_toolbar',) +MIDDLEWARE_CLASSES += ('debug_toolbar.middleware.DebugToolbarMiddleware',) +INTERNAL_IPS = ('127.0.0.1',) + +DEBUG_TOOLBAR_PANELS = ( + 'debug_toolbar.panels.version.VersionDebugPanel', + 'debug_toolbar.panels.timer.TimerDebugPanel', + 'debug_toolbar.panels.settings_vars.SettingsVarsDebugPanel', + 'debug_toolbar.panels.headers.HeaderDebugPanel', + 'debug_toolbar.panels.request_vars.RequestVarsDebugPanel', + 'debug_toolbar.panels.sql.SQLDebugPanel', + 'debug_toolbar.panels.signals.SignalDebugPanel', + 'debug_toolbar.panels.logger.LoggingPanel', + +# Enabling the profiler has a weird bug as of django-debug-toolbar==0.9.4 and +# Django=1.3.1/1.4 where requests to views get duplicated (your method gets +# hit twice). So you can uncomment when you need to diagnose performance +# problems, but you shouldn't leave it on. +# 'debug_toolbar.panels.profiling.ProfilingDebugPanel', +) diff --git a/lms/urls.py b/lms/urls.py index 328efacc13..bea57e8e01 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -57,9 +57,6 @@ if settings.COURSEWARE_ENABLED: url(r'^modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'), url(r'^change_setting$', 'student.views.change_setting'), url(r'^s/(?P