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: provisionally support V2 libraries in LibraryContentBlock (randomized only) #33263

Merged
merged 78 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
78 commits
Select commit Hold shift + click to select a range
d81d774
feat!: remove LibrarySourcedBlock
kdmccormick Sep 14, 2023
9eee194
feat: support V2 libraries in LibraryContentBlock (randomized only)
kdmccormick Sep 14, 2023
258216b
Merge branch 'master' into kdmccormick/library-content-v2
kdmccormick Sep 15, 2023
ef54bd6
fix: user tasks out of lms, move tasks.py
connorhaugh Sep 18, 2023
9b0b722
Merge commit 'refs/pull/33263/head' of github.com:openedx/edx-platfor…
connorhaugh Sep 18, 2023
1daf629
fix: lint
connorhaugh Sep 19, 2023
982abc7
fix: lint
connorhaugh Sep 19, 2023
0dbe410
fix: lint fix
connorhaugh Sep 19, 2023
3299cef
fix: move codeowner attribute to layer
connorhaugh Sep 19, 2023
109a33c
fix: remove codeowner attribute from update_children_task
kdmccormick Oct 5, 2023
4b4a34e
build: put back set_code_owner_attribute but as a regular function call
kdmccormick Oct 5, 2023
9b2f0f0
feat: add cms-only assertions to content_libraries tasks
kdmccormick Oct 5, 2023
0f730f8
Merge branch 'master' into kdmccormick/library-content-v2
kdmccormick Oct 17, 2023
fccbaa4
style: fix indentation
kdmccormick Oct 17, 2023
6dcb047
fix: add missing imports for _assert_cms
kdmccormick Oct 17, 2023
f2230db
docs: remove outdated is_v2_lib from function docstring
kdmccormick Oct 17, 2023
e25e2a3
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 18, 2023
f1370d0
test: run ./xmodule/ LibraryContentTests only in CMS
kdmccormick Oct 18, 2023
8fe86a1
test: run ./xmodule/ tests with CMS settings
kdmccormick Oct 18, 2023
71cde02
Revert "test: run ./xmodule/ LibraryContentTests only in CMS"
kdmccormick Oct 18, 2023
9f9c047
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 18, 2023
42e82ba
Merge branch 'kdmccormick/test-xmodule-with-cms' into kdmccormick/lib…
kdmccormick Oct 18, 2023
3a25800
test: run ./xmodule/ LibraryContentTests only in CMS
kdmccormick Oct 18, 2023
8ac429a
style: remove duplicate imports fomr test_lib_tools
kdmccormick Oct 18, 2023
3cf5fc3
test: fix patching in test_list_available_libraries
kdmccormick Oct 18, 2023
caecac1
test: attempt at stricter enforcement of library tools in cms only
kdmccormick Oct 18, 2023
ae73343
Merge branch 'master' into kdmccormick/library-content-v2
kdmccormick Oct 19, 2023
2241ca5
temp: fix: put library_tools back in LMS, at least for now
kdmccormick Oct 19, 2023
4379f0f
test: fix skip_unless_[lms|cms]
kdmccormick Oct 19, 2023
e911056
test: fix library_content completion test w/ settings hack
kdmccormick Oct 19, 2023
1fadf7a
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 19, 2023
0809ad4
test: update test patch for new libraries tasks location
kdmccormick Oct 19, 2023
22cb7d5
test: use string mock to satisfy importlinter
kdmccormick Oct 19, 2023
2609a43
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 19, 2023
9c23889
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 23, 2023
0cf735f
fix: getting status of library_content's update_children task
kdmccormick Oct 24, 2023
3dfa080
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 24, 2023
535fb52
fix: squash: _are_children_updating utiltity func
kdmccormick Oct 24, 2023
2c69e81
Revert "fix: squash: _are_children_updating utiltity func"
kdmccormick Oct 24, 2023
781cdf5
Revert "fix: getting status of library_content's update_children task"
kdmccormick Oct 24, 2023
b9fc30d
fix: getting status of library_content's update_children task
kdmccormick Oct 24, 2023
c609b0b
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 26, 2023
35db46c
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Oct 27, 2023
455424e
Merge branch 'master' into kdmccormick/library-content-v2
kdmccormick Oct 30, 2023
6666243
refactor: delete unused library_tools code; rename update_children
kdmccormick Oct 31, 2023
65e9599
test: update unit tests for previous refactor commit
kdmccormick Oct 31, 2023
3d9b305
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 2, 2023
e96f8c8
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 3, 2023
d8baa20
fix: duplicate library_content children asynchronously (#33652)
kdmccormick Nov 6, 2023
c44d361
fix: bad user_id kwarg in call to trigger_duplicate_children
kdmccormick Nov 6, 2023
74f711f
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 6, 2023
9dfa9ef
test: reorder library_content test_duplicate_version assertions for b…
kdmccormick Nov 6, 2023
0e3cd00
fix: various fixes relating to library version awareness (WIP)
kdmccormick Nov 6, 2023
2298803
fix: fail better when library_tools is missing
kdmccormick Nov 8, 2023
3ab2d2a
refactor: rename to be clearer when we're upgrading lib versions vs j…
kdmccormick Nov 8, 2023
2f0bac1
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 8, 2023
8432225
test: change 'refresh' to 'reselect' in library_content test name
kdmccormick Nov 8, 2023
7a40ee4
fix: differentiate upgrade_and_sync http handler from its internal me…
kdmccormick Nov 8, 2023
eff235a
fix: make get_latest_library_version always return str
kdmccormick Nov 8, 2023
82a8b42
fix: use get_tools() consistently. also, turn upgrade_to_latest into …
kdmccormick Nov 8, 2023
54e12ad
test(libraries): only sync when necessary, and only upgrade when nece…
kdmccormick Nov 8, 2023
776e2bd
fix: don't stringify lib version when it's None
kdmccormick Nov 8, 2023
2a5c85e
fix: lib version type annoations; also remove extraneous update_items
kdmccormick Nov 8, 2023
f7d2970
test: fix setup for import/export tests
kdmccormick Nov 13, 2023
e0c5d27
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 13, 2023
2fe32eb
fix: post_editor_saved should only apply when source lib or capa type…
kdmccormick Nov 15, 2023
6398c35
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 15, 2023
0d01595
fix: correct post_editor saved (wip - test still failing
kdmccormick Nov 15, 2023
195bd83
fix: return 400 (not 500) when upgrade_and_sync called with bad lib key
kdmccormick Nov 16, 2023
aeb8d74
test: consolidate & fix test cases around nonexistent source libraries
kdmccormick Nov 16, 2023
5b603b6
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 16, 2023
c0c5456
fix: typos in last two commits
kdmccormick Nov 16, 2023
1dba7c6
test: test_duplicate_library_content_block
kdmccormick Nov 17, 2023
c9459ef
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 17, 2023
671dd83
fix: import shouldn't fail if lib doesn't exist
kdmccormick Nov 17, 2023
03fc0dc
style: ignore pylint violation
kdmccormick Nov 20, 2023
f4ea5f2
refactor: undo accidental test case rename
kdmccormick Nov 20, 2023
10da278
Merge remote-tracking branch 'upstream/master' into kdmccormick/libra…
kdmccormick Nov 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 7 additions & 11 deletions cms/djangoapps/contentstore/tests/test_libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def _refresh_children(self, lib_content_block, status_code_expected=200):
lib_content_block.runtime._services['user'] = user_service # pylint: disable=protected-access

handler_url = reverse_usage_url(
'component_handler',
'preview_handler',
lib_content_block.location,
kwargs={'handler': 'refresh_children'}
)
Expand Down Expand Up @@ -359,8 +359,6 @@ def test_change_after_first_sync(self):
self.assertEqual(resp.status_code, 200)
lc_block = modulestore().get_item(lc_block.location)
self.assertEqual(len(lc_block.children), 1) # Children should not be deleted due to a bad setting.
html_block = modulestore().get_item(lc_block.children[0])
self.assertEqual(html_block.data, data_value)

def test_refreshes_children_if_libraries_change(self):
""" Tests that children are automatically refreshed if libraries list changes """
Expand Down Expand Up @@ -406,7 +404,7 @@ def test_refreshes_children_if_libraries_change(self):
html_block = modulestore().get_item(lc_block.children[0])
self.assertEqual(html_block.data, data2)

@patch("xmodule.library_tools.SearchEngine.get_search_engine", Mock(return_value=None, autospec=True))
@patch("xmodule.tasks.SearchEngine.get_search_engine", Mock(return_value=None, autospec=True))
def test_refreshes_children_if_capa_type_change(self):
""" Tests that children are automatically refreshed if capa type field changes """
name1, name2 = "Option Problem", "Multiple Choice Problem"
Expand Down Expand Up @@ -993,24 +991,22 @@ def test_duplicated_version(self):
self.library = store.get_library(self.lib_key)

# Refresh our reference to the block
self.lc_block = store.get_item(self.lc_block.location)
self.lc_block = self._refresh_children(self.lc_block)
self.problem_in_course = store.get_item(self.problem_in_course.location)

# The library has changed...
self.assertEqual(len(self.library.children), 2)

# But the block hasn't.
self.assertEqual(len(self.lc_block.children), 1)
self.assertEqual(self.problem_in_course.location, self.lc_block.children[0])
self.assertEqual(self.problem_in_course.display_name, self.original_display_name)
connorhaugh marked this conversation as resolved.
Show resolved Hide resolved
# and the block has changed too.
self.assertEqual(len(self.lc_block.children), 2)

# Duplicate self.lc_block:
duplicate = store.get_item(
duplicate_block(self.course.location, self.lc_block.location, self.user)
)
# The duplicate should have identical children to the original:
self.assertEqual(len(duplicate.children), 1)
self.assertEqual(len(duplicate.children), 2)
self.assertTrue(self.lc_block.source_library_version)
self.assertEqual(self.lc_block.source_library_version, duplicate.source_library_version)
problem2_in_course = store.get_item(duplicate.children[0])
self.assertEqual(problem2_in_course.display_name, self.original_display_name)
self.assertEqual(problem2_in_course.display_name, self.problem.display_name)
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
9 changes: 3 additions & 6 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 @@ -2675,10 +2675,7 @@ def setUp(self):
XBlockStudioConfiguration.objects.create(
name="openassessment", enabled=True, support_level="us"
)
# Library Sourced Block and Library Content block has it's own category.
XBlockStudioConfiguration.objects.create(
name="library_sourced", enabled=True, support_level="fs"
)
# Library Content block has its own category.
XBlockStudioConfiguration.objects.create(
name="library_content", enabled=True, support_level="fs"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ def handle_xblock(request, usage_key_string=None):
the public studio content API.
"""
if usage_key_string:

usage_key = usage_key_with_run(usage_key_string)

access_check = (
Expand Down Expand Up @@ -218,15 +219,16 @@ 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)
_post_editor_saved_callback(get_xblock(usage_key, request.user))
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 @@ -253,6 +255,8 @@ def handle_xblock(request, usage_key_string=None):
request.user,
request.json.get("display_name"),
)
_post_editor_saved_callback(get_xblock(dest_usage_key, request.user))

return JsonResponse(
{
"locator": str(dest_usage_key),
Expand Down Expand Up @@ -296,7 +300,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 @@ -372,7 +375,15 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None):
return modulestore().update_item(xblock, user.id)


def _save_xblock( # lint-amnesty, pylint: disable=too-many-statements
def _post_editor_saved_callback(xblock):
"""
Updates the xblock in the modulestore after saving xblock.
"""
if callable(getattr(xblock, "post_editor_saved", None)):
xblock.post_editor_saved()


def _save_xblock(
user,
xblock,
data=None,
Expand All @@ -387,12 +398,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
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved
"""
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
2 changes: 0 additions & 2 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1994,8 +1994,6 @@
]

LIBRARY_BLOCK_TYPES = [
# Per https://github.com/openedx/build-test-release-wg/issues/231
# we removed the library source content block from defaults until complete.
{
'component': 'library_content',
'boilerplate_name': None
Expand Down
7 changes: 5 additions & 2 deletions cms/lib/xblock/tagging/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,12 @@ def test_preview_html(self):
tree = etree.parse(StringIO(problem_html), parser)

main_div_nodes = tree.xpath('/html/body/div/section/div')
self.assertEqual(len(main_div_nodes), 1)
self.assertEqual(len(main_div_nodes), 2)

div_node = main_div_nodes[0]
loader_div_node = main_div_nodes[0]
self.assertIn('ui-loading', loader_div_node.get('class'))

div_node = main_div_nodes[1]
self.assertEqual(div_node.get('data-init'), 'StructuredTagsInit')
self.assertEqual(div_node.get('data-runtime-class'), 'PreviewRuntime')
self.assertEqual(div_node.get('data-block-type'), 'tagging_aside')
Expand Down