Skip to content

Commit

Permalink
feat: provisionally support V2 libraries in LibraryContentBlock (rand…
Browse files Browse the repository at this point in the history
…omized only) (#33263)

Refactors and reworks the LibraryContentBlock so that its
sync-from-library operations are asynchronous and work with
V2 content libraries. This also required us to make
library_content block duplication asynchronous, as that
involves syncing from the source library.

For the sake of clarity, this PR includes two major method renames:

* update_children(...) -> sync_from_library(...)
* refresh_library(...) -> sync_from_library(upgrade_to_latest=True, ...)

an an XBlock HTTP handler rename:

  /refresh_children -> /upgrade_and_sync

There are still a couple issues with import or duplication
of library_content blocks referencing V2 libraries other than
latest. These will be resolved in an upcoming PR.

Part of: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3820617729/Spec+Memo+Content+Library+Authoring+Experience+V2
Follow-up work: #33640

Co-authored-by: Connor Haugh <chaugh@2u.com>
Co-authored-by: Eugene Dyudyunov <evgen.dyudyunov@raccoongang.com>
  • Loading branch information
3 people authored Nov 20, 2023
1 parent c53cf9f commit e800ae7
Show file tree
Hide file tree
Showing 33 changed files with 1,596 additions and 572 deletions.
147 changes: 84 additions & 63 deletions cms/djangoapps/contentstore/tests/test_libraries.py

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions cms/djangoapps/contentstore/views/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ def container_handler(request, usage_key_string):

# Get the status of the user's clipboard so they can paste components if they have something to paste
user_clipboard = content_staging_api.get_user_clipboard_json(request.user.id, request)
library_block_types = [problem_type['component'] for problem_type in LIBRARY_BLOCK_TYPES]
is_library_xblock = xblock.location.block_type in library_block_types

return render_to_response('container.html', {
'language_code': request.LANGUAGE_CODE,
'context_course': course, # Needed only for display of menus at top of page.
Expand All @@ -203,6 +206,7 @@ def container_handler(request, usage_key_string):
'xblock_locator': xblock.location,
'unit': unit,
'is_unit_page': is_unit_page,
'is_collapsible': is_library_xblock,
'subsection': subsection,
'section': section,
'position': index,
Expand All @@ -218,6 +222,7 @@ def container_handler(request, usage_key_string):
'templates': CONTAINER_TEMPLATES,
# Status of the user's clipboard, exactly as would be returned from the "GET clipboard" REST API.
'user_clipboard': user_clipboard,
'is_fullwidth_content': is_library_xblock,
})
else:
return HttpResponseBadRequest("Only supports HTML requests")
Expand Down
6 changes: 5 additions & 1 deletion cms/djangoapps/contentstore/views/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
wrap_xblock_aside
)

from ..utils import get_visibility_partition_info
from ..utils import get_visibility_partition_info, StudioPermissionsService
from .access import get_user_role
from .session_kv_store import SessionKeyValueStore

Expand Down Expand Up @@ -198,6 +198,7 @@ def _prepare_runtime_for_preview(request, block):
deprecated_anonymous_user_id = anonymous_id_for_user(request.user, None)

