Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions cms/djangoapps/contentstore/views/block.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.views.decorators.http import require_http_methods
from opaque_keys.edx.keys import CourseKey
from web_fragments.fragment import Fragment
from rest_framework.decorators import api_view

from cms.djangoapps.contentstore.utils import load_services_for_studio
from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW
Expand All @@ -28,6 +29,7 @@
wrap_xblock,
wrap_xblock_aside,
)
from openedx.core.lib.api.view_utils import view_auth_classes
from xmodule.modulestore.django import (
modulestore,
) # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -84,9 +86,10 @@
# ends, which end up reading from an outdated state of the database. For more information see the discussion in the
# following PR: https://github.com/openedx/edx-platform/pull/34020
@transaction.non_atomic_requests
@require_http_methods(("DELETE", "GET", "PUT", "POST", "PATCH"))
@login_required
# @require_http_methods(("DELETE", "GET", "PUT", "POST", "PATCH"))
@view_auth_classes()
@expect_json

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we can drop the expect_json decorator here, as django_rest_framework should handle json gracefully. Not sure if we can/should force json-only - I think django_rest_framework will also accept and parse form encoded data too.

Comment on lines 88 to 91

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the existing decorators, I'm not sure yet if they are fully compatible with the DRF api_view, or if any particular order is required. For example, I discovered that here expect_json must be before api_view, otherwise the request crashes with an error about reading the request body more than once.

@api_view(http_method_names=["DELETE", "GET", "PUT", "POST", "PATCH"])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure yet if the api_view decorator makes any changes that would subtly break any assumptions of normal django request handlers (eg. with error handling). Also not sure if we should use the function based decorator here, or go for a class based API view - similar to https://github.com/samuelallan72/edx-platform/blob/efc77256a832e464f6aba3e3a7bef9de1702a539/openedx/core/djangoapps/content_libraries/rest_api/blocks.py#L430-L440 (this code was written in #35765 as part of an API conversion to fix a similar issue)

def xblock_handler(request, usage_key_string=None):
"""
The restful handler for xblock requests.
Expand Down Expand Up @@ -145,9 +148,9 @@ def xblock_handler(request, usage_key_string=None):
return handle_xblock(request, usage_key_string)


@require_http_methods("GET")
@login_required
@view_auth_classes()
@expect_json
@api_view(http_method_names=["GET"])
def xblock_view_handler(request, usage_key_string, view_name):
"""
The restful handler for requests for rendered xblock views.
Expand Down Expand Up @@ -306,8 +309,9 @@ def xblock_view_handler(request, usage_key_string, view_name):


@xframe_options_exempt
@require_http_methods(["GET"])
@login_required
@view_auth_classes()
@expect_json
@api_view(http_method_names=["GET"])
def xblock_edit_view(request, usage_key_string):
"""
Return rendered xblock edit view.
Expand Down Expand Up @@ -338,9 +342,9 @@ def xblock_edit_view(request, usage_key_string):
return render_to_response('container_editor.html', container_handler_context)


@require_http_methods("GET")
@login_required
@view_auth_classes()
@expect_json
@api_view(http_method_names=["GET"])
def xblock_outline_handler(request, usage_key_string):
"""
The restful handler for requests for XBlock information about the block and its children.
Expand Down Expand Up @@ -371,9 +375,9 @@ def xblock_outline_handler(request, usage_key_string):
raise Http404


@require_http_methods("GET")
@login_required
@view_auth_classes()
@expect_json
@api_view(http_method_names=["GET"])
def xblock_container_handler(request, usage_key_string):
"""
The restful handler for requests for XBlock information about the block and its children.
Expand All @@ -400,8 +404,9 @@ def xblock_container_handler(request, usage_key_string):
raise Http404


@login_required
@require_http_methods(("GET", "DELETE"))
@view_auth_classes()
@expect_json
@api_view(http_method_names=["GET", "DELETE"])
def orphan_handler(request, course_key_string):
"""
View for handling orphan related requests. GET gets all of the current orphans.
Expand Down
Loading