diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 6e3027678a..30f312a70f 100644 --- a/common/djangoapps/student/views.py +++ b/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) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 858cdb1c87..b6853ae12a 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -1,62 +1,33 @@ -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 diff --git a/lms/djangoapps/courseware/decorators.py b/lms/djangoapps/courseware/decorators.py deleted file mode 100644 index 0c57b330df..0000000000 --- a/lms/djangoapps/courseware/decorators.py +++ /dev/null @@ -1,64 +0,0 @@ -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[^/]+/[^/]+/[^/]+)/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) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index d691ee3b09..f2e7edadef 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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}) diff --git a/lms/djangoapps/simplewiki/views.py b/lms/djangoapps/simplewiki/views.py index f0f2950fa3..a0f1463fe5 100644 --- a/lms/djangoapps/simplewiki/views.py +++ b/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)] diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index bf0c3dc015..b7633dd1d3 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -1,11 +1,11 @@ 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):