Skip to content

Commit

Permalink
feat: call XBlock handlers using the same version of the block
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Oct 11, 2024
1 parent badefc3 commit 89f369c
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 25 deletions.
21 changes: 18 additions & 3 deletions openedx/core/djangoapps/xblock/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,13 @@ def render_block_view(block, view_name, user): # pylint: disable=unused-argumen
return fragment


def get_handler_url(usage_key, handler_name, user):
def get_handler_url(
usage_key: UsageKeyV2,
handler_name: str,
user: UserType,
*,
version: int | LatestVersion = LatestVersion.AUTO,
):
"""
A method for getting the URL to any XBlock handler. The URL must be usable
without any authentication (no cookie, no OAuth/JWT), and may expire. (So
Expand All @@ -265,6 +271,11 @@ def get_handler_url(usage_key, handler_name, user):
usage_key - Usage Key (Opaque Key object or string)
handler_name - Name of the handler or a dummy name like 'any_handler'
user - Django User (registered or anonymous)
version - Run the handler against a specific version of the
block (e.g. when viewing an old version of it in
Studio). Some blocks use handlers to load their data
so it's important the handler matches the student_view
etc.
This view does not check/care if the XBlock actually exists.
"""
Expand All @@ -282,12 +293,16 @@ def get_handler_url(usage_key, handler_name, user):
# and this XBlock:
secure_token = get_secure_token_for_xblock_handler(user_id, usage_key_str)
# Now generate the URL to that handler:
path = reverse('xblock_api:xblock_handler', kwargs={
kwargs = {
'usage_key_str': usage_key_str,
'user_id': user_id,
'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)

# We must return an absolute URL. We can't just use
# rest_framework.reverse.reverse to get the absolute URL because this method
# can be called by the XBlock from python as well and in that case we don't
Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/xblock/rest_api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@
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
re_path(r'^embed/(?P<view_name>[\w\-]+)/$', views.embed_block_view),
path('embed/<str:view_name>/', views.embed_block_view),
])),
]
42 changes: 26 additions & 16 deletions openedx/core/djangoapps/xblock/rest_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,23 @@
invalid_not_found_fmt = "XBlock {usage_key} does not exist, or you don't have permission to view it."


def parse_version_request(version_str: str | None) -> LatestVersion | int:
"""
Given a version parameter from a query string (?version=14, ?version=draft,
?version=published), get the LatestVersion parameter to use with the API.
"""
if version_str is None:
return LatestVersion.AUTO # AUTO = published if we're in the LMS, draft if we're in Studio.
if version_str == "draft":
return LatestVersion.DRAFT
if version_str == "published":
return LatestVersion.PUBLISHED
try:
return int(version_str)
except ValueError:
raise serializers.ValidationError("Invalid version specifier. Expected 'draft', 'published', or an integer.")


@api_view(['GET'])
@view_auth_classes(is_authenticated=False)
@permission_classes((permissions.AllowAny, )) # Permissions are handled at a lower level, by the learning context
Expand Down Expand Up @@ -110,19 +127,7 @@ def embed_block_view(request, usage_key_str, view_name):
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e

# Check if a specific version has been requested
version = LatestVersion.AUTO # AUTO = published if we're in the LMS, draft if we're in Studio.
if version_request := request.GET.get("version"):
if version_request == "draft":
version = LatestVersion.DRAFT
elif version_request == "published":
version = LatestVersion.PUBLISHED
else:
try:
version = int(version_request)
except ValueError:
raise serializers.ValidationError(
"Invalid version specifier. Expected 'draft', 'published', or an integer."
)
version = parse_version_request(request.GET.get("version"))

try:
block = load_block(usage_key, request.user, check_permission=CheckPerm.CAN_LEARN, version=version)
Expand All @@ -131,9 +136,13 @@ def embed_block_view(request, usage_key_str, view_name):

