From 6d6b3a59fe77ea47c71d84f9877f8570e9771971 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Apr 2013 15:27:36 -0400 Subject: [PATCH] violation fixes --- .../contentstore/course_info_model.py | 3 +- .../contentstore/module_info_model.py | 1 - cms/djangoapps/contentstore/utils.py | 12 +- cms/djangoapps/contentstore/views.py | 122 +++++++++--------- .../models/settings/course_details.py | 1 - .../models/settings/course_grading.py | 24 ++-- .../models/settings/course_metadata.py | 16 ++- cms/envs/aws.py | 2 +- cms/envs/common.py | 2 +- .../xmodule/xmodule/contentstore/content.py | 1 + 10 files changed, 89 insertions(+), 95 deletions(-) diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index 589db4ac56..ada3873992 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -97,8 +97,7 @@ def update_course_updates(location, update, passed_id=None): if (len(new_html_parsed) == 1): content = new_html_parsed[0].tail else: - content = "\n".join([html.tostring(ele) - for ele in new_html_parsed[1:]]) + content = "\n".join([html.tostring(ele) for ele in new_html_parsed[1:]]) return {"id": passed_id, "date": update['date'], diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index 8ea6add88d..91f722a699 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -1,7 +1,6 @@ from static_replace import replace_static_urls from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore import Location -from django.http import Http404 def get_module_info(store, location, parent_location=None, rewrite_static_links=False): diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index ec439b3312..a5a3b47bce 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1,4 +1,3 @@ -import logging from django.conf import settings from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore @@ -9,7 +8,7 @@ import copy DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info'] #In order to instantiate an open ended tab automatically, need to have this data -OPEN_ENDED_PANEL = {"name" : "Open Ended Panel", "type" : "open_ended"} +OPEN_ENDED_PANEL = {"name": "Open Ended Panel", "type": "open_ended"} def get_modulestore(location): @@ -87,11 +86,10 @@ def get_lms_link_for_item(location, preview=False, course_id=None): if settings.LMS_BASE is not None: if preview: - lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE', - 'preview.' + settings.LMS_BASE) + lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE', 'preview.' + settings.LMS_BASE) else: lms_base = settings.LMS_BASE - + lms_link = "//{lms_base}/courses/{course_id}/jump_to/{location}".format( lms_base=lms_base, course_id=course_id, @@ -193,6 +191,7 @@ class CoursePageNames: CourseOutline = "course_index" Checklists = "checklists" + def add_open_ended_panel_tab(course): """ Used to add the open ended panel tab to a course if it does not exist. @@ -209,6 +208,7 @@ def add_open_ended_panel_tab(course): changed = True return changed, course_tabs + def remove_open_ended_panel_tab(course): """ Used to remove the open ended panel tab from a course if it exists. @@ -221,6 +221,6 @@ def remove_open_ended_panel_tab(course): #Check to see if open ended panel is defined in the course if OPEN_ENDED_PANEL in course_tabs: #Add panel to the tabs if it is not defined - course_tabs = [ct for ct in course_tabs if ct!=OPEN_ENDED_PANEL] + course_tabs = [ct for ct in course_tabs if ct != OPEN_ENDED_PANEL] changed = True return changed, course_tabs diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 3169b437ed..9765937986 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -14,9 +14,6 @@ from tempfile import mkdtemp from django.core.servers.basehttp import FileWrapper from django.core.files.temp import NamedTemporaryFile -# to install PIL on MacOSX: 'easy_install http://dist.repoze.org/PIL-1.1.6.tar.gz' -from PIL import Image - from django.http import HttpResponse, Http404, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseServerError from django.http import HttpResponseNotFound from django.contrib.auth.decorators import login_required @@ -245,7 +242,7 @@ def edit_subsection(request, location): for field in item.fields if field.name not in ['display_name', 'start', 'due', 'format'] and - field.scope == Scope.settings + field.scope == Scope.settings ) can_view_live = False @@ -257,18 +254,18 @@ def edit_subsection(request, location): break return render_to_response('edit_subsection.html', - {'subsection': item, - 'context_course': course, - 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'), - 'lms_link': lms_link, - 'preview_link': preview_link, - 'course_graders': json.dumps(CourseGradingModel.fetch(course.location).graders), - 'parent_location': course.location, - 'parent_item': parent, - 'policy_metadata': policy_metadata, - 'subsection_units': subsection_units, - 'can_view_live': can_view_live - }) + {'subsection': item, + 'context_course': course, + 'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'), + 'lms_link': lms_link, + 'preview_link': preview_link, + 'course_graders': json.dumps(CourseGradingModel.fetch(course.location).graders), + 'parent_location': course.location, + 'parent_item': parent, + 'policy_metadata': policy_metadata, + 'subsection_units': subsection_units, + 'can_view_live': can_view_live + }) @login_required @@ -347,7 +344,7 @@ def edit_unit(request, location): index = index + 1 preview_lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE', - 'preview.' + settings.LMS_BASE) + 'preview.' + settings.LMS_BASE) preview_lms_link = '//{preview_lms_base}/courses/{org}/{course}/{course_name}/courseware/{section}/{subsection}/{index}'.format( preview_lms_base=preview_lms_base, @@ -623,7 +620,6 @@ def delete_item(request): store = get_modulestore(item_loc) - # @TODO: this probably leaves draft items dangling. My preferance would be for the semantic to be # if item.location.revision=None, then delete both draft and published version # if caller wants to only delete the draft than the caller should put item.location.revision='draft' @@ -665,7 +661,7 @@ def save_item(request): if not has_access(request.user, item_location): raise PermissionDenied() - store = get_modulestore(Location(item_location)); + store = get_modulestore(Location(item_location)) if request.POST.get('data') is not None: data = request.POST['data'] @@ -800,7 +796,7 @@ def upload_asset(request, org, course, coursename): # Does the course actually exist?!? Get anything from it to prove its existance try: - item = modulestore().get_item(location) + modulestore().get_item(location) except: # no return it as a Bad Request response logging.error('Could not find course' + location) @@ -834,24 +830,22 @@ def upload_asset(request, org, course, coursename): readback = contentstore().find(content.location) response_payload = {'displayname': content.name, - 'uploadDate': get_default_time_display(readback.last_modified_at.timetuple()), - 'url': StaticContent.get_url_path_from_location(content.location), - 'thumb_url': StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None, - 'msg': 'Upload completed' - } + 'uploadDate': get_default_time_display(readback.last_modified_at.timetuple()), + 'url': StaticContent.get_url_path_from_location(content.location), + 'thumb_url': StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None, + 'msg': 'Upload completed' + } response = HttpResponse(json.dumps(response_payload)) response['asset_url'] = StaticContent.get_url_path_from_location(content.location) return response - -''' -This view will return all CMS users who are editors for the specified course -''' @login_required @ensure_csrf_cookie def manage_users(request, location): - + ''' + This view will return all CMS users who are editors for the specified course + ''' # check that logged in user has permissions to this item if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME) and not has_access(request.user, location, role=STAFF_ROLE_NAME): raise PermissionDenied() @@ -878,14 +872,14 @@ def create_json_response(errmsg=None): return resp -''' -This POST-back view will add a user - specified by email - to the list of editors for -the specified course -''' @expect_json @login_required @ensure_csrf_cookie def add_user(request, location): + ''' + This POST-back view will add a user - specified by email - to the list of editors for + the specified course + ''' email = request.POST["email"] if email == '': @@ -911,14 +905,15 @@ def add_user(request, location): return create_json_response() -''' -This POST-back view will remove a user - specified by email - from the list of editors for -the specified course -''' @expect_json @login_required @ensure_csrf_cookie def remove_user(request, location): + ''' + This POST-back view will remove a user - specified by email - from the list of editors for + the specified course + ''' + email = request.POST["email"] # check that logged in user has admin permissions on this course @@ -993,13 +988,12 @@ def reorder_static_tabs(request): for tab in course.tabs: if tab['type'] == 'static_tab': reordered_tabs.append({'type': 'static_tab', - 'name': tab_items[static_tab_idx].display_name, - 'url_slug': tab_items[static_tab_idx].location.name}) + 'name': tab_items[static_tab_idx].display_name, + 'url_slug': tab_items[static_tab_idx].location.name}) static_tab_idx += 1 else: reordered_tabs.append(tab) - # OK, re-assemble the static tabs in the new order course.tabs = reordered_tabs modulestore('direct').update_metadata(course.location, own_metadata(course)) @@ -1011,7 +1005,6 @@ def reorder_static_tabs(request): def edit_tabs(request, org, course, coursename): location = ['i4x', org, course, 'course', coursename] course_item = modulestore().get_item(location) - static_tabs_loc = Location('i4x', org, course, 'static_tab', None) # check that logged in user has permissions to this item if not has_access(request.user, location): @@ -1040,7 +1033,7 @@ def edit_tabs(request, org, course, coursename): 'active_tab': 'pages', 'context_course': course_item, 'components': components - }) + }) def not_found(request): @@ -1102,21 +1095,21 @@ def course_info_updates(request, org, course, provided_id=None): if request.method == 'GET': return HttpResponse(json.dumps(get_course_updates(location)), - mimetype="application/json") + mimetype="application/json") elif real_method == 'DELETE': try: return HttpResponse(json.dumps(delete_course_update(location, - request.POST, provided_id)), mimetype="application/json") + request.POST, provided_id)), mimetype="application/json") except: return HttpResponseBadRequest("Failed to delete", - content_type="text/plain") + content_type="text/plain") elif request.method == 'POST': try: return HttpResponse(json.dumps(update_course_updates(location, - request.POST, provided_id)), mimetype="application/json") + request.POST, provided_id)), mimetype="application/json") except: return HttpResponseBadRequest("Failed to save", - content_type="text/plain") + content_type="text/plain") @expect_json @@ -1184,7 +1177,7 @@ def course_config_graders_page(request, org, course, name): return render_to_response('settings_graders.html', { 'context_course': course_module, - 'course_location' : location, + 'course_location': location, 'course_details': json.dumps(course_details, cls=CourseSettingsEncoder) }) @@ -1203,8 +1196,8 @@ def course_config_advanced_page(request, org, course, name): return render_to_response('settings_advanced.html', { 'context_course': course_module, - 'course_location' : location, - 'advanced_dict' : json.dumps(CourseMetadata.fetch(location)), + 'course_location': location, + 'advanced_dict': json.dumps(CourseMetadata.fetch(location)), }) @@ -1225,7 +1218,8 @@ def course_settings_updates(request, org, course, name, section): manager = CourseDetails elif section == 'grading': manager = CourseGradingModel - else: return + else: + return if request.method == 'GET': # Cannot just do a get w/o knowing the course name :-( @@ -1320,6 +1314,7 @@ def course_advanced_updates(request, org, course, name): response_json = json.dumps(CourseMetadata.update_from_json(location, request_body, filter_tabs=filter_tabs)) return HttpResponse(response_json, mimetype="application/json") + @ensure_csrf_cookie @login_required def get_checklists(request, org, course, name): @@ -1345,10 +1340,10 @@ def get_checklists(request, org, course, name): if copied or modified: modulestore.update_metadata(location, own_metadata(course_module)) return render_to_response('checklists.html', - { - 'context_course': course_module, - 'checklists': checklists - }) + { + 'context_course': course_module, + 'checklists': checklists + }) @ensure_csrf_cookie @@ -1433,7 +1428,6 @@ def asset_index(request, org, course, name): # sort in reverse upload date order assets = sorted(assets, key=lambda asset: asset['uploadDate'], reverse=True) - thumbnails = contentstore().get_all_content_thumbnails_for_course(course_reference) asset_display = [] for asset in assets: id = asset['_id'] @@ -1527,10 +1521,10 @@ def initialize_course_tabs(course): # This logic is repeated in xmodule/modulestore/tests/factories.py # so if you change anything here, you need to also change it there. course.tabs = [{"type": "courseware"}, - {"type": "course_info", "name": "Course Info"}, - {"type": "discussion", "name": "Discussion"}, - {"type": "wiki", "name": "Wiki"}, - {"type": "progress", "name": "Progress"}] + {"type": "course_info", "name": "Course Info"}, + {"type": "discussion", "name": "Discussion"}, + {"type": "wiki", "name": "Wiki"}, + {"type": "progress", "name": "Progress"}] modulestore('direct').update_metadata(course.location.url(), own_metadata(course)) @@ -1586,8 +1580,10 @@ def import_course(request, org, course, name): shutil.move(r / fname, course_dir) module_store, course_items = import_from_xml(modulestore('direct'), settings.GITHUB_REPO_ROOT, - [course_subdir], load_error_modules=False, static_content_store=contentstore(), - target_location_namespace=Location(location), draft_store=modulestore()) + [course_subdir], load_error_modules=False, + static_content_store=contentstore(), + target_location_namespace=Location(location), + draft_store=modulestore()) # we can blow this away when we're done importing. shutil.rmtree(course_dir) diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 876000c7fe..0dbb47b31b 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -174,7 +174,6 @@ class CourseDetails(object): return result - # TODO move to a more general util? Is there a better way to do the isinstance model check? class CourseSettingsEncoder(json.JSONEncoder): def default(self, obj): diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index ee9b4ac0eb..4ea9f2f5db 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -45,14 +45,13 @@ class CourseGradingModel(object): # return empty model else: - return { - "id": index, + return {"id": index, "type": "", "min_count": 0, "drop_count": 0, "short_label": None, "weight": 0 - } + } @staticmethod def fetch_cutoffs(course_location): @@ -95,7 +94,6 @@ class CourseGradingModel(object): return CourseGradingModel.fetch(course_location) - @staticmethod def update_grader_from_json(course_location, grader): """ @@ -137,7 +135,6 @@ class CourseGradingModel(object): return cutoffs - @staticmethod def update_grace_period_from_json(course_location, graceperiodjson): """ @@ -210,8 +207,7 @@ class CourseGradingModel(object): location = Location(location) descriptor = get_modulestore(location).get_item(location) - return { - "graderType": descriptor.lms.format if descriptor.lms.format is not None else 'Not Graded', + return {"graderType": descriptor.lms.format if descriptor.lms.format is not None else 'Not Graded', "location": location, "id": 99 # just an arbitrary value to } @@ -231,7 +227,6 @@ class CourseGradingModel(object): get_modulestore(location).update_metadata(location, descriptor._model_data._kvs._metadata) - @staticmethod def convert_set_grace_period(descriptor): # 5 hours 59 minutes 59 seconds => converted to iso format @@ -262,13 +257,12 @@ class CourseGradingModel(object): @staticmethod def parse_grader(json_grader): # manual to clear out kruft - result = { - "type": json_grader["type"], - "min_count": int(json_grader.get('min_count', 0)), - "drop_count": int(json_grader.get('drop_count', 0)), - "short_label": json_grader.get('short_label', None), - "weight": float(json_grader.get('weight', 0)) / 100.0 - } + result = {"type": json_grader["type"], + "min_count": int(json_grader.get('min_count', 0)), + "drop_count": int(json_grader.get('drop_count', 0)), + "short_label": json_grader.get('short_label', None), + "weight": float(json_grader.get('weight', 0)) / 100.0 + } return result diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 70f69315ff..4429e35692 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -6,6 +6,7 @@ from xblock.core import Scope from xmodule.course_module import CourseDescriptor import copy + class CourseMetadata(object): ''' For CRUD operations on metadata fields which do not have specific editors @@ -13,8 +14,13 @@ class CourseMetadata(object): The objects have no predefined attrs but instead are obj encodings of the editable metadata. ''' - FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', - 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod', 'checklists'] + FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', + 'end', + 'enrollment_start', + 'enrollment_end', + 'tabs', + 'graceperiod', + 'checklists'] @classmethod def fetch(cls, course_location): @@ -48,7 +54,7 @@ class CourseMetadata(object): descriptor = get_modulestore(course_location).get_item(course_location) dirty = False - + #Copy the filtered list to avoid permanently changing the class attribute filtered_list = copy.copy(cls.FILTERED_LIST) #Don't filter on the tab attribute if filter_tabs is False @@ -71,7 +77,7 @@ class CourseMetadata(object): if dirty: get_modulestore(course_location).update_metadata(course_location, - own_metadata(descriptor)) + own_metadata(descriptor)) # Could just generate and return a course obj w/o doing any db reads, # but I put the reads in as a means to confirm it persisted correctly @@ -92,6 +98,6 @@ class CourseMetadata(object): delattr(descriptor.lms, key) get_modulestore(course_location).update_metadata(course_location, - own_metadata(descriptor)) + own_metadata(descriptor)) return cls.fetch(course_location) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index edf67badfe..59ad8b835e 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -67,4 +67,4 @@ MODULESTORE = AUTH_TOKENS['MODULESTORE'] CONTENTSTORE = AUTH_TOKENS['CONTENTSTORE'] # Datadog for events! -DATADOG_API = AUTH_TOKENS.get("DATADOG_API") \ No newline at end of file +DATADOG_API = AUTH_TOKENS.get("DATADOG_API") diff --git a/cms/envs/common.py b/cms/envs/common.py index 37cfeea7a1..ca08a56a70 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -167,7 +167,7 @@ STATICFILES_DIRS = [ PROJECT_ROOT / "static", # This is how you would use the textbook images locally -# ("book", ENV_ROOT / "book_images") +# ("book", ENV_ROOT / "book_images") ] # Locale/Internationalization diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 9dc4b1367b..428d38a50d 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -9,6 +9,7 @@ import StringIO from xmodule.modulestore import Location from .django import contentstore +# to install PIL on MacOSX: 'easy_install http://dist.repoze.org/PIL-1.1.6.tar.gz' from PIL import Image