Skip to content

Commit

Permalink
WIP - don't use separate implementations for v2 vs. v1
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Jun 1, 2024
1 parent fe71266 commit 5968b86
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 48 deletions.
7 changes: 3 additions & 4 deletions lms/djangoapps/courseware/unit_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from rest_framework.decorators import api_view
from rest_framework.exceptions import ValidationError
from xblock.fields import Scope
from xblock.core import XBlock2
from xblock.core import XBlock, XBlock2Mixin

from lms.djangoapps.course_blocks.api import get_course_blocks
from lms.djangoapps.courseware.models import StudentModule, XModuleUserStateSummaryField
Expand Down Expand Up @@ -66,11 +66,10 @@ def get_unit_blocks(request, usage_id):
"block_type": usage_key.block_type,
}
try:
block_class = XBlock2.load_class(usage_key.block_type, fallback_to_v1=True)
if issubclass(block_class, XBlock2):
block_class = XBlock.load_class(usage_key.block_type)
if issubclass(block_class, XBlock2Mixin):
block_data_out["xblock_api_version"] = 2
block_data_out["content_fields"] = {}
block_data_out["system_fields"] = {}
block_data_out["user_fields"] = {}

def add_field(field_name, value):
Expand Down
7 changes: 1 addition & 6 deletions lms/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from openedx.core.apidocs import api_info
from openedx.core.djangoapps.auth_exchange.views import LoginWithAccessTokenView
from openedx.core.djangoapps.catalog.models import CatalogIntegration
from openedx.core.djangoapps.common_views.xblock import xblock_resource, xblock_resource_v2
from openedx.core.djangoapps.common_views.xblock import xblock_resource
from openedx.core.djangoapps.cors_csrf import views as cors_csrf_views
from openedx.core.djangoapps.course_groups import views as course_groups_views
from openedx.core.djangoapps.debug import views as openedx_debug_views
Expand Down Expand Up @@ -354,11 +354,6 @@
xblock_resource,
name='xblock_resource_url',
),
re_path(
r'xblock/resource-v2/(?P<block_type>[^/]+)/(?P<uri>.*)$',
xblock_resource_v2,
name='xblock_resource_url_v2',
),

# New (Learning-Core-based) XBlock REST API
path('', include(('openedx.core.djangoapps.xblock.rest_api.urls', 'openedx.core.djangoapps.xblock'),
Expand Down
14 changes: 2 additions & 12 deletions openedx/core/djangoapps/common_views/xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,14 @@
log = logging.getLogger(__name__)


def xblock_resource(request, block_type, uri, xblock_version=1): # pylint: disable=unused-argument
def xblock_resource(request, block_type, uri): # pylint: disable=unused-argument
"""
Return a package resource for the specified XBlock.
"""
try:
# Figure out what the XBlock class is from the block type, and
# then open whatever resource has been requested.
if xblock_version == 2:
xblock_class = XBlock2.load_class(block_type)
else:
xblock_class = XBlock.load_class(block_type)
xblock_class = XBlock.load_class(block_type)
content = xblock_class.open_local_resource(uri)
except OSError:
log.info('Failed to load xblock resource', exc_info=True)
Expand All @@ -33,10 +30,3 @@ def xblock_resource(request, block_type, uri, xblock_version=1): # pylint: disa

mimetype, _ = mimetypes.guess_type(uri)
return HttpResponse(content, content_type=mimetype)


def xblock_resource_v2(request, block_type, uri):
"""
Return a package resource for the specified v2 XBlock.
"""
return xblock_resource(request, block_type, uri, xblock_version=2)
8 changes: 1 addition & 7 deletions xmodule/html_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,17 +353,11 @@ def index_dictionary(self):


@edxnotes
class HtmlBlock(HtmlBlockMixin): # lint-amnesty, pylint: disable=abstract-method
class HtmlBlock(XBlock2Mixin, HtmlBlockMixin): # lint-amnesty, pylint: disable=abstract-method
"""
This is the actual HTML XBlock.
Nothing extra is required; this is just a wrapper to include edxnotes support.
"""


class HtmlBlockV2(XBlock2Mixin, HtmlBlockMixin):
"""
The new version of the HTML block.
"""
resources_dir = "assets/html"


Expand Down
44 changes: 25 additions & 19 deletions xmodule/x_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from web_fragments.fragment import Fragment
from webob import Response
from webob.multidict import MultiDict
from xblock.core import XBlock, XBlockAside
from xblock.core import XBlock, XBlock2Mixin, XBlockAside
from xblock.fields import (
Dict,
Float,
Expand Down Expand Up @@ -586,28 +586,34 @@ def bind_for_student(self, user_id, wrappers=None):
if getattr(self.runtime, 'position', None):
self.position = self.runtime.position # update the position of the tab
return

if isinstance(self, XBlock2Mixin):
if self.scope_ids.user_id is not None:
raise RuntimeError("v2 XBlocks cannot be rebound to a different user")
# Update scope_ids to point to the new user.
self.scope_ids = self.scope_ids._replace(user_id=user_id)
else:
# If we are switching users mid-request, save the data from the old user.
self.save()

# If we are switching users mid-request, save the data from the old user.
self.save()

# Update scope_ids to point to the new user.
self.scope_ids = self.scope_ids._replace(user_id=user_id)
# Update scope_ids to point to the new user.
self.scope_ids = self.scope_ids._replace(user_id=user_id)

# Clear out any cached instantiated children.
self.clear_child_cache()
# Clear out any cached instantiated children.
self.clear_child_cache()

# Clear out any cached field data scoped to the old user.
for field in self.fields.values(): # lint-amnesty, pylint: disable=no-member
if field.scope in (Scope.parent, Scope.children):
continue
# Clear out any cached field data scoped to the old user.
for field in self.fields.values(): # lint-amnesty, pylint: disable=no-member
if field.scope in (Scope.parent, Scope.children):
continue

if field.scope.user == UserScope.ONE:
field._del_cached_value(self) # pylint: disable=protected-access
# not the most elegant way of doing this, but if we're removing
# a field from the module's field_data_cache, we should also
# remove it from its _dirty_fields
if field in self._dirty_fields:
del self._dirty_fields[field]
if field.scope.user == UserScope.ONE:
field._del_cached_value(self) # pylint: disable=protected-access
# not the most elegant way of doing this, but if we're removing
# a field from the module's field_data_cache, we should also
# remove it from its _dirty_fields
if field in self._dirty_fields:
del self._dirty_fields[field]

if wrappers:
# Put user-specific wrappers around the field-data service for this block.
Expand Down

0 comments on commit 5968b86

Please sign in to comment.