fragment = _render_block_view(block, view_name, request.user)
handler_urls = {
str(key): _get_handler_url(key, 'handler_name', request.user)
for key in itertools.chain([block.scope_ids.usage_id], getattr(block, 'children', []))
str(block.usage_key): _get_handler_url(block.usage_key, 'handler_name', request.user, version=version)
}
# Currently we don't support child blocks so we don't need this pre-loading of child handler URLs:
# handler_urls = {
# str(key): _get_handler_url(key, 'handler_name', request.user)
# for key in itertools.chain([block.scope_ids.usage_id], getattr(block, 'children', []))
# }
lms_root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL)
context = {
'fragment': fragment,
Expand Down Expand Up @@ -221,7 +230,8 @@ def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name,
raise AuthenticationFailed("Invalid user ID format.")

request_webob = DjangoWebobRequest(request) # Convert from django request to the webob format that XBlocks expect
block = load_block(usage_key, user)

block = load_block(usage_key, user, version=parse_version_request(request.GET.get("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
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,13 @@ def save_block(self, block):
if not self.authored_data_store.has_changes(block):
return # No changes, so no action needed.

if block._runtime_requested_version != LatestVersion.DRAFT:
# Not sure if this is an important restriction but it seems like overwriting the latest version based on
# an old version is likely an accident, so for now we're not going to allow it.
raise ValidationError(
"Do not make changes to a component starting from the published or past versions. Use the latest draft."
)

# Verify that the user has permission to write to authored data in this
# learning context:
if self.user is not None:
Expand Down
25 changes: 20 additions & 5 deletions openedx/core/djangoapps/xblock/runtime/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""
from __future__ import annotations
import logging
from typing import Callable, Optional
from typing import Callable, Optional, Protocol
from urllib.parse import urljoin # pylint: disable=import-error

import crum
Expand All @@ -15,7 +15,7 @@
from django.core.cache import cache
from django.core.exceptions import PermissionDenied
from eventtracking import tracker
from opaque_keys.edx.keys import UsageKey, LearningContextKey
from opaque_keys.edx.keys import UsageKeyV2, LearningContextKey
from web_fragments.fragment import Fragment
from xblock.core import XBlock
from xblock.exceptions import NoSuchServiceError
Expand All @@ -38,7 +38,7 @@
from openedx.core.types import User as UserType
from openedx.core.djangoapps.enrollments.services import EnrollmentsService
from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
from openedx.core.djangoapps.xblock.data import AuthoredDataMode, StudentDataMode
from openedx.core.djangoapps.xblock.data import AuthoredDataMode, StudentDataMode, LatestVersion
from openedx.core.djangoapps.xblock.runtime.ephemeral_field_data import EphemeralKeyValueStore
from openedx.core.djangoapps.xblock.runtime.mixin import LmsBlockMixin
from openedx.core.djangoapps.xblock.utils import get_xblock_id_for_anonymous_user
Expand All @@ -63,6 +63,18 @@ def function(event_type, event):
return function


class GetHandlerFunction(Protocol):
def __call__(
self,
usage_key: UsageKeyV2,
handler_name: str,
user: UserType,
*,
version: int | LatestVersion = LatestVersion.AUTO,
) -> str:
...


class XBlockRuntime(RuntimeShim, Runtime):
"""
This class manages one or more instantiated XBlocks for a particular user,
Expand Down Expand Up @@ -99,7 +111,7 @@ def __init__(
self,
user: UserType | None,
*,
handler_url: Callable[[UsageKey, str, UserType | None], str],
handler_url: GetHandlerFunction,
student_data_mode: StudentDataMode,
authored_data_mode: AuthoredDataMode,
id_reader: Optional[IdReader] = None,
Expand Down Expand Up @@ -140,7 +152,10 @@ def handler_url(self, block, handler_name: str, suffix='', query='', thirdparty=
if thirdparty:
log.warning("thirdparty handlers are not supported by this runtime for XBlock %s.", type(block))

url = self.handler_url_fn(block.scope_ids.usage_id, handler_name, self.user)
# 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 {}
url = self.handler_url_fn(block.usage_key, handler_name, self.user, **kwargs)
if suffix:
if not url.endswith('/'):
url += '/'
Expand Down

0 comments on commit 89f369c

Please sign in to comment.