Skip to content

revert: temp: let XBlock API users optionally use LabXchange block types #31315

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.

Sign up for GitHub

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

Merged
bradenmacdonald merged 2 commits into from
Jan 11, 2023
Merged

revert: temp: let XBlock API users optionally use LabXchange block types #31315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5370185
revert: temp: let XBlock API users optionally use LabXchange block types
pomegranited Nov 21, 2022
f28ba85
Merge branch 'master' into jill/remove-labxchange
pomegranited Dec 19, 2022
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
186 changes: 0 additions & 186 deletions openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,14 @@
"""
from uuid import UUID
from unittest.mock import patch
from urllib.parse import urlparse, parse_qsl

import json
import ddt
from django.conf import settings
from django.contrib.auth.models import Group
from django.test.client import Client
from django.test.utils import override_settings
from organizations.models import Organization
from rest_framework.test import APITestCase
from web_fragments.fragment import Fragment
from webob import Response
from xblock.core import XBlock

from openedx.core.djangoapps.content_libraries.libraries_index import LibraryBlockIndexer, ContentLibraryIndexer
from openedx.core.djangoapps.content_libraries.tests.base import (
Expand All @@ -26,7 +21,6 @@
URL_BLOCK_RENDER_VIEW,
URL_BLOCK_GET_HANDLER_URL,
URL_BLOCK_XBLOCK_HANDLER,
URL_LIB_BLOCK_OLX,
)
from openedx.core.djangoapps.content_libraries.constants import VIDEO, COMPLEX, PROBLEM, CC_4_BY, ALL_RIGHTS_RESERVED
from openedx.core.djangolib.blockstore_cache import cache
Expand Down Expand Up @@ -999,183 +993,3 @@ def test_not_found_fails_correctly(self):
self.assertEqual(response.json(), {
'detail': f"XBlock {valid_not_found_key} does not exist, or you don't have permission to view it.",
})


class AltBlock(XBlock):
"""Class for testing LabXchange XBlock type overrides."""
@XBlock.handler
def student_view_user_state(self, request, suffix=""):
"""
Returns a JSON response for testing.
"""
view_state = {
"id": str(self.location),
"block_type": str(self.location.block_type),
"override_type": str(self.__class__),
}
return Response(
json.dumps(view_state),
content_type='application/json',
charset='UTF-8',
)

def student_view(self, context=None):
"""
Returns an HTML fragment for testing.
"""
return Fragment(f"<div data-usage='{self.location}' data-block-type='{self.location.block_type}'>"
"<div class='AltBlock-wrapper'/></div>")


@ddt.ddt
class ContentLibrariesXBlockTypeOverrideTestMixin:
"""
Tests for Blockstore-based Content Libraries XBlock API,
where the expected XBlock type returned is overridden in the request.
"""
BLOCK_DATA = (
('block-wo-override', {}, 'video'),
('block-w-override', {'lx_block_types': '1'}, 'alt-block'),
)

def setUp(self):
super().setUp()
if settings.ENABLE_ELASTICSEARCH_FOR_TESTS:
ContentLibraryIndexer.remove_all_items()
LibraryBlockIndexer.remove_all_items()

self.olx = """
<video display_name="Test Block Type Overrides"
youtube_id_1_0="rE42zZ-3wNo"
transcripts="{&quot;en&quot;: &quot;transcript.srt&quot;}" />
""".strip()

def create_block(self, slug, block_type='video'):
"""
Add a new library containing a block, using the given slug to keep them unique.
"""
lib = self._create_library(slug=slug, title='Test Block Type Overrides', library_type=COMPLEX)
block = self._add_block_to_library(lib['id'], block_type, slug)
self._set_library_block_olx(block['id'], self.olx)
self._commit_library_changes(lib['id'])
return block['id']

@ddt.data(*BLOCK_DATA)
@ddt.unpack
@patch("openedx.core.djangoapps.xblock.rest_api.views.LX_BLOCK_TYPES_OVERRIDE", {'video': 'alt-block'})
@XBlock.register_temp_plugin(AltBlock, 'alt-block')
def test_block_type_metadata(self, slug, api_args, expected_type):
"""
Check that the metadata API returns the overridden block type.
"""
block_key = self.create_block(f"metadata-{slug}")
response = self.client.get(
URL_BLOCK_METADATA_URL.format(block_key=block_key),
api_args,
)
assert response.data['block_id'] == str(block_key)
assert response.data['block_type'] == expected_type
assert response.data['display_name'] == 'Test Block Type Overrides'

@ddt.data(*BLOCK_DATA)
@ddt.unpack
@patch("openedx.core.djangoapps.xblock.rest_api.views.LX_BLOCK_TYPES_OVERRIDE", {'video': 'alt-block'})
@XBlock.register_temp_plugin(AltBlock, 'alt-block')
def test_block_type_olx(self, slug, api_args, expected_type):
"""
Check that the OLX API is unchanged when overriding the block type.
"""
block_key = self.create_block(f"olx-{slug}")
response = self.client.get(
URL_LIB_BLOCK_OLX.format(block_key=block_key),
api_args,
)
assert response.data['olx'] == self.olx

@ddt.data(*BLOCK_DATA)
@ddt.unpack
@patch("openedx.core.djangoapps.xblock.rest_api.views.LX_BLOCK_TYPES_OVERRIDE", {'video': 'alt-block'})
@XBlock.register_temp_plugin(AltBlock, 'alt-block')
def test_block_type_render(self, slug, api_args, expected_type):
"""
Check that the rendered block HTML uses the overridden block type.
"""
block_key = self.create_block(f"render-{slug}")
response = self.client.get(
URL_BLOCK_RENDER_VIEW.format(block_key=block_key, view_name='student_view'),
api_args,
)
assert response.data['block_id'] == str(block_key)
assert response.data['block_type'] == expected_type
assert response.data['display_name'] == 'Test Block Type Overrides'
assert f"data-usage='{block_key}'" in response.data['content']
assert f"data-block-type='{expected_type}'" in response.data['content']
if expected_type == 'video':
assert 'class="video-wrapper"' in response.data['content']
else:
assert "class='AltBlock-wrapper'" in response.data['content']

@ddt.data(*BLOCK_DATA)
@ddt.unpack
@patch("openedx.core.djangoapps.xblock.rest_api.views.LX_BLOCK_TYPES_OVERRIDE", {'video': 'alt-block'})
@XBlock.register_temp_plugin(AltBlock, 'alt-block')
def test_block_type_handler(self, slug, api_args, expected_type):
# Check that the handler_url contains the block type override params
block_key = self.create_block(f"handler-{slug}")
response = self.client.get(
URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name='student_view_user_state'),
api_args,
)
handler_url = response.data['handler_url']
parsed_url = urlparse(handler_url)
parsed_qs = dict(parse_qsl(parsed_url.query))
assert parsed_qs == api_args

# Ensure the invoked handler hits the expected Block
if expected_type == 'video':
expected_response = {
'all_sources': [],
'duration': None,
'encoded_videos': {
'youtube': {
'file_size': 0,
'url': 'https://www.youtube.com/watch?v=rE42zZ-3wNo',
}
},
'only_on_web': False,
'saved_video_position': 0.0,
'speed': None,
'transcripts': {},
}
else:
expected_response = {
"id": f'lb:CL-TEST:handler-{slug}:video:handler-{slug}',
"block_type": 'video',
"override_type": "<class 'xblock.internal.AltBlockWithMixins'>",
}
response = self.client.post(handler_url).json()
# Can't match the Transcripts download URL exactly, but we can check that it's there and roughly correct
if 'transcripts' in response:
assert f"lb:CL-TEST:handler-{slug}:video:handler-{slug}" in response['transcripts']['en']
del response['transcripts']['en']
assert response == expected_response


@elasticsearch_test
class ContentLibrariesXBlockTypeOverrideBlockstoreServiceTest(
ContentLibrariesXBlockTypeOverrideTestMixin,
ContentLibrariesRestApiBlockstoreServiceTest,
):
"""
Tests for the Content Libraries XBlock API type override using the standalone Blockstore service.
"""


@elasticsearch_test
class ContentLibrariesXBlockTypeOverrideTest(
ContentLibrariesXBlockTypeOverrideTestMixin,
ContentLibrariesRestApiTest,
):
"""
Tests for the Content Libraries XBlock API type override using the installed Blockstore app.
"""
15 changes: 4 additions & 11 deletions openedx/core/djangoapps/xblock/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import logging
import threading
from urllib.parse import urlencode

from django.urls import reverse
from django.utils.translation import gettext as _
Expand Down Expand Up @@ -59,7 +58,7 @@ def get_runtime_system():
return getattr(get_runtime_system, cache_name)


def load_block(usage_key, user, block_type_overrides=None):
def load_block(usage_key, user):
"""
Load the specified XBlock for the given user.

