Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support rendering XBlocks in the Learning MFE [FC-0035] #34161

Closed
Closed
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion lms/templates/staff_problem_info.html
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ <h2>${_("Submission History Viewer")}</h2>
<script type="text/javascript">
// assumes courseware.html's loaded this method.
$(function () {
setup_debug('${element_id | n, js_escaped_string}',
(typeof setup_debug !== 'undefined') && setup_debug('${element_id | n, js_escaped_string}',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment about why this change is necessary.

%if edit_link:
'${edit_link | n, js_escaped_string}',
%else:
Expand Down
18 changes: 14 additions & 4 deletions openedx/core/djangoapps/xblock/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from django.urls import reverse
from django.utils.translation import gettext as _
from opaque_keys.edx.keys import UsageKeyV2
from opaque_keys.edx.locator import BundleDefinitionLocator
from opaque_keys.edx.locator import BlockUsageLocator, BundleDefinitionLocator
from rest_framework.exceptions import NotFound
from xblock.core import XBlock
from xblock.exceptions import NoSuchViewError
Expand Down Expand Up @@ -120,17 +120,27 @@ def get_block_metadata(block, includes=()):
editable_children: children in the same bundle, as opposed to linked
children in other bundles.
"""
usage_id = block.scope_ids.usage_id

if isinstance(usage_id, BlockUsageLocator):
# The second method is more efficient, but it doesn't work with the DescriptorSystem.
display_name = block.display_name
else:
display_name = get_block_display_name(block)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't get_block_display_name just check for the display_name attribute with fallback? Why does it break?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, when would usage_id not be a BlockUsageLocator?

data = {
"block_id": str(block.scope_ids.usage_id),
"block_id": str(usage_id),
"block_type": block.scope_ids.block_type,
"display_name": get_block_display_name(block),
"display_name": display_name,
}

if "index_dictionary" in includes:
data["index_dictionary"] = block.index_dictionary()

if "student_view_data" in includes:
data["student_view_data"] = block.student_view_data() if hasattr(block, 'student_view_data') else None
if isinstance(usage_id, BlockUsageLocator):
data["student_view_data"] = render_block_view(block, 'student_view', None).content
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the content the HTML rendering? Shouldn't student_view_data always be a JSON serializable dict on the XBlocks that support it?

else:
data["student_view_data"] = block.student_view_data() if hasattr(block, 'student_view_data') else None

if "children" in includes:
data["children"] = block.children if hasattr(block, 'children') else [] # List of usage keys of children
Expand Down
32 changes: 29 additions & 3 deletions openedx/core/djangoapps/xblock/rest_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
from xblock.django.request import DjangoWebobRequest, webob_to_django_response
from xblock.fields import Scope

from lms.djangoapps.courseware.block_render import get_block_by_usage_id
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import BlockUsageLocator
from openedx.core.lib.api.view_utils import view_auth_classes
from ..api import (
get_block_display_name,
Expand Down Expand Up @@ -52,7 +54,13 @@ def block_metadata(request, usage_key_str):
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e

block = load_block(usage_key, request.user)
if isinstance(usage_key, BlockUsageLocator):
block, __ = get_block_by_usage_id(
request, str(usage_key.course_key), str(usage_key), disable_staff_debug_info=True
)
else:
block = load_block(usage_key, request.user)

includes = request.GET.get("include", "").split(",")
metadata_dict = get_block_metadata(block, includes=includes)
if 'children' in metadata_dict:
Expand All @@ -74,7 +82,13 @@ def render_block_view(request, usage_key_str, view_name):
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e

block = load_block(usage_key, request.user)
if isinstance(usage_key, BlockUsageLocator):
block, __ = get_block_by_usage_id(
request, str(usage_key.course_key), str(usage_key), disable_staff_debug_info=True
)
else:
block = load_block(usage_key, request.user)

fragment = _render_block_view(block, view_name, request.user)
response_data = get_block_metadata(block)
response_data.update(fragment.to_dict())
Expand Down Expand Up @@ -152,7 +166,19 @@ 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)

if isinstance(usage_key, BlockUsageLocator):
# HACK: These requests are coming from ajax calls, so they do not the authenticated HTTP client in in the
# learning MFE. We don't want to modify the signature of the get_block_by_usage_id function, so we just
# manually set the user on the request object. This is safe because the user permissions are checked above and
# the request object is not used for anything else in the function.
request.user = user
block, __ = get_block_by_usage_id(
request, str(usage_key.course_key), str(usage_key), disable_staff_debug_info=True
)
else:
block = load_block(usage_key, user)

# 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
Loading