services = {
"studio_user_permissions": StudioPermissionsService(request.user),
"i18n": XBlockI18nService,
'mako': mako_service,
"settings": SettingsService(),
Expand Down Expand Up @@ -310,6 +311,9 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
'is_reorderable': is_reorderable,
'can_edit': can_edit,
'can_edit_visibility': context.get('can_edit_visibility', is_course),
'is_loading': context.get('is_loading', False),
'is_selected': context.get('is_selected', False),
'selectable': context.get('selectable', False),
'selected_groups_label': selected_groups_label,
'can_add': context.get('can_add', True),
'can_move': context.get('can_move', is_course),
Expand Down
133 changes: 131 additions & 2 deletions cms/djangoapps/contentstore/views/tests/test_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ def test_get_empty_container_fragment(self):
self.assertNotRegex(html, r"wrapper-xblock[^-]+")

# Verify that the header and article tags are still added
self.assertIn('<header class="xblock-header xblock-header-vertical">', html)
self.assertIn('<header class="xblock-header xblock-header-vertical ">', html)
self.assertIn('<article class="xblock-render">', html)

def test_get_container_fragment(self):
Expand All @@ -233,7 +233,7 @@ def test_get_container_fragment(self):

# Verify that the Studio nesting wrapper has been added
self.assertIn("level-nesting", html)
self.assertIn('<header class="xblock-header xblock-header-vertical">', html)
self.assertIn('<header class="xblock-header xblock-header-vertical ">', html)
self.assertIn('<article class="xblock-render">', html)

# Verify that the Studio element wrapper has been added
Expand Down Expand Up @@ -950,6 +950,135 @@ def test_shallow_duplicate(self):
self.assertEqual(len(destination_chapter.get_children()), 0)
self.assertEqual(destination_chapter.display_name, "Source Chapter")

def test_duplicate_library_content_block(self): # pylint: disable=too-many-statements
"""
Test the LibraryContentBlock's special duplication process.
"""
store = modulestore()

# Create a library with two blocks (HTML 1 and HTML 2).
# These are the "Original" version of the blocks.
lib = LibraryFactory()
BlockFactory(
parent=lib,
category="html",
display_name="HTML 1 Title (Original)",
data="HTML 1 Content (Original)",
publish_item=False,
)
BlockFactory(
parent=lib,
category="html",
display_name="HTML 2 Title (Original)",
data="HTML 2 Content (Original)",
publish_item=False,
)
original_lib_version = store.get_library(
lib.location.library_key, remove_version=False, remove_branch=False,
).location.library_key.version_guid
assert original_lib_version is not None

# Create a library content block (lc), point it out our library, and sync it.
unit = BlockFactory(
parent_location=self.seq_usage_key,
category="vertical",
display_name="Parent Unit of LC and its Dupe",
publish_item=False,
)
lc = BlockFactory(
parent=unit,
category="library_content",
source_library_id=str(lib.location.library_key),
display_name="LC Block",
max_count=1,
publish_item=False,
)
lc.sync_from_library()
lc = store.get_item(lc.location) # we must reload because sync_from_library happens out-of-thread
assert lc.source_library_version == str(original_lib_version)
lc_html_1 = store.get_item(lc.children[0])
lc_html_2 = store.get_item(lc.children[1])
assert lc_html_1.display_name == "HTML 1 Title (Original)"
assert lc_html_2.display_name == "HTML 2 Title (Original)"
assert lc_html_1.data == "HTML 1 Content (Original)"
assert lc_html_2.data == "HTML 2 Content (Original)"

# Override the title and data of HTML 1 under lc ("Course Override").
# Note that title is settings-scoped and data is content-scoped.
lc_html_1.display_name = "HTML 1 Title (Course Override)"
lc_html_1.data = "HTML 1 Content (Course Override)"
store.update_item(lc_html_1, self.user.id)

# Now, update the titles and contents of both HTML 1 and HTML 2 ("Lib Update").
# This will yield a new version of the library (updated_lib_version).
lib_html_1 = store.get_item(lib.children[0])
lib_html_2 = store.get_item(lib.children[1])
assert lib_html_1.display_name == "HTML 1 Title (Original)"
assert lib_html_2.display_name == "HTML 2 Title (Original)"
lib_html_1.display_name = "HTML 1 Title (Lib Update)"
lib_html_2.display_name = "HTML 2 Title (Lib Update)"
lib_html_1.data = "HTML 1 Content (Lib Update)"
lib_html_2.data = "HTML 2 Content (Lib Update)"
store.update_item(lib_html_1, self.user.id)
store.update_item(lib_html_2, self.user.id)
updated_lib_version = store.get_library(
lib.location.library_key, remove_version=False, remove_branch=False,
).location.library_key.version_guid
assert updated_lib_version is not None
assert updated_lib_version != original_lib_version

# DUPLICATE lc.
# Unit should now contain both lc and dupe.
# All settings should match between lc and dupe.
dupe = store.get_item(
self._duplicate_item(
parent_usage_key=unit.location,
source_usage_key=lc.location,
display_name="Dupe LC Block",
)
)
lc = store.get_item(lc.location)
unit = store.get_item(unit.location)
assert unit.children == [lc.location, dupe.location]
assert len(lc.children) == len(dupe.children) == 2
assert lc.max_count == dupe.max_count == 1
assert lc.source_library_id == dupe.source_library_id == str(lib.location.library_key)
assert lc.source_library_version == dupe.source_library_version == str(original_lib_version)

# The lc block's children should remain unchanged.
# That is: HTML 1 has overrides, HTML 2 has originals.
lc_html_1 = store.get_item(lc.children[0])
assert lc_html_1.display_name == "HTML 1 Title (Course Override)"
assert lc_html_1.data == "HTML 1 Content (Course Override)"
lc_html_2 = store.get_item(lc.children[1])
assert lc_html_2.display_name == "HTML 2 Title (Original)"
assert lc_html_2.data == "HTML 2 Content (Original)"

# Now, the dupe's children should copy *settings* overrides over from the lc block,
# but we don't actually expect it to copy *content* overrides over from the lc block.
# (Yes, this is weird. One would expect it to copy all fields from the lc block, whether settings or content.
# But that's the existing behavior, so we're going to test for it, for now at least.
# We may change this in the future: https://github.com/openedx/edx-platform/issues/33739)
dupe_html_1 = store.get_item(dupe.children[0])
assert dupe_html_1.display_name == "HTML 1 Title (Course Override)" # <- as you'd expect
assert dupe_html_1.data == "HTML 1 Content (Original)" # <- weird!
dupe_html_2 = store.get_item(dupe.children[1])
assert dupe_html_2.display_name == "HTML 2 Title (Original)" # <- as you'd expect
assert dupe_html_2.data == "HTML 2 Content (Original)" # <- as you'd expect

# Finally, upgrade the dupe's library version, and make sure it pulls in updated library block *content*,
# whilst preserving *settings overrides* (specifically, HTML 1's title override).
dupe.sync_from_library(upgrade_to_latest=True)
dupe = store.get_item(dupe.location)
assert dupe.source_library_version == str(updated_lib_version)
assert len(dupe.children) == 2
dupe_html_1 = store.get_item(dupe.children[0])
dupe_html_2 = store.get_item(dupe.children[1])
assert dupe_html_1.display_name == "HTML 1 Title (Course Override)"
assert dupe_html_1.data == "HTML 1 Content (Lib Update)"
assert dupe_html_2.display_name == "HTML 2 Title (Lib Update)"
assert dupe_html_2.data == "HTML 2 Content (Lib Update)"


@ddt.ddt
class TestMoveItem(ItemTest):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def test_library_content_preview_html(self):
self.assertEqual(len(lc_block.children), 0)

# Refresh children to be reflected in lc_block
lc_block = self._refresh_children(lc_block)
lc_block = self._upgrade_and_sync(lc_block)
self.assertEqual(len(lc_block.children), 1)

self.validate_preview_html(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1112,7 +1112,7 @@ def _setup_source_course_with_library_content(self, publish=False, version=None)
lc_block = self._add_library_content_block(
vertical, self.lib_key, publish_item=publish, other_settings=dict(source_library_version=version)
)
self._refresh_children(lc_block)
lc_block.sync_from_library(upgrade_to_latest=True)

def get_lib_content_block_children(self, block_location):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@
contentstore/views/block.py to this file, because the logic is reused in another view now.
Along with it, we moved the business logic of the other views in that file, since that is related.
"""

import logging
from datetime import datetime
from uuid import uuid4

from attrs import asdict
from django.conf import settings
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import (User) # lint-amnesty, pylint: disable=imported-auth-user
from django.contrib.auth.models import User # pylint: disable=imported-auth-user
from django.core.exceptions import PermissionDenied
from django.http import HttpResponse, HttpResponseBadRequest
from django.utils.timezone import timezone
Expand Down Expand Up @@ -57,37 +56,15 @@
from openedx.core.lib.gating import api as gating_api
from openedx.core.lib.cache_utils import request_cached
from openedx.core.toggles import ENTRANCE_EXAMS
from xmodule.course_block import (
DEFAULT_START_DATE,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.library_tools import (
LibraryToolsService,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore import (
EdxJSONEncoder,
ModuleStoreEnum,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import (
modulestore,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.draft_and_published import (
DIRECT_ONLY_CATEGORIES,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import (
InvalidLocationError,
ItemNotFoundError,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.inheritance import (
own_metadata,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.services import (
ConfigurationService,
SettingsService,
TeamsConfigurationService,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.tabs import (
CourseTabList,
) # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.course_block import DEFAULT_START_DATE
from xmodule.library_tools import LibraryToolsService
from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES
from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError
from xmodule.modulestore.inheritance import own_metadata
from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService
from xmodule.tabs import CourseTabList

from ..utils import (
ancestor_has_staff_lock,
Expand Down Expand Up @@ -180,6 +157,7 @@ def handle_xblock(request, usage_key_string=None):
the public CMS API.
"""
if usage_key_string:

usage_key = usage_key_with_run(usage_key_string)

access_check = (
Expand Down Expand Up @@ -220,15 +198,15 @@ def handle_xblock(request, usage_key_string=None):
_delete_item(usage_key, request.user)
return JsonResponse()
else: # Since we have a usage_key, we are updating an existing xblock.
return modify_xblock(usage_key, request)
modified_xblock = modify_xblock(usage_key, request)
return modified_xblock

elif request.method in ("PUT", "POST"):
if "duplicate_source_locator" in request.json:
parent_usage_key = usage_key_with_run(request.json["parent_locator"])
duplicate_source_usage_key = usage_key_with_run(
request.json["duplicate_source_locator"]
)

source_course = duplicate_source_usage_key.course_key
dest_course = parent_usage_key.course_key
if not has_studio_write_access(
Expand All @@ -255,6 +233,7 @@ def handle_xblock(request, usage_key_string=None):
request.user,
request.json.get("display_name"),
)

return JsonResponse(
{
"locator": str(dest_usage_key),
Expand Down Expand Up @@ -298,7 +277,6 @@ def handle_xblock(request, usage_key_string=None):

def modify_xblock(usage_key, request):
request_data = request.json
print(f'In modify_xblock with data = {request_data.get("data")}, fields = {request_data.get("fields")}')
return _save_xblock(
request.user,
get_xblock(usage_key, request.user),
Expand Down Expand Up @@ -360,21 +338,27 @@ def load_services_for_studio(runtime, user):
def _update_with_callback(xblock, user, old_metadata=None, old_content=None):
"""
Updates the xblock in the modulestore.
But before doing so, it calls the xblock's editor_saved callback function.
But before doing so, it calls the xblock's editor_saved callback function,
and after doing so, it calls the xblock's post_editor_saved callback function.
TODO: Remove getattrs from this function.
See https://github.com/openedx/edx-platform/issues/33715
"""
if callable(getattr(xblock, "editor_saved", None)):
if old_metadata is None:
old_metadata = own_metadata(xblock)
if old_content is None:
old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content)
if old_metadata is None:
old_metadata = own_metadata(xblock)
if old_content is None:
old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content)
if hasattr(xblock, "editor_saved"):
load_services_for_studio(xblock.runtime, user)
xblock.editor_saved(user, old_metadata, old_content)

# Update after the callback so any changes made in the callback will get persisted.
return modulestore().update_item(xblock, user.id)
xblock_updated = modulestore().update_item(xblock, user.id)
if hasattr(xblock_updated, "post_editor_saved"):
load_services_for_studio(xblock_updated.runtime, user)
xblock_updated.post_editor_saved(user, old_metadata, old_content)
return xblock_updated


def _save_xblock( # lint-amnesty, pylint: disable=too-many-statements
def _save_xblock(
user,
xblock,
data=None,
Expand All @@ -389,12 +373,11 @@ def _save_xblock( # lint-amnesty, pylint: disable=too-many-statements
publish=None,
fields=None,
summary_configuration_enabled=None,
):
): # lint-amnesty, pylint: disable=too-many-statements
"""
Saves xblock w/ its fields. Has special processing for grader_type, publish, and nullout and Nones in metadata.
nullout means to truly set the field to None whereas nones in metadata mean to unset them (so they revert
to default).
"""
store = modulestore()
# Perform all xblock changes within a (single-versioned) transaction
Expand Down Expand Up @@ -881,6 +864,8 @@ def _duplicate_block(
# Allow an XBlock to do anything fancy it may need to when duplicated from another block.
# These blocks may handle their own children or parenting if needed. Let them return booleans to
# let us know if we need to handle these or not.
# TODO: Make this a proper method in the base class so we don't need getattr.
# See https://github.com/openedx/edx-platform/issues/33715
load_services_for_studio(dest_block.runtime, user)
children_handled = dest_block.studio_post_duplicate(store, source_item)

Expand Down
Loading

0 comments on commit e800ae7

Please sign in to comment.