Expand All @@ -72,8 +71,6 @@ def load_block(usage_key, user, block_type_overrides=None):
Args:
usage_key(OpaqueKey): block identifier
user(User): user requesting the block
block_type_overrides(dict): optional dict of block types to override in returned block metadata:
{'from_block_type': 'to_block_type'}
"""
# Is this block part of a course, a library, or what?
# Get the Learning Context Implementation based on the usage key
Expand All @@ -92,7 +89,7 @@ def load_block(usage_key, user, block_type_overrides=None):

runtime = get_runtime_system().get_runtime(user=user)

return runtime.get_block(usage_key, block_type_overrides=block_type_overrides)
return runtime.get_block(usage_key)


def get_block_metadata(block, includes=()):
Expand Down Expand Up @@ -222,7 +219,7 @@ def render_block_view(block, view_name, user): # pylint: disable=unused-argumen
return fragment


def get_handler_url(usage_key, handler_name, user, extra_params=None):
def get_handler_url(usage_key, handler_name, user):
"""
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 @@ -239,7 +236,6 @@ def get_handler_url(usage_key, handler_name, user, extra_params=None):
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)
extra_params - Optional extra params to append to the handler_url (dict)

This view does not check/care if the XBlock actually exists.
"""
Expand All @@ -263,11 +259,8 @@ def get_handler_url(usage_key, handler_name, user, extra_params=None):
'secure_token': secure_token,
'handler_name': handler_name,
})
qstring = urlencode(extra_params) if extra_params else ''
if qstring:
qstring = '?' + qstring
# 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
# have access to the request.
return site_root_url + path + qstring
return site_root_url + path
44 changes: 4 additions & 40 deletions openedx/core/djangoapps/xblock/rest_api/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,9 @@

User = get_user_model()

LX_BLOCK_TYPES_OVERRIDE = {
'problem': 'lx_question',
'video': 'lx_video',
'html': 'lx_html',
}


invalid_not_found_fmt = "XBlock {usage_key} does not exist, or you don't have permission to view it."


def _block_type_overrides(request_args):
"""
If the request contains the argument `lx_block_types=1`, then
returns a dict of LabXchange block types, which override the default block types.

Otherwise, returns None.

FYI: This is a temporary change, added to assist LabXchange with the transition to using their custom runtime.
"""
if request_args.get('lx_block_types'):
return LX_BLOCK_TYPES_OVERRIDE
return None


@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 All @@ -64,15 +43,13 @@ def block_metadata(request, usage_key_str):

* "include": a comma-separated list of keys to include.
Valid keys are "index_dictionary" and "student_view_data".
* "lx_block_types": optional boolean; set to use the LabXchange XBlock classes to load the requested block.
The block ID and OLX remain unchanged; they will use the original block type.
"""
try:
usage_key = UsageKey.from_string(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, block_type_overrides=_block_type_overrides(request.GET))
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 @@ -88,17 +65,13 @@ def block_metadata(request, usage_key_str):
def render_block_view(request, usage_key_str, view_name):
"""
Get the HTML, JS, and CSS needed to render the given XBlock.

Accepts the following query parameters:
* "lx_block_types": optional boolean; set to use the LabXchange XBlock classes to load the requested block.
The block ID and OLX remain unchanged; they will use the original block type.
"""
try:
usage_key = UsageKey.from_string(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, block_type_overrides=_block_type_overrides(request.GET))
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 All @@ -113,17 +86,13 @@ def get_handler_url(request, usage_key_str, handler_name):
the given XBlock handler.

The URL will expire but is guaranteed to be valid for a minimum of 2 days.

The following query parameters will be appended to the returned handler_url:
* "lx_block_types": optional boolean; set to use the LabXchange XBlock classes to load the requested block.
The block ID and OLX remain unchanged; they will use the original block type.
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise NotFound(invalid_not_found_fmt.format(usage_key=usage_key_str)) from e

handler_url = _get_handler_url(usage_key, handler_name, request.user, request.GET)
handler_url = _get_handler_url(usage_key, handler_name, request.user)
return Response({"handler_url": handler_url})


Expand All @@ -140,11 +109,6 @@ def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name,
This endpoint has a unique authentication scheme that involves a temporary
auth token included in the URL (see below). As a result it can be exempt
from CSRF, session auth, and JWT/OAuth.

Accepts the following query parameters (in addition to those passed to the handler):

* "lx_block_types": optional boolean; set to use the LabXchange XBlock classes to load the requested block.
The block ID and OLX remain unchanged; they will use the original block type.
"""
try:
usage_key = UsageKey.from_string(usage_key_str)
Expand Down Expand Up @@ -185,7 +149,7 @@ 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_type_overrides=_block_type_overrides(request.GET))
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