From 46cd9b0ad99fa2bd509d4dae4d453c01ed04ea3a Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 15 Sep 2014 11:24:55 -0400 Subject: [PATCH] Add bulk_operations wrapper around all course-query-intensive Studio views. --- .../contentstore/views/component.py | 123 ++-- cms/djangoapps/contentstore/views/course.py | 566 +++++++++--------- cms/djangoapps/contentstore/views/item.py | 231 +++---- 3 files changed, 468 insertions(+), 452 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 3ffe8b135f..acac8ddb82 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -141,74 +141,75 @@ def container_handler(request, usage_key_string): if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): usage_key = UsageKey.from_string(usage_key_string) - try: - course, xblock, lms_link = _get_item_in_course(request, usage_key) - except ItemNotFoundError: - return HttpResponseBadRequest() + with modulestore().bulk_operations(usage_key.course_key): + try: + course, xblock, lms_link = _get_item_in_course(request, usage_key) + except ItemNotFoundError: + return HttpResponseBadRequest() - component_templates = get_component_templates(course) - ancestor_xblocks = [] - parent = get_parent_xblock(xblock) - action = request.REQUEST.get('action', 'view') + component_templates = get_component_templates(course) + ancestor_xblocks = [] + parent = get_parent_xblock(xblock) + action = request.REQUEST.get('action', 'view') - is_unit_page = is_unit(xblock) - unit = xblock if is_unit_page else None + is_unit_page = is_unit(xblock) + unit = xblock if is_unit_page else None - while parent and parent.category != 'course': - if unit is None and is_unit(parent): - unit = parent - ancestor_xblocks.append(parent) - parent = get_parent_xblock(parent) - ancestor_xblocks.reverse() + while parent and parent.category != 'course': + if unit is None and is_unit(parent): + unit = parent + ancestor_xblocks.append(parent) + parent = get_parent_xblock(parent) + ancestor_xblocks.reverse() - assert unit is not None, "Could not determine unit page" - subsection = get_parent_xblock(unit) - assert subsection is not None, "Could not determine parent subsection from unit " + unicode(unit.location) - section = get_parent_xblock(subsection) - assert section is not None, "Could not determine ancestor section from unit " + unicode(unit.location) + assert unit is not None, "Could not determine unit page" + subsection = get_parent_xblock(unit) + assert subsection is not None, "Could not determine parent subsection from unit " + unicode(unit.location) + section = get_parent_xblock(subsection) + assert section is not None, "Could not determine ancestor section from unit " + unicode(unit.location) - # Fetch the XBlock info for use by the container page. Note that it includes information - # about the block's ancestors and siblings for use by the Unit Outline. - xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page) + # Fetch the XBlock info for use by the container page. Note that it includes information + # about the block's ancestors and siblings for use by the Unit Outline. + xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page) - # Create the link for preview. - preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE') - # need to figure out where this item is in the list of children as the - # preview will need this - index = 1 - for child in subsection.get_children(): - if child.location == unit.location: - break - index += 1 - preview_lms_link = ( - u'//{preview_lms_base}/courses/{org}/{course}/{course_name}/courseware/{section}/{subsection}/{index}' - ).format( - preview_lms_base=preview_lms_base, - lms_base=settings.LMS_BASE, - org=course.location.org, - course=course.location.course, - course_name=course.location.name, - section=section.location.name, - subsection=subsection.location.name, - index=index - ) + # Create the link for preview. + preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE') + # need to figure out where this item is in the list of children as the + # preview will need this + index = 1 + for child in subsection.get_children(): + if child.location == unit.location: + break + index += 1 + preview_lms_link = ( + u'//{preview_lms_base}/courses/{org}/{course}/{course_name}/courseware/{section}/{subsection}/{index}' + ).format( + preview_lms_base=preview_lms_base, + lms_base=settings.LMS_BASE, + org=course.location.org, + course=course.location.course, + course_name=course.location.name, + section=section.location.name, + subsection=subsection.location.name, + index=index + ) - return render_to_response('container.html', { - 'context_course': course, # Needed only for display of menus at top of page. - 'action': action, - 'xblock': xblock, - 'xblock_locator': xblock.location, - 'unit': unit, - 'is_unit_page': is_unit_page, - 'subsection': subsection, - 'section': section, - 'new_unit_category': 'vertical', - 'ancestor_xblocks': ancestor_xblocks, - 'component_templates': json.dumps(component_templates), - 'xblock_info': xblock_info, - 'draft_preview_link': preview_lms_link, - 'published_preview_link': lms_link, - }) + return render_to_response('container.html', { + 'context_course': course, # Needed only for display of menus at top of page. + 'action': action, + 'xblock': xblock, + 'xblock_locator': xblock.location, + 'unit': unit, + 'is_unit_page': is_unit_page, + 'subsection': subsection, + 'section': section, + 'new_unit_category': 'vertical', + 'ancestor_xblocks': ancestor_xblocks, + 'component_templates': json.dumps(component_templates), + 'xblock_info': xblock_info, + 'draft_preview_link': preview_lms_link, + 'published_preview_link': lms_link, + }) else: return HttpResponseBadRequest("Only supports HTML requests") diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 6b0d6878ba..468d0efb5b 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -212,8 +212,10 @@ def course_handler(request, course_key_string=None): response_format = request.REQUEST.get('format', 'html') if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): if request.method == 'GET': - course_module = _get_course_module(CourseKey.from_string(course_key_string), request.user, depth=None) - return JsonResponse(_course_outline_json(request, course_module)) + course_key = CourseKey.from_string(course_key_string) + with modulestore().bulk_operations(course_key): + course_module = _get_course_module(course_key, request.user, depth=None) + return JsonResponse(_course_outline_json(request, course_module)) elif request.method == 'POST': # not sure if this is only post. If one will have ids, it goes after access return _create_or_rerun_course(request) elif not has_course_access(request.user, CourseKey.from_string(course_key_string)): @@ -248,15 +250,16 @@ def course_rerun_handler(request, course_key_string): if not GlobalStaff().has_user(request.user): raise PermissionDenied() course_key = CourseKey.from_string(course_key_string) - course_module = _get_course_module(course_key, request.user, depth=3) - if request.method == 'GET': - return render_to_response('course-create-rerun.html', { - 'source_course_key': course_key, - 'display_name': course_module.display_name, - 'user': request.user, - 'course_creator_status': _get_course_creator_status(request.user), - 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False) - }) + with modulestore().bulk_operations(course_key): + course_module = _get_course_module(course_key, request.user, depth=3) + if request.method == 'GET': + return render_to_response('course-create-rerun.html', { + 'source_course_key': course_key, + 'display_name': course_module.display_name, + 'user': request.user, + 'course_creator_status': _get_course_creator_status(request.user), + 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False) + }) def _course_outline_json(request, course_module): """ @@ -644,20 +647,20 @@ def course_info_handler(request, course_key_string): html: return html for editing the course info handouts and updates. """ course_key = CourseKey.from_string(course_key_string) - course_module = _get_course_module(course_key, request.user) - if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): - - return render_to_response( - 'course_info.html', - { - 'context_course': course_module, - 'updates_url': reverse_course_url('course_info_update_handler', course_key), - 'handouts_locator': course_key.make_usage_key('course_info', 'handouts'), - 'base_asset_url': StaticContent.get_base_url_path_for_course_assets(course_module.id) - } - ) - else: - return HttpResponseBadRequest("Only supports html requests") + with modulestore().bulk_operations(course_key): + course_module = _get_course_module(course_key, request.user) + if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): + return render_to_response( + 'course_info.html', + { + 'context_course': course_module, + 'updates_url': reverse_course_url('course_info_update_handler', course_key), + 'handouts_locator': course_key.make_usage_key('course_info', 'handouts'), + 'base_asset_url': StaticContent.get_base_url_path_for_course_assets(course_module.id) + } + ) + else: + return HttpResponseBadRequest("Only supports html requests") # pylint: disable=unused-argument @@ -727,43 +730,44 @@ def settings_handler(request, course_key_string): json: update the Course and About xblocks through the CourseDetails model """ course_key = CourseKey.from_string(course_key_string) - course_module = _get_course_module(course_key, request.user) - if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': - upload_asset_url = reverse_course_url('assets_handler', course_key) + with modulestore().bulk_operations(course_key): + course_module = _get_course_module(course_key, request.user) + if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': + upload_asset_url = reverse_course_url('assets_handler', course_key) - # see if the ORG of this course can be attributed to a 'Microsite'. In that case, the - # course about page should be editable in Studio - about_page_editable = not microsite.get_value_for_org( - course_module.location.org, - 'ENABLE_MKTG_SITE', - settings.FEATURES.get('ENABLE_MKTG_SITE', False) - ) - - short_description_editable = settings.FEATURES.get('EDITABLE_SHORT_DESCRIPTION', True) - - return render_to_response('settings.html', { - 'context_course': course_module, - 'course_locator': course_key, - 'lms_link_for_about_page': utils.get_lms_link_for_about_page(course_key), - 'course_image_url': utils.course_image_url(course_module), - 'details_url': reverse_course_url('settings_handler', course_key), - 'about_page_editable': about_page_editable, - 'short_description_editable': short_description_editable, - 'upload_asset_url': upload_asset_url - }) - elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): - if request.method == 'GET': - return JsonResponse( - CourseDetails.fetch(course_key), - # encoder serializes dates, old locations, and instances - encoder=CourseSettingsEncoder - ) - else: # post or put, doesn't matter. - return JsonResponse( - CourseDetails.update_from_json(course_key, request.json, request.user), - encoder=CourseSettingsEncoder + # see if the ORG of this course can be attributed to a 'Microsite'. In that case, the + # course about page should be editable in Studio + about_page_editable = not microsite.get_value_for_org( + course_module.location.org, + 'ENABLE_MKTG_SITE', + settings.FEATURES.get('ENABLE_MKTG_SITE', False) ) + short_description_editable = settings.FEATURES.get('EDITABLE_SHORT_DESCRIPTION', True) + + return render_to_response('settings.html', { + 'context_course': course_module, + 'course_locator': course_key, + 'lms_link_for_about_page': utils.get_lms_link_for_about_page(course_key), + 'course_image_url': utils.course_image_url(course_module), + 'details_url': reverse_course_url('settings_handler', course_key), + 'about_page_editable': about_page_editable, + 'short_description_editable': short_description_editable, + 'upload_asset_url': upload_asset_url + }) + elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): + if request.method == 'GET': + return JsonResponse( + CourseDetails.fetch(course_key), + # encoder serializes dates, old locations, and instances + encoder=CourseSettingsEncoder + ) + else: # post or put, doesn't matter. + return JsonResponse( + CourseDetails.update_from_json(course_key, request.json, request.user), + encoder=CourseSettingsEncoder + ) + @login_required @ensure_csrf_cookie @@ -781,41 +785,42 @@ def grading_handler(request, course_key_string, grader_index=None): json w/ grader_index: create or update the specific grader (create if index out of range) """ course_key = CourseKey.from_string(course_key_string) - course_module = _get_course_module(course_key, request.user) + with modulestore().bulk_operations(course_key): + course_module = _get_course_module(course_key, request.user) - if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': - course_details = CourseGradingModel.fetch(course_key) + if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': + course_details = CourseGradingModel.fetch(course_key) - return render_to_response('settings_graders.html', { - 'context_course': course_module, - 'course_locator': course_key, - 'course_details': json.dumps(course_details, cls=CourseSettingsEncoder), - 'grading_url': reverse_course_url('grading_handler', course_key), - }) - elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): - if request.method == 'GET': - if grader_index is None: - return JsonResponse( - CourseGradingModel.fetch(course_key), - # encoder serializes dates, old locations, and instances - encoder=CourseSettingsEncoder - ) - else: - return JsonResponse(CourseGradingModel.fetch_grader(course_key, grader_index)) - elif request.method in ('POST', 'PUT'): # post or put, doesn't matter. - # None implies update the whole model (cutoffs, graceperiod, and graders) not a specific grader - if grader_index is None: - return JsonResponse( - CourseGradingModel.update_from_json(course_key, request.json, request.user), - encoder=CourseSettingsEncoder - ) - else: - return JsonResponse( - CourseGradingModel.update_grader_from_json(course_key, request.json, request.user) - ) - elif request.method == "DELETE" and grader_index is not None: - CourseGradingModel.delete_grader(course_key, grader_index, request.user) - return JsonResponse() + return render_to_response('settings_graders.html', { + 'context_course': course_module, + 'course_locator': course_key, + 'course_details': json.dumps(course_details, cls=CourseSettingsEncoder), + 'grading_url': reverse_course_url('grading_handler', course_key), + }) + elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): + if request.method == 'GET': + if grader_index is None: + return JsonResponse( + CourseGradingModel.fetch(course_key), + # encoder serializes dates, old locations, and instances + encoder=CourseSettingsEncoder + ) + else: + return JsonResponse(CourseGradingModel.fetch_grader(course_key, grader_index)) + elif request.method in ('POST', 'PUT'): # post or put, doesn't matter. + # None implies update the whole model (cutoffs, graceperiod, and graders) not a specific grader + if grader_index is None: + return JsonResponse( + CourseGradingModel.update_from_json(course_key, request.json, request.user), + encoder=CourseSettingsEncoder + ) + else: + return JsonResponse( + CourseGradingModel.update_grader_from_json(course_key, request.json, request.user) + ) + elif request.method == "DELETE" and grader_index is not None: + CourseGradingModel.delete_grader(course_key, grader_index, request.user) + return JsonResponse() # pylint: disable=invalid-name @@ -892,41 +897,42 @@ def advanced_settings_handler(request, course_key_string): metadata dicts. """ course_key = CourseKey.from_string(course_key_string) - course_module = _get_course_module(course_key, request.user) - if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': + with modulestore().bulk_operations(course_key): + course_module = _get_course_module(course_key, request.user) + if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': - return render_to_response('settings_advanced.html', { - 'context_course': course_module, - 'advanced_dict': json.dumps(CourseMetadata.fetch(course_module)), - 'advanced_settings_url': reverse_course_url('advanced_settings_handler', course_key) - }) - elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): - if request.method == 'GET': - return JsonResponse(CourseMetadata.fetch(course_module)) - else: - try: - # Whether or not to filter the tabs key out of the settings metadata - filter_tabs = _config_course_advanced_components(request, course_module) + return render_to_response('settings_advanced.html', { + 'context_course': course_module, + 'advanced_dict': json.dumps(CourseMetadata.fetch(course_module)), + 'advanced_settings_url': reverse_course_url('advanced_settings_handler', course_key) + }) + elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): + if request.method == 'GET': + return JsonResponse(CourseMetadata.fetch(course_module)) + else: + try: + # Whether or not to filter the tabs key out of the settings metadata + filter_tabs = _config_course_advanced_components(request, course_module) - # validate data formats and update - is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json( - course_module, - request.json, - filter_tabs=filter_tabs, - user=request.user, - ) + # validate data formats and update + is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json( + course_module, + request.json, + filter_tabs=filter_tabs, + user=request.user, + ) - if is_valid: - return JsonResponse(updated_data) - else: - return JsonResponseBadRequest(errors) + if is_valid: + return JsonResponse(updated_data) + else: + return JsonResponseBadRequest(errors) - # Handle all errors that validation doesn't catch - except (TypeError, ValueError) as err: - return HttpResponseBadRequest( - django.utils.html.escape(err.message), - content_type="text/plain" - ) + # Handle all errors that validation doesn't catch + except (TypeError, ValueError) as err: + return HttpResponseBadRequest( + django.utils.html.escape(err.message), + content_type="text/plain" + ) class TextbookValidationError(Exception): @@ -1004,63 +1010,64 @@ def textbooks_list_handler(request, course_key_string): json: overwrite all textbooks in the course with the given list """ course_key = CourseKey.from_string(course_key_string) - course = _get_course_module(course_key, request.user) store = modulestore() + with store.bulk_operations(course_key): + course = _get_course_module(course_key, request.user) - if not "application/json" in request.META.get('HTTP_ACCEPT', 'text/html'): - # return HTML page - upload_asset_url = reverse_course_url('assets_handler', course_key) - textbook_url = reverse_course_url('textbooks_list_handler', course_key) - return render_to_response('textbooks.html', { - 'context_course': course, - 'textbooks': course.pdf_textbooks, - 'upload_asset_url': upload_asset_url, - 'textbook_url': textbook_url, - }) + if not "application/json" in request.META.get('HTTP_ACCEPT', 'text/html'): + # return HTML page + upload_asset_url = reverse_course_url('assets_handler', course_key) + textbook_url = reverse_course_url('textbooks_list_handler', course_key) + return render_to_response('textbooks.html', { + 'context_course': course, + 'textbooks': course.pdf_textbooks, + 'upload_asset_url': upload_asset_url, + 'textbook_url': textbook_url, + }) - # from here on down, we know the client has requested JSON - if request.method == 'GET': - return JsonResponse(course.pdf_textbooks) - elif request.method == 'PUT': - try: - textbooks = validate_textbooks_json(request.body) - except TextbookValidationError as err: - return JsonResponse({"error": err.message}, status=400) + # from here on down, we know the client has requested JSON + if request.method == 'GET': + return JsonResponse(course.pdf_textbooks) + elif request.method == 'PUT': + try: + textbooks = validate_textbooks_json(request.body) + except TextbookValidationError as err: + return JsonResponse({"error": err.message}, status=400) - tids = set(t["id"] for t in textbooks if "id" in t) - for textbook in textbooks: - if not "id" in textbook: - tid = assign_textbook_id(textbook, tids) - textbook["id"] = tid - tids.add(tid) + tids = set(t["id"] for t in textbooks if "id" in t) + for textbook in textbooks: + if not "id" in textbook: + tid = assign_textbook_id(textbook, tids) + textbook["id"] = tid + tids.add(tid) - if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs): - course.tabs.append(PDFTextbookTabs()) - course.pdf_textbooks = textbooks - store.update_item(course, request.user.id) - return JsonResponse(course.pdf_textbooks) - elif request.method == 'POST': - # create a new textbook for the course - try: - textbook = validate_textbook_json(request.body) - except TextbookValidationError as err: - return JsonResponse({"error": err.message}, status=400) - if not textbook.get("id"): - tids = set(t["id"] for t in course.pdf_textbooks if "id" in t) - textbook["id"] = assign_textbook_id(textbook, tids) - existing = course.pdf_textbooks - existing.append(textbook) - course.pdf_textbooks = existing - if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs): - course.tabs.append(PDFTextbookTabs()) - store.update_item(course, request.user.id) - resp = JsonResponse(textbook, status=201) - resp["Location"] = reverse_course_url( - 'textbooks_detail_handler', - course.id, - kwargs={'textbook_id': textbook["id"]} - ) - return resp + if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs): + course.tabs.append(PDFTextbookTabs()) + course.pdf_textbooks = textbooks + store.update_item(course, request.user.id) + return JsonResponse(course.pdf_textbooks) + elif request.method == 'POST': + # create a new textbook for the course + try: + textbook = validate_textbook_json(request.body) + except TextbookValidationError as err: + return JsonResponse({"error": err.message}, status=400) + if not textbook.get("id"): + tids = set(t["id"] for t in course.pdf_textbooks if "id" in t) + textbook["id"] = assign_textbook_id(textbook, tids) + existing = course.pdf_textbooks + existing.append(textbook) + course.pdf_textbooks = existing + if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs): + course.tabs.append(PDFTextbookTabs()) + store.update_item(course, request.user.id) + resp = JsonResponse(textbook, status=201) + resp["Location"] = reverse_course_url( + 'textbooks_detail_handler', + course.id, + kwargs={'textbook_id': textbook["id"]} + ) + return resp @login_required @@ -1079,45 +1086,46 @@ def textbooks_detail_handler(request, course_key_string, textbook_id): json: remove textbook """ course_key = CourseKey.from_string(course_key_string) - course_module = _get_course_module(course_key, request.user) store = modulestore() - matching_id = [tb for tb in course_module.pdf_textbooks - if unicode(tb.get("id")) == unicode(textbook_id)] - if matching_id: - textbook = matching_id[0] - else: - textbook = None - - if request.method == 'GET': - if not textbook: - return JsonResponse(status=404) - return JsonResponse(textbook) - elif request.method in ('POST', 'PUT'): # can be either and sometimes - # django is rewriting one to the other - try: - new_textbook = validate_textbook_json(request.body) - except TextbookValidationError as err: - return JsonResponse({"error": err.message}, status=400) - new_textbook["id"] = textbook_id - if textbook: - i = course_module.pdf_textbooks.index(textbook) - new_textbooks = course_module.pdf_textbooks[0:i] - new_textbooks.append(new_textbook) - new_textbooks.extend(course_module.pdf_textbooks[i + 1:]) - course_module.pdf_textbooks = new_textbooks + with store.bulk_operations(course_key): + course_module = _get_course_module(course_key, request.user) + matching_id = [tb for tb in course_module.pdf_textbooks + if unicode(tb.get("id")) == unicode(textbook_id)] + if matching_id: + textbook = matching_id[0] else: - course_module.pdf_textbooks.append(new_textbook) - store.update_item(course_module, request.user.id) - return JsonResponse(new_textbook, status=201) - elif request.method == 'DELETE': - if not textbook: - return JsonResponse(status=404) - i = course_module.pdf_textbooks.index(textbook) - remaining_textbooks = course_module.pdf_textbooks[0:i] - remaining_textbooks.extend(course_module.pdf_textbooks[i + 1:]) - course_module.pdf_textbooks = remaining_textbooks - store.update_item(course_module, request.user.id) - return JsonResponse() + textbook = None + + if request.method == 'GET': + if not textbook: + return JsonResponse(status=404) + return JsonResponse(textbook) + elif request.method in ('POST', 'PUT'): # can be either and sometimes + # django is rewriting one to the other + try: + new_textbook = validate_textbook_json(request.body) + except TextbookValidationError as err: + return JsonResponse({"error": err.message}, status=400) + new_textbook["id"] = textbook_id + if textbook: + i = course_module.pdf_textbooks.index(textbook) + new_textbooks = course_module.pdf_textbooks[0:i] + new_textbooks.append(new_textbook) + new_textbooks.extend(course_module.pdf_textbooks[i + 1:]) + course_module.pdf_textbooks = new_textbooks + else: + course_module.pdf_textbooks.append(new_textbook) + store.update_item(course_module, request.user.id) + return JsonResponse(new_textbook, status=201) + elif request.method == 'DELETE': + if not textbook: + return JsonResponse(status=404) + i = course_module.pdf_textbooks.index(textbook) + remaining_textbooks = course_module.pdf_textbooks[0:i] + remaining_textbooks.extend(course_module.pdf_textbooks[i + 1:]) + course_module.pdf_textbooks = remaining_textbooks + store.update_item(course_module, request.user.id) + return JsonResponse() class GroupConfigurationsValidationError(Exception): @@ -1314,42 +1322,43 @@ def group_configurations_list_handler(request, course_key_string): json: create new group configuration """ course_key = CourseKey.from_string(course_key_string) - course = _get_course_module(course_key, request.user) store = modulestore() + with store.bulk_operations(course_key): + course = _get_course_module(course_key, request.user) - if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): - group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key) - course_outline_url = reverse_course_url('course_handler', course_key) - split_test_enabled = SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules + if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): + group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key) + course_outline_url = reverse_course_url('course_handler', course_key) + split_test_enabled = SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules - configurations = GroupConfiguration.add_usage_info(course, store) + configurations = GroupConfiguration.add_usage_info(course, store) - return render_to_response('group_configurations.html', { - 'context_course': course, - 'group_configuration_url': group_configuration_url, - 'course_outline_url': course_outline_url, - 'configurations': configurations if split_test_enabled else None, - }) - elif "application/json" in request.META.get('HTTP_ACCEPT'): - if request.method == 'POST': - # create a new group configuration for the course - try: - new_configuration = GroupConfiguration(request.body, course).get_user_partition() - except GroupConfigurationsValidationError as err: - return JsonResponse({"error": err.message}, status=400) + return render_to_response('group_configurations.html', { + 'context_course': course, + 'group_configuration_url': group_configuration_url, + 'course_outline_url': course_outline_url, + 'configurations': configurations if split_test_enabled else None, + }) + elif "application/json" in request.META.get('HTTP_ACCEPT'): + if request.method == 'POST': + # create a new group configuration for the course + try: + new_configuration = GroupConfiguration(request.body, course).get_user_partition() + except GroupConfigurationsValidationError as err: + return JsonResponse({"error": err.message}, status=400) - course.user_partitions.append(new_configuration) - response = JsonResponse(new_configuration.to_json(), status=201) + course.user_partitions.append(new_configuration) + response = JsonResponse(new_configuration.to_json(), status=201) - response["Location"] = reverse_course_url( - 'group_configurations_detail_handler', - course.id, - kwargs={'group_configuration_id': new_configuration.id} # pylint: disable=no-member - ) - store.update_item(course, request.user.id) - return response - else: - return HttpResponse(status=406) + response["Location"] = reverse_course_url( + 'group_configurations_detail_handler', + course.id, + kwargs={'group_configuration_id': new_configuration.id} # pylint: disable=no-member + ) + store.update_item(course, request.user.id) + return response + else: + return HttpResponse(status=406) @login_required @@ -1364,46 +1373,47 @@ def group_configurations_detail_handler(request, course_key_string, group_config json: update group configuration based on provided information """ course_key = CourseKey.from_string(course_key_string) - course = _get_course_module(course_key, request.user) store = modulestore() - matching_id = [p for p in course.user_partitions - if unicode(p.id) == unicode(group_configuration_id)] - if matching_id: - configuration = matching_id[0] - else: - configuration = None - - if request.method in ('POST', 'PUT'): # can be either and sometimes - # django is rewriting one to the other - try: - new_configuration = GroupConfiguration(request.body, course, group_configuration_id).get_user_partition() - except GroupConfigurationsValidationError as err: - return JsonResponse({"error": err.message}, status=400) - - if configuration: - index = course.user_partitions.index(configuration) - course.user_partitions[index] = new_configuration + with store.bulk_operations(course_key): + course = _get_course_module(course_key, request.user) + matching_id = [p for p in course.user_partitions + if unicode(p.id) == unicode(group_configuration_id)] + if matching_id: + configuration = matching_id[0] else: - course.user_partitions.append(new_configuration) - store.update_item(course, request.user.id) - configuration = GroupConfiguration.update_usage_info(store, course, new_configuration) - return JsonResponse(configuration, status=201) - elif request.method == "DELETE": - if not configuration: - return JsonResponse(status=404) + configuration = None - # Verify that group configuration is not already in use. - usages = GroupConfiguration.get_usage_info(course, store) - if usages.get(int(group_configuration_id)): - return JsonResponse( - {"error": _("This Group Configuration is already in use and cannot be removed.")}, - status=400 - ) + if request.method in ('POST', 'PUT'): # can be either and sometimes + # django is rewriting one to the other + try: + new_configuration = GroupConfiguration(request.body, course, group_configuration_id).get_user_partition() + except GroupConfigurationsValidationError as err: + return JsonResponse({"error": err.message}, status=400) - index = course.user_partitions.index(configuration) - course.user_partitions.pop(index) - store.update_item(course, request.user.id) - return JsonResponse(status=204) + if configuration: + index = course.user_partitions.index(configuration) + course.user_partitions[index] = new_configuration + else: + course.user_partitions.append(new_configuration) + store.update_item(course, request.user.id) + configuration = GroupConfiguration.update_usage_info(store, course, new_configuration) + return JsonResponse(configuration, status=201) + elif request.method == "DELETE": + if not configuration: + return JsonResponse(status=404) + + # Verify that group configuration is not already in use. + usages = GroupConfiguration.get_usage_info(course, store) + if usages.get(int(group_configuration_id)): + return JsonResponse( + {"error": _("This Group Configuration is already in use and cannot be removed.")}, + status=400 + ) + + index = course.user_partitions.index(configuration) + course.user_partitions.pop(index) + store.update_item(course, request.user.id) + return JsonResponse(status=204) def _get_course_creator_status(user): diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 93fefb3c3e..132bcbff6c 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -461,54 +461,55 @@ def _create_item(request): raise PermissionDenied() store = modulestore() - parent = store.get_item(usage_key) - dest_usage_key = usage_key.replace(category=category, name=uuid4().hex) + with store.bulk_operations(usage_key.course_key): + parent = store.get_item(usage_key) + dest_usage_key = usage_key.replace(category=category, name=uuid4().hex) - # get the metadata, display_name, and definition from the request - metadata = {} - data = None - template_id = request.json.get('boilerplate') - if template_id: - clz = parent.runtime.load_block_type(category) - if clz is not None: - template = clz.get_template(template_id) - if template is not None: - metadata = template.get('metadata', {}) - data = template.get('data') + # get the metadata, display_name, and definition from the request + metadata = {} + data = None + template_id = request.json.get('boilerplate') + if template_id: + clz = parent.runtime.load_block_type(category) + if clz is not None: + template = clz.get_template(template_id) + if template is not None: + metadata = template.get('metadata', {}) + data = template.get('data') - if display_name is not None: - metadata['display_name'] = display_name + if display_name is not None: + metadata['display_name'] = display_name - # TODO need to fix components that are sending definition_data as strings, instead of as dicts - # For now, migrate them into dicts here. - if isinstance(data, basestring): - data = {'data': data} + # TODO need to fix components that are sending definition_data as strings, instead of as dicts + # For now, migrate them into dicts here. + if isinstance(data, basestring): + data = {'data': data} - created_block = store.create_child( - request.user.id, - usage_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=data, - metadata=metadata, - runtime=parent.runtime, - ) - - # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so - # if we add one then we need to also add it to the policy information (i.e. metadata) - # we should remove this once we can break this reference from the course to static tabs - if category == 'static_tab': - display_name = display_name or _("Empty") # Prevent name being None - course = store.get_course(dest_usage_key.course_key) - course.tabs.append( - StaticTab( - name=display_name, - url_slug=dest_usage_key.name, - ) + created_block = store.create_child( + request.user.id, + usage_key, + dest_usage_key.block_type, + block_id=dest_usage_key.block_id, + definition_data=data, + metadata=metadata, + runtime=parent.runtime, ) - store.update_item(course, request.user.id) - return JsonResponse({"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)}) + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + if category == 'static_tab': + display_name = display_name or _("Empty") # Prevent name being None + course = store.get_course(dest_usage_key.course_key) + course.tabs.append( + StaticTab( + name=display_name, + url_slug=dest_usage_key.name, + ) + ) + store.update_item(course, request.user.id) + + return JsonResponse({"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)}) def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_name=None): @@ -516,52 +517,53 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ Duplicate an existing xblock as a child of the supplied parent_usage_key. """ store = modulestore() - source_item = store.get_item(duplicate_source_usage_key) - # Change the blockID to be unique. - dest_usage_key = source_item.location.replace(name=uuid4().hex) - category = dest_usage_key.block_type + with store.bulk_operations(duplicate_source_usage_key.course_key): + source_item = store.get_item(duplicate_source_usage_key) + # Change the blockID to be unique. + dest_usage_key = source_item.location.replace(name=uuid4().hex) + category = dest_usage_key.block_type - # Update the display name to indicate this is a duplicate (unless display name provided). - duplicate_metadata = own_metadata(source_item) - if display_name is not None: - duplicate_metadata['display_name'] = display_name - else: - if source_item.display_name is None: - duplicate_metadata['display_name'] = _("Duplicate of {0}").format(source_item.category) + # Update the display name to indicate this is a duplicate (unless display name provided). + duplicate_metadata = own_metadata(source_item) + if display_name is not None: + duplicate_metadata['display_name'] = display_name else: - duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) + if source_item.display_name is None: + duplicate_metadata['display_name'] = _("Duplicate of {0}").format(source_item.category) + else: + duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) - dest_module = store.create_item( - user.id, - dest_usage_key.course_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), - metadata=duplicate_metadata, - runtime=source_item.runtime, - ) + dest_module = store.create_item( + user.id, + dest_usage_key.course_key, + dest_usage_key.block_type, + block_id=dest_usage_key.block_id, + definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), + metadata=duplicate_metadata, + runtime=source_item.runtime, + ) - # Children are not automatically copied over (and not all xblocks have a 'children' attribute). - # Because DAGs are not fully supported, we need to actually duplicate each child as well. - if source_item.has_children: - dest_module.children = [] - for child in source_item.children: - dupe = _duplicate_item(dest_module.location, child, user=user) - dest_module.children.append(dupe) - store.update_item(dest_module, user.id) + # Children are not automatically copied over (and not all xblocks have a 'children' attribute). + # Because DAGs are not fully supported, we need to actually duplicate each child as well. + if source_item.has_children: + dest_module.children = [] + for child in source_item.children: + dupe = _duplicate_item(dest_module.location, child, user=user) + dest_module.children.append(dupe) + store.update_item(dest_module, user.id) - if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: - parent = store.get_item(parent_usage_key) - # If source was already a child of the parent, add duplicate immediately afterward. - # Otherwise, add child to end. - if source_item.location in parent.children: - source_index = parent.children.index(source_item.location) - parent.children.insert(source_index + 1, dest_module.location) - else: - parent.children.append(dest_module.location) - store.update_item(parent, user.id) + if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: + parent = store.get_item(parent_usage_key) + # If source was already a child of the parent, add duplicate immediately afterward. + # Otherwise, add child to end. + if source_item.location in parent.children: + source_index = parent.children.index(source_item.location) + parent.children.insert(source_index + 1, dest_module.location) + else: + parent.children.append(dest_module.location) + store.update_item(parent, user.id) - return dest_module.location + return dest_module.location def _delete_item(usage_key, user): @@ -571,16 +573,17 @@ def _delete_item(usage_key, user): """ store = modulestore() - # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so - # if we add one then we need to also add it to the policy information (i.e. metadata) - # we should remove this once we can break this reference from the course to static tabs - if usage_key.category == 'static_tab': - course = store.get_course(usage_key.course_key) - existing_tabs = course.tabs or [] - course.tabs = [tab for tab in existing_tabs if tab.get('url_slug') != usage_key.name] - store.update_item(course, user.id) + with store.bulk_operations(usage_key.course_key): + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + if usage_key.category == 'static_tab': + course = store.get_course(usage_key.course_key) + existing_tabs = course.tabs or [] + course.tabs = [tab for tab in existing_tabs if tab.get('url_slug') != usage_key.name] + store.update_item(course, user.id) - store.delete_item(usage_key, user.id) + store.delete_item(usage_key, user.id) # pylint: disable=W0613 @@ -618,17 +621,18 @@ def _get_xblock(usage_key, user): in the CREATE_IF_NOT_FOUND list, an xblock will be created and saved automatically. """ store = modulestore() - try: - return store.get_item(usage_key, depth=None) - except ItemNotFoundError: - if usage_key.category in CREATE_IF_NOT_FOUND: - # Create a new one for certain categories only. Used for course info handouts. - return store.create_item(user.id, usage_key.course_key, usage_key.block_type, block_id=usage_key.block_id) - else: - raise - except InvalidLocationError: - log.error("Can't find item by location.") - return JsonResponse({"error": "Can't find item by location: " + unicode(usage_key)}, 404) + with store.bulk_operations(usage_key.course_key): + try: + return store.get_item(usage_key, depth=None) + except ItemNotFoundError: + if usage_key.category in CREATE_IF_NOT_FOUND: + # Create a new one for certain categories only. Used for course info handouts. + return store.create_item(user.id, usage_key.course_key, usage_key.block_type, block_id=usage_key.block_id) + else: + raise + except InvalidLocationError: + log.error("Can't find item by location.") + return JsonResponse({"error": "Can't find item by location: " + unicode(usage_key)}, 404) def _get_module_info(xblock, rewrite_static_links=True): @@ -636,19 +640,20 @@ def _get_module_info(xblock, rewrite_static_links=True): metadata, data, id representation of a leaf module fetcher. :param usage_key: A UsageKey """ - data = getattr(xblock, 'data', '') - if rewrite_static_links: - data = replace_static_urls( - data, - None, - course_id=xblock.location.course_key - ) + with modulestore().bulk_operations(xblock.location.course_key): + data = getattr(xblock, 'data', '') + if rewrite_static_links: + data = replace_static_urls( + data, + None, + course_id=xblock.location.course_key + ) - # Pre-cache has changes for the entire course because we'll need it for the ancestor info - modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None)) + # Pre-cache has changes for the entire course because we'll need it for the ancestor info + modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None)) - # Note that children aren't being returned until we have a use case. - return create_xblock_info(xblock, data=data, metadata=own_metadata(xblock), include_ancestor_info=True) + # Note that children aren't being returned until we have a use case. + return create_xblock_info(xblock, data=data, metadata=own_metadata(xblock), include_ancestor_info=True) def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False,