From 80619da4290ec933b10cc1a8eb7d0068169d8950 Mon Sep 17 00:00:00 2001 From: Julian Arni Date: Thu, 15 Aug 2013 10:21:32 -0400 Subject: [PATCH] Review fixes --- .../contentstore/views/component.py | 35 ++++++----- cms/djangoapps/contentstore/views/course.py | 60 +++++++++---------- 2 files changed, 47 insertions(+), 48 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 292bc841ff..a5fec7c033 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -2,14 +2,15 @@ import json import logging from collections import defaultdict -from django.http import HttpResponse, HttpResponseBadRequest, \ - HttpResponseForbidden +from django.http import ( HttpResponse, HttpResponseBadRequest, + HttpResponseForbidden ) from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_http_methods from django.core.exceptions import PermissionDenied from django_future.csrf import ensure_csrf_cookie from django.conf import settings -from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError +from xmodule.modulestore.exceptions import ( ItemNotFoundError, + InvalidLocationError ) from mitxmako.shortcuts import render_to_response from xmodule.modulestore import Location @@ -20,8 +21,8 @@ from xblock.core import Scope from util.json_request import expect_json, JsonResponse from contentstore.module_info_model import get_module_info, set_module_info -from contentstore.utils import get_modulestore, get_lms_link_for_item, \ - compute_unit_state, UnitState, get_course_for_item +from contentstore.utils import ( get_modulestore, get_lms_link_for_item, + compute_unit_state, UnitState, get_course_for_item ) from models.settings.course_grading import CourseGradingModel @@ -90,7 +91,8 @@ def edit_subsection(request, location): # we're for now assuming a single parent if len(parent_locs) != 1: logging.error( - 'Multiple (or none) parents have been found for' + location + 'Multiple (or none) parents have been found for %', + location ) # this should blow up if we don't find any parents, which would be erroneous @@ -209,7 +211,8 @@ def edit_unit(request, location): pass else: log.error( - "Improper format for course advanced keys!" + course_advanced_keys + "Improper format for course advanced keys! %", + course_advanced_keys ) components = [ @@ -293,11 +296,12 @@ def assignment_type_update(request, org, course, category, name): return HttpResponseForbidden() if request.method == 'GET': - return JsonResponse(CourseGradingModel.get_section_grader_type(location)) + rsp = CourseGradingModel.get_section_grader_type(location) elif request.method in ('POST', 'PUT'): # post or put, doesn't matter. - return JsonResponse(CourseGradingModel.update_section_grader_type( + rsp = CourseGradingModel.update_section_grader_type( location, request.POST - )) + ) + return JsonResponse(rsp) @login_required @@ -368,7 +372,7 @@ def module_info(request, module_location): rewrite_static_links = request.GET.get('rewrite_url_links', 'True') in ['True', 'true'] logging.debug('rewrite_static_links = {0} {1}'.format( - request.GET.get('rewrite_url_links', 'False'), + request.GET.get('rewrite_url_links', False), rewrite_static_links) ) @@ -377,13 +381,14 @@ def module_info(request, module_location): raise PermissionDenied() if request.method == 'GET': - return JsonResponse(get_module_info( + rsp = get_module_info( get_modulestore(location), location, rewrite_static_links=rewrite_static_links - )) + ) elif request.method in ("POST", "PUT"): - return JsonResponse(set_module_info( + rsp = set_module_info( get_modulestore(location), location, request.POST - )) + ) + return JsonResponse(rsp) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index b47f8e9ffb..753df66fe0 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -123,14 +123,14 @@ def create_new_course(request): pass if existing_course is not None: return JsonResponse({ - 'ErrMsg': _(('There is already a course defined with the same ' + 'ErrMsg': _('There is already a course defined with the same ' 'organization, course number, and course run. Please ' 'change either organization or course number to be ' - 'unique.')), - 'OrgErrMsg': _(('Please change either the organization or ' - 'course number so that it is unique.')), - 'CourseErrMsg': _(('Please change either the organization or ' - 'course number so that it is unique.')), + 'unique.'), + 'OrgErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), + 'CourseErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), }) course_search_location = ['i4x', dest_location.org, dest_location.course, @@ -139,13 +139,13 @@ def create_new_course(request): courses = modulestore().get_items(course_search_location) if len(courses) > 0: return JsonResponse({ - 'ErrMsg': _(('There is already a course defined with the same ' + 'ErrMsg': _('There is already a course defined with the same ' 'organization and course number. Please ' - 'change at least one field to be unique.')), - 'OrgErrMsg': _(('Please change either the organization or ' - 'course number so that it is unique.')), - 'CourseErrMsg': _(('Please change either the organization or ' - 'course number so that it is unique.')), + 'change at least one field to be unique.'), + 'OrgErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), + 'CourseErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), }) # instantiate the CourseDescriptor and then persist it @@ -206,9 +206,7 @@ def course_info(request, org, course, name, provided_id=None): 'context_course': course_module, 'url_base': "/" + org + "/" + course + "/", 'course_updates': json.dumps(get_course_updates(location)), - 'handouts_location': Location(['i4x', org, course, 'course_info', - 'handouts']).url() - }) + 'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url() }) @expect_json @@ -227,9 +225,6 @@ def course_info_updates(request, org, course, provided_id=None): # get current updates location = ['i4x', org, course, 'course_info', "updates"] - # Hmmm, provided_id is coming as empty string on create whereas I believe - # it used to be None :-( Possibly due to my removing the seemingly - # redundant pattern in urls.py if provided_id == '': provided_id = None @@ -241,22 +236,21 @@ def course_info_updates(request, org, course, provided_id=None): return JsonResponse(get_course_updates(location)) elif request.method == 'DELETE': try: - return JsonResponse(delete_course_update(location, request.POST, - provided_id - )) + return JsonResponse(delete_course_update(location, request.POST, provided_id)) except: - return HttpResponseBadRequest("Failed to delete", - content_type="text/plain") - elif request.method in ('POST', 'PUT'): # can be either and sometimes - # django is rewriting one to the - # other + return HttpResponseBadRequest( + "Failed to delete", + content_type="text/plain" + ) + # can be either and sometimes django is rewriting one to the other: + elif request.method in ('POST', 'PUT'): try: - return JsonResponse(update_course_updates(location, request.POST, - provided_id - )) + return JsonResponse(update_course_updates(location, request.POST, provided_id)) except: - return HttpResponseBadRequest("Failed to save", - content_type="text/plain") + return HttpResponseBadRequest( + "Failed to save", + content_type="text/plain" + ) @login_required @@ -563,8 +557,8 @@ def textbook_index(request, org, course, name): if request.is_ajax(): if request.method == 'GET': return JsonResponse(course_module.pdf_textbooks) - elif request.method in ('POST', 'PUT'): # can be either and sometimes - # django is rewriting one to the other + # can be either and sometimes django is rewriting one to the other: + elif request.method in ('POST', 'PUT'): try: textbooks = validate_textbooks_json(request.body) except TextbookValidationError as err: