Skip to content

Commit

Permalink
fix: problem block could not be used with versioned handler URls
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Oct 18, 2024
1 parent 83827a3 commit dc8fd74
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 22 deletions.
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/xblock/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,9 @@ def get_handler_url(
'secure_token': secure_token,
'handler_name': handler_name,
}
path = reverse('xblock_api:xblock_handler', kwargs=kwargs)
if version != LatestVersion.AUTO:
path += "?version=" + (str(version) if isinstance(version, int) else version.value)
kwargs["version"] = str(version) if isinstance(version, int) else version.value
path = reverse('xblock_api:xblock_handler', kwargs=kwargs)

# We must return an absolute URL. We can't just use
# rest_framework.reverse.reverse to get the absolute URL because this method
Expand Down
38 changes: 22 additions & 16 deletions openedx/core/djangoapps/xblock/rest_api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,31 @@
# urls_studio and urls_lms, and/or the views could be likewise duplicated.
app_name = 'openedx.core.djangoapps.xblock.rest_api'

block_endpoints = [
# get metadata about an XBlock:
path('', views.block_metadata),
# get/post full json fields of an XBlock:
path('fields/', views.BlockFieldsView.as_view()),
# render one of this XBlock's views (e.g. student_view)
re_path(r'^view/(?P<view_name>[\w\-]+)/$', views.render_block_view),
# get the URL needed to call this XBlock's handlers
re_path(r'^handler_url/(?P<handler_name>[\w\-]+)/$', views.get_handler_url),
# call one of this block's handlers
re_path(
r'^handler/(?P<user_id>\w+)-(?P<secure_token>\w+)/(?P<handler_name>[\w\-]+)/(?P<suffix>.+)?$',
views.xblock_handler,
name='xblock_handler',
),
# API endpoints related to a specific version of this XBlock:
]

urlpatterns = [
path('api/xblock/v2/', include([
path('xblocks/<str:usage_key_str>/', include([
# get metadata about an XBlock:
path('', views.block_metadata),
# get/post full json fields of an XBlock:
path('fields/', views.BlockFieldsView.as_view()),
# render one of this XBlock's views (e.g. student_view)
re_path(r'^view/(?P<view_name>[\w\-]+)/$', views.render_block_view),
# get the URL needed to call this XBlock's handlers
re_path(r'^handler_url/(?P<handler_name>[\w\-]+)/$', views.get_handler_url),
# call one of this block's handlers
re_path(
r'^handler/(?P<user_id>\w+)-(?P<secure_token>\w+)/(?P<handler_name>[\w\-]+)/(?P<suffix>.+)?$',
views.xblock_handler,
name='xblock_handler',
),
])),
# This pattern with the version must come first so '@' isn't included in usage_key_str below
path(r'xblocks/<str:usage_key_str>@<str:version>/', include(block_endpoints)),
path(r'xblocks/<str:usage_key_str>/', include(block_endpoints)),
])),
# Non-API views (these return HTML, not JSON):
path('xblocks/v2/<str:usage_key_str>/', include([
# render one of this XBlock's views (e.g. student_view) for embedding in an iframe
# NOTE: this endpoint is **unstable** and subject to changes after Sumac
Expand Down
6 changes: 3 additions & 3 deletions openedx/core/djangoapps/xblock/rest_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def parse_version_request(version_str: str | None) -> LatestVersion | int:
return int(version_str)
except ValueError:
raise serializers.ValidationError( # pylint: disable=raise-missing-from
"Invalid version specifier '{version_str}'. Expected 'draft', 'published', or an integer."
f"Invalid version specifier '{version_str}'. Expected 'draft', 'published', or an integer."
)


Expand Down Expand Up @@ -184,7 +184,7 @@ def get_handler_url(request, usage_key_str, handler_name):
# and https://github.com/openedx/XBlock/pull/383 for context.
@csrf_exempt
@xframe_options_exempt
def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name, suffix=None):
def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name, suffix=None, version=None):
"""
Run an XBlock's handler and return the result
Expand Down Expand Up @@ -232,7 +232,7 @@ def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name,

request_webob = DjangoWebobRequest(request) # Convert from django request to the webob format that XBlocks expect

block = load_block(usage_key, user, version=parse_version_request(request.GET.get("version")))
block = load_block(usage_key, user, version=parse_version_request(version))
# Run the handler, and save any resulting XBlock field value changes:
response_webob = block.handle(handler_name, request_webob, suffix)
response = webob_to_django_response(response_webob)
Expand Down
9 changes: 8 additions & 1 deletion openedx/core/djangoapps/xblock/runtime/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,14 @@ def handler_url(self, block, handler_name: str, suffix='', query='', thirdparty=

# Note: it's important that we call handlers based on the same version of the block
# (draft block -> draft data available to handler; published block -> published data available to handler)
kwargs = {"version": block._runtime_requested_version} if hasattr(block, "_runtime_requested_version") else {} # pylint: disable=protected-access
kwargs = {}
if hasattr(block, "_runtime_requested_version"): # pylint: disable=protected-access
if self.authored_data_mode == AuthoredDataMode.DEFAULT_DRAFT:
default_version = LatestVersion.DRAFT
else:
default_version = LatestVersion.PUBLISHED
if block._runtime_requested_version != default_version: # pylint: disable=protected-access
kwargs["version"] = block._runtime_requested_version # pylint: disable=protected-access
url = self.handler_url_fn(block.usage_key, handler_name, self.user, **kwargs)
if suffix:
if not url.endswith('/'):
Expand Down

0 comments on commit dc8fd74

Please sign in to comment.