diff --git a/cms/djangoapps/contentstore/core/course_optimizer_provider.py b/cms/djangoapps/contentstore/core/course_optimizer_provider.py index 66353980d5d2..8d40b958f08e 100644 --- a/cms/djangoapps/contentstore/core/course_optimizer_provider.py +++ b/cms/djangoapps/contentstore/core/course_optimizer_provider.py @@ -8,6 +8,8 @@ from cms.djangoapps.contentstore.tasks import CourseLinkCheckTask, LinkState from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import get_xblock from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import usage_key_with_run +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore # Restricts status in the REST API to only those which the requesting user has permission to view. @@ -285,3 +287,26 @@ def _create_dto_recursive(xblock_node, xblock_dictionary): xblock_children.append(xblock_entry) return {level: xblock_children} if level else None + + +def sort_course_sections(course_key, data): + """Retrieve and sort course sections based on the published course structure.""" + course_blocks = modulestore().get_items( + course_key, + qualifiers={'category': 'course'}, + revision=ModuleStoreEnum.RevisionOption.published_only + ) + + if not course_blocks or 'LinkCheckOutput' not in data or 'sections' not in data['LinkCheckOutput']: + return data # Return unchanged data if course_blocks or required keys are missing + + sorted_section_ids = [section.location.block_id for section in course_blocks[0].get_children()] + + sections_map = {section['id']: section for section in data['LinkCheckOutput']['sections']} + data['LinkCheckOutput']['sections'] = [ + sections_map[section_id] + for section_id in sorted_section_ids + if section_id in sections_map + ] + + return data diff --git a/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py b/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py index 72bd4120321e..ca0b73af71da 100644 --- a/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py +++ b/cms/djangoapps/contentstore/core/tests/test_course_optimizer_provider.py @@ -1,12 +1,14 @@ """ Tests for course optimizer """ +from unittest import mock from unittest.mock import Mock from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.core.course_optimizer_provider import ( _update_node_tree_and_dictionary, - _create_dto_recursive + _create_dto_recursive, + sort_course_sections ) from cms.djangoapps.contentstore.tasks import LinkState @@ -222,3 +224,74 @@ def test_create_dto_recursive_returns_for_full_tree(self): expected = _create_dto_recursive(mock_node_tree, mock_dictionary) self.assertEqual(expected_result, expected) + + @mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True) + def test_returns_unchanged_data_if_no_course_blocks(self, mock_modulestore): + """Test that the function returns unchanged data if no course blocks exist.""" + mock_modulestore_instance = Mock() + mock_modulestore.return_value = mock_modulestore_instance + mock_modulestore_instance.get_items.return_value = [] + + data = {} + result = sort_course_sections("course-v1:Test+Course", data) + assert result == data # Should return the original data + + @mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True) + def test_returns_unchanged_data_if_linkcheckoutput_missing(self, mock_modulestore): + """Test that the function returns unchanged data if 'LinkCheckOutput' is missing.""" + + mock_modulestore_instance = Mock() + mock_modulestore.return_value = mock_modulestore_instance + + data = {'LinkCheckStatus': 'Uninitiated'} # No 'LinkCheckOutput' + mock_modulestore_instance.get_items.return_value = data + + result = sort_course_sections("course-v1:Test+Course", data) + assert result == data + + @mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True) + def test_returns_unchanged_data_if_sections_missing(self, mock_modulestore): + """Test that the function returns unchanged data if 'sections' is missing.""" + + mock_modulestore_instance = Mock() + mock_modulestore.return_value = mock_modulestore_instance + + data = {'LinkCheckStatus': 'Success', 'LinkCheckOutput': {}} # No 'LinkCheckOutput' + mock_modulestore_instance.get_items.return_value = data + + result = sort_course_sections("course-v1:Test+Course", data) + assert result == data + + @mock.patch('cms.djangoapps.contentstore.core.course_optimizer_provider.modulestore', autospec=True) + def test_sorts_sections_correctly(self, mock_modulestore): + """Test that the function correctly sorts sections based on published course structure.""" + + mock_course_block = Mock() + mock_course_block.get_children.return_value = [ + Mock(location=Mock(block_id="section2")), + Mock(location=Mock(block_id="section3")), + Mock(location=Mock(block_id="section1")), + ] + + mock_modulestore_instance = Mock() + mock_modulestore.return_value = mock_modulestore_instance + mock_modulestore_instance.get_items.return_value = [mock_course_block] + + data = { + "LinkCheckOutput": { + "sections": [ + {"id": "section1", "name": "Intro"}, + {"id": "section2", "name": "Advanced"}, + {"id": "section3", "name": "Bonus"}, # Not in course structure + ] + } + } + + result = sort_course_sections("course-v1:Test+Course", data) + expected_sections = [ + {"id": "section2", "name": "Advanced"}, + {"id": "section3", "name": "Bonus"}, + {"id": "section1", "name": "Intro"}, + ] + + assert result["LinkCheckOutput"]["sections"] == expected_sections diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index d2e7023c217f..333cdbeb76ee 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -82,6 +82,22 @@ def is_unit(xblock, parent_xblock=None): return False +def is_library_content(xblock): + """ + Returns true if the specified xblock is library content. + """ + return xblock.category == 'library_content' + + +def get_parent_if_split_test(xblock): + """ + Returns the parent of the specified xblock if it is a split test, otherwise returns None. + """ + parent_xblock = get_parent_xblock(xblock) + if parent_xblock and parent_xblock.category == 'split_test': + return parent_xblock + + def xblock_has_own_studio_page(xblock, parent_xblock=None): """ Returns true if the specified xblock has an associated Studio page. Most xblocks do diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py b/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py index 9aa23838e6cf..24c8dd0d18f8 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py @@ -6,7 +6,7 @@ from rest_framework.response import Response from user_tasks.models import UserTaskStatus -from cms.djangoapps.contentstore.core.course_optimizer_provider import get_link_check_data +from cms.djangoapps.contentstore.core.course_optimizer_provider import get_link_check_data, sort_course_sections from cms.djangoapps.contentstore.rest_api.v0.serializers.course_optimizer import LinkCheckSerializer from cms.djangoapps.contentstore.tasks import check_broken_links from common.djangoapps.student.auth import has_course_author_access, has_studio_read_access @@ -139,6 +139,7 @@ def get(self, request: Request, course_id: str): self.permission_denied(request) data = get_link_check_data(request, course_id) - serializer = LinkCheckSerializer(data) + data = sort_course_sections(course_key, data) + serializer = LinkCheckSerializer(data) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index d9bf6a9b1fcf..fc588065cff6 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -1369,13 +1369,13 @@ def _filter_by_status(results): retry_list = [] for result in results: status, block_id, url = result['status'], result['block_id'], result['url'] - if status is None: + if status is None and _is_studio_url(url): retry_list.append([block_id, url]) elif status == 200: continue elif status == 403 and _is_studio_url(url): filtered_results.append([block_id, url, LinkState.LOCKED]) - elif status == 403 and not _is_studio_url(url): + elif status in [403, None] and not _is_studio_url(url): filtered_results.append([block_id, url, LinkState.EXTERNAL_FORBIDDEN]) else: filtered_results.append([block_id, url, LinkState.BROKEN]) diff --git a/cms/djangoapps/contentstore/tests/test_tasks.py b/cms/djangoapps/contentstore/tests/test_tasks.py index 98d2b2f008e2..103e24deb821 100644 --- a/cms/djangoapps/contentstore/tests/test_tasks.py +++ b/cms/djangoapps/contentstore/tests/test_tasks.py @@ -498,18 +498,20 @@ def test_filter_by_status(self): {'status': 200, 'block_id': 'block1', 'url': 'https://example.com'}, {'status': None, 'block_id': 'block2', 'url': 'https://retry.com'}, {'status': 403, 'block_id': 'block3', 'url': 'https://' + settings.CMS_BASE}, + {'status': None, 'block_id': 'block3', 'url': 'https://' + settings.CMS_BASE}, {'status': 403, 'block_id': 'block4', 'url': 'https://external.com'}, {'status': 404, 'block_id': 'block5', 'url': 'https://broken.com'} ] expected_filtered_results = [ + ['block2', 'https://retry.com', LinkState.EXTERNAL_FORBIDDEN], ['block3', 'https://' + settings.CMS_BASE, LinkState.LOCKED], ['block4', 'https://external.com', LinkState.EXTERNAL_FORBIDDEN], ['block5', 'https://broken.com', LinkState.BROKEN], ] expected_retry_list = [ - ['block2', 'https://retry.com'] + ['block3', 'https://' + settings.CMS_BASE] ] filtered_results, retry_list = _filter_by_status(results) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 914c07884665..50aaaaded7a6 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -25,7 +25,11 @@ from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.xblock_django.api import authorable_xblocks, disabled_xblocks from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag -from cms.djangoapps.contentstore.helpers import is_unit +from cms.djangoapps.contentstore.helpers import ( + get_parent_if_split_test, + is_unit, + is_library_content, +) from cms.djangoapps.contentstore.toggles import ( libraries_v1_enabled, libraries_v2_enabled, @@ -148,11 +152,12 @@ def container_handler(request, usage_key_string): # pylint: disable=too-many-st except ItemNotFoundError: return HttpResponseBadRequest() - is_unit_page = is_unit(xblock) - unit = xblock if is_unit_page else None + if use_new_unit_page(course.id): + if is_unit(xblock) or is_library_content(xblock): + return redirect(get_unit_url(course.id, xblock.location)) - if is_unit_page and use_new_unit_page(course.id): - return redirect(get_unit_url(course.id, unit.location)) + if split_xblock := get_parent_if_split_test(xblock): + return redirect(get_unit_url(course.id, split_xblock.location)) container_handler_context = get_container_handler_context(request, usage_key, course, xblock) container_handler_context.update({ diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 9a5efe70fc21..19e20804c2fb 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -611,6 +611,7 @@ def _create_block(request): modulestore().update_item(created_block, request.user.id) response["upstreamRef"] = upstream_ref response["static_file_notices"] = asdict(static_file_notices) + response["parent_locator"] = parent_locator return JsonResponse(response) diff --git a/cms/envs/common.py b/cms/envs/common.py index 59eca17b8133..5d4dea7c2359 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1879,6 +1879,7 @@ "openedx_learning.apps.authoring.components", "openedx_learning.apps.authoring.contents", "openedx_learning.apps.authoring.publishing", + "openedx_learning.apps.authoring.units", ] diff --git a/cms/static/images/advanced-icon.svg b/cms/static/images/advanced-icon.svg new file mode 100644 index 000000000000..86096aefd236 --- /dev/null +++ b/cms/static/images/advanced-icon.svg @@ -0,0 +1,6 @@ + diff --git a/cms/static/images/drag-and-drop-v2-icon.svg b/cms/static/images/drag-and-drop-v2-icon.svg new file mode 100644 index 000000000000..733fb744d7b3 --- /dev/null +++ b/cms/static/images/drag-and-drop-v2-icon.svg @@ -0,0 +1,6 @@ + diff --git a/cms/static/images/itembank-icon.svg b/cms/static/images/itembank-icon.svg new file mode 100644 index 000000000000..b2198e1abac1 --- /dev/null +++ b/cms/static/images/itembank-icon.svg @@ -0,0 +1,3 @@ + diff --git a/cms/static/images/library-icon.svg b/cms/static/images/library-icon.svg new file mode 100644 index 000000000000..a4fe81244e06 --- /dev/null +++ b/cms/static/images/library-icon.svg @@ -0,0 +1,3 @@ + diff --git a/cms/static/images/library_v2-icon.svg b/cms/static/images/library_v2-icon.svg new file mode 100644 index 000000000000..4d69dbf9a063 --- /dev/null +++ b/cms/static/images/library_v2-icon.svg @@ -0,0 +1,3 @@ + diff --git a/cms/static/images/openassessment-icon.svg b/cms/static/images/openassessment-icon.svg new file mode 100644 index 000000000000..4841b6b43dae --- /dev/null +++ b/cms/static/images/openassessment-icon.svg @@ -0,0 +1,6 @@ + diff --git a/cms/static/images/problem-icon.svg b/cms/static/images/problem-icon.svg new file mode 100644 index 000000000000..7d51f436f7ee --- /dev/null +++ b/cms/static/images/problem-icon.svg @@ -0,0 +1,6 @@ + diff --git a/cms/static/images/text-icon.svg b/cms/static/images/text-icon.svg new file mode 100644 index 000000000000..8588a471c954 --- /dev/null +++ b/cms/static/images/text-icon.svg @@ -0,0 +1,3 @@ + diff --git a/cms/static/images/video-icon.svg b/cms/static/images/video-icon.svg new file mode 100644 index 000000000000..08f7444b621f --- /dev/null +++ b/cms/static/images/video-icon.svg @@ -0,0 +1,3 @@ + diff --git a/cms/static/js/views/components/add_xblock.js b/cms/static/js/views/components/add_xblock.js index 29ce5eec7673..dd2f11dbe82a 100644 --- a/cms/static/js/views/components/add_xblock.js +++ b/cms/static/js/views/components/add_xblock.js @@ -42,10 +42,38 @@ function($, _, gettext, BaseView, ViewUtils, AddXBlockButton, AddXBlockMenu, Add }, showComponentTemplates: function(event) { - var type; + var type, parentLocator, model, parentBlockType; event.preventDefault(); event.stopPropagation(); + type = $(event.currentTarget).data('type'); + parentLocator = $(event.currentTarget).closest('.xblock[data-usage-id]').data('usage-id'); + parentBlockType = $(event.currentTarget).parents('.xblock-author_view').last().data('block-type'); + model = this.collection.models.find(function(item) { return item.type === type; }) || {}; + + try { + if (this.options.isIframeEmbed && parentBlockType !== 'split_test') { + window.parent.postMessage( + { + type: 'showComponentTemplates', + payload: { + type: type, + parentLocator: parentLocator, + model: { + type: model.type, + display_name: model.display_name, + templates: model.templates, + support_legend: model.support_legend, + }, + } + }, document.referrer + ); + return true; + } + } catch (e) { + console.error(e); + } + this.$('.new-component').slideUp(250); this.$('.new-component-' + type).slideDown(250); this.$('.new-component-' + type + ' div').focus(); @@ -65,11 +93,25 @@ function($, _, gettext, BaseView, ViewUtils, AddXBlockButton, AddXBlockMenu, Add var self = this, $element = $(event.currentTarget), saveData = $element.data(), - oldOffset = ViewUtils.getScrollOffset(this.$el); + oldOffset = ViewUtils.getScrollOffset(this.$el), + usageId = $element.closest('.xblock[data-usage-id]').data('usage-id'); event.preventDefault(); this.closeNewComponent(event); if (saveData.type === 'library_v2') { + try { + if (this.options.isIframeEmbed) { + return window.parent.postMessage( + { + type: 'showSingleComponentPicker', + payload: { usageId }, + }, document.referrer + ); + } + } catch (e) { + console.error(e); + } + var modal = new AddLibraryContent(); modal.showComponentPicker( this.options.libraryContentPickerUrl, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 2bd3cc18a3b4..684bd0ab7de0 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -40,6 +40,7 @@ function($, _, Backbone, gettext, BasePage, 'change .header-library-checkbox': 'toggleLibraryComponent', 'click .collapse-button': 'collapseXBlock', 'click .xblock-view-action-button': 'viewXBlockContent', + 'click .xblock-view-group-link': 'viewXBlockContent', }, options: { @@ -60,8 +61,9 @@ function($, _, Backbone, gettext, BasePage, initialize: function(options) { BasePage.prototype.initialize.call(this, options); this.viewClass = options.viewClass || this.defaultViewClass; - this.isLibraryPage = (this.model.attributes.category === 'library'); - this.isLibraryContentPage = (this.model.attributes.category === 'library_content'); + this.isLibraryPage = this.model.attributes.category === 'library'; + this.isLibraryContentPage = this.model.attributes.category === 'library_content'; + this.isSplitTestContentPage = this.model.attributes.category === 'split_test'; this.nameEditor = new XBlockStringFieldEditor({ el: this.$('.wrapper-xblock-field'), model: this.model @@ -131,13 +133,16 @@ function($, _, Backbone, gettext, BasePage, if (this.options.isIframeEmbed) { window.addEventListener('message', (event) => { - const { data } = event; + const { data: initialData } = event; - if (!data) return; + if (!initialData) return; let xblockElement; let xblockWrapper; + const data = { ...initialData }; + data.payload = { ...data?.payload, ...data?.payload?.data }; + if (data.payload && data.payload.locator) { xblockElement = $(`[data-locator="${data.payload.locator}"]`); xblockWrapper = $("li.studio-xblock-wrapper[data-locator='" + data.payload.locator + "']"); @@ -173,6 +178,25 @@ function($, _, Backbone, gettext, BasePage, this.listenTo(Backbone, 'move:onXBlockMoved', this.onXBlockMoved); }, + postMessageToParent: function(body, callbackFn = null) { + try { + window.parent.postMessage(body, document.referrer); + if (callbackFn) { + callbackFn(); + } + } catch (e) { + console.error('Failed to post message:', e); + } + }, + + postMessageForHideProcessingNotification: function () { + this.postMessageToParent({ + type: 'hideProcessingNotification', + message: 'Hide processing notification', + payload: {}, + }); + }, + getViewParameters: function() { return { el: this.$('.wrapper-xblock'), @@ -237,18 +261,14 @@ function($, _, Backbone, gettext, BasePage, const scrollOffset = scrollOffsetString ? parseInt(scrollOffsetString, 10) : 0; if (scrollOffset) { - try { - window.parent.postMessage( - { - type: 'scrollToXBlock', - message: 'Scroll to XBlock', - payload: { scrollOffset } - }, document.referrer - ); - localStorage.removeItem('modalEditLastYPosition'); - } catch (e) { - console.error(e); - } + self.postMessageToParent( + { + type: 'scrollToXBlock', + message: 'Scroll to XBlock', + payload: { scrollOffset } + }, + () => localStorage.removeItem('modalEditLastYPosition') + ); } } }, @@ -272,13 +292,14 @@ function($, _, Backbone, gettext, BasePage, renderAddXBlockComponents: function() { var self = this; - if (self.options.canEdit && !self.options.isIframeEmbed) { + if (self.options.canEdit && (!self.options.isIframeEmbed || self.isSplitTestContentPage)) { this.$('.add-xblock-component').each(function(index, element) { var component = new AddXBlockComponent({ el: element, createComponent: _.bind(self.createComponent, self), collection: self.options.templates, libraryContentPickerUrl: self.options.libraryContentPickerUrl, + isIframeEmbed: self.options.isIframeEmbed, }); component.render(); }); @@ -288,7 +309,7 @@ function($, _, Backbone, gettext, BasePage, }, initializePasteButton() { - if (this.options.canEdit && !this.options.isIframeEmbed) { + if (this.options.canEdit && (!this.options.isIframeEmbed || this.isSplitTestContentPage)) { // We should have the user's clipboard status. const data = this.options.clipboardData; this.refreshPasteButton(data); @@ -305,7 +326,8 @@ function($, _, Backbone, gettext, BasePage, refreshPasteButton(data) { // Do not perform any changes on paste button since they are not // rendered on Library or LibraryContent pages - if (!this.isLibraryPage && !this.isLibraryContentPage && !this.options.isIframeEmbed) { + if (!this.isLibraryPage && !this.isLibraryContentPage && (!this.options.isIframeEmbed || this.isSplitTestContentPage)) { + this.postMessageForHideProcessingNotification(); // 'data' is the same data returned by the "get clipboard status" API endpoint // i.e. /api/content-staging/v1/clipboard/ if (this.options.canEdit && data.content) { @@ -340,17 +362,11 @@ function($, _, Backbone, gettext, BasePage, /** The user has clicked on the "Paste Component button" */ pasteComponent(event) { event.preventDefault(); - try { - if (this.options.isIframeEmbed) { - window.parent.postMessage( - { - type: 'pasteComponent', - payload: {} - }, document.referrer - ); - } - } catch (e) { - console.error(e); + if (this.options.isIframeEmbed) { + this.postMessageToParent({ + type: 'pasteComponent', + payload: {}, + }); } // Get the ID of the container (usually a unit/vertical) that we're pasting into: const parentElement = this.findXBlockElement(event.target); @@ -375,6 +391,9 @@ function($, _, Backbone, gettext, BasePage, placeholderElement.remove(); }); }).done((data) => { + if (this.options.isIframeEmbed) { + this.postMessageForHideProcessingNotification(); + } const { conflicting_files: conflictingFiles, error_files: errorFiles, @@ -646,17 +665,11 @@ function($, _, Backbone, gettext, BasePage, subMenu.classList.toggle('is-shown'); if (!subMenu.classList.contains('is-shown') && this.options.isIframeEmbed) { - try { - window.parent.postMessage( - { - type: 'toggleCourseXBlockDropdown', - message: 'Adjust the height of the dropdown menu', - payload: { courseXBlockDropdownHeight: 0 } - }, document.referrer - ); - } catch (error) { - console.error(error); - } + this.postMessageToParent({ + type: 'toggleCourseXBlockDropdown', + message: 'Adjust the height of the dropdown menu', + payload: { courseXBlockDropdownHeight: 0 } + }); } // Calculate the viewport height and the dropdown menu height. @@ -668,33 +681,21 @@ function($, _, Backbone, gettext, BasePage, if (courseUnitXBlockIframeHeight < courseXBlockDropdownHeight) { // If the dropdown menu is taller than the iframe, adjust the height of the dropdown menu. - try { - window.parent.postMessage( - { - type: 'toggleCourseXBlockDropdown', - message: 'Adjust the height of the dropdown menu', - payload: { courseXBlockDropdownHeight }, - }, document.referrer - ); - } catch (error) { - console.error(error); - } + this.postMessageToParent({ + type: 'toggleCourseXBlockDropdown', + message: 'Adjust the height of the dropdown menu', + payload: { courseXBlockDropdownHeight }, + }); } else if ((courseXBlockDropdownHeight + clickYPosition) > courseUnitXBlockIframeHeight) { if (courseXBlockDropdownHeight > courseUnitXBlockIframeHeight / 2) { // If the dropdown menu is taller than half the iframe, send a message to adjust its height. - try { - window.parent.postMessage( - { - type: 'toggleCourseXBlockDropdown', - message: 'Adjust the height of the dropdown menu', - payload: { - courseXBlockDropdownHeight: courseXBlockDropdownHeight / 2, - }, - }, document.referrer - ); - } catch (error) { - console.error(error); - } + this.postMessageToParent({ + type: 'toggleCourseXBlockDropdown', + message: 'Adjust the height of the dropdown menu', + payload: { + courseXBlockDropdownHeight: courseXBlockDropdownHeight / 2, + }, + }); } else { // Move the dropdown menu upward to prevent it from overflowing out of the viewport. if (this.options.isIframeEmbed) { @@ -719,18 +720,12 @@ function($, _, Backbone, gettext, BasePage, }, openManageTags: function(event) { - const contentId = this.findXBlockElement(event.target).data('locator'); - try { - if (this.options.isIframeEmbed) { - window.parent.postMessage( - { - type: 'openManageTags', - payload: { contentId } - }, document.referrer - ); - } - } catch (e) { - console.error(e); + const contentId = this.findXBlockElement(event.target).data('locator'); + if (this.options.isIframeEmbed) { + this.postMessageToParent({ + type: 'openManageTags', + payload: { contentId }, + }); } const taxonomyTagsWidgetUrl = this.model.get('taxonomy_tags_widget_url'); @@ -747,13 +742,17 @@ function($, _, Backbone, gettext, BasePage, const usageId = encodeURI(primaryHeader.attr('data-usage-id')); try { if (this.options.isIframeEmbed) { - return window.parent.postMessage( + window.parent.postMessage( { - type: 'copyXBlock', + type: this.isSplitTestContentPage ? 'copyXBlockLegacy' : 'copyXBlock', message: 'Copy the XBlock', payload: { usageId } }, document.referrer ); + + if (!this.isSplitTestContentPage) { + return; + } } } catch (e) { console.error(e); @@ -795,6 +794,7 @@ function($, _, Backbone, gettext, BasePage, setTimeout(checkStatus, 1_000); return deferred; } else { + this.postMessageForHideProcessingNotification(); throw new Error(`Unexpected clipboard status "${status}" in successful API response.`); } }); @@ -909,15 +909,12 @@ function($, _, Backbone, gettext, BasePage, this.deleteComponent(this.findXBlockElement(event.target)); }, - createPlaceholderElement: function() { - return $('
', {class: 'studio-xblock-wrapper'}); - }, - createComponent: function(template, target, iframeMessageData) { // A placeholder element is created in the correct location for the new xblock // and then onNewXBlock will replace it with a rendering of the xblock. Note that // for xblocks that can't be replaced inline, the entire parent will be refreshed. var parentElement = this.findXBlockElement(target), + self = this, parentLocator = parentElement.data('locator'), buttonPanel = target?.closest('.add-xblock-component'), listPanel = buttonPanel?.prev(), @@ -929,28 +926,55 @@ function($, _, Backbone, gettext, BasePage, placeholderElement, $container; - if (this.options.isIframeEmbed) { + if (this.options.isIframeEmbed && !this.isSplitTestContentPage) { $container = $('ol.reorderable-container.ui-sortable'); scrollOffset = 0; } else { $container = listPanel; - scrollOffset = ViewUtils.getScrollOffset(buttonPanel); + if (!target.length && iframeMessageData.payload.parent_locator) { + $container = $('.xblock[data-usage-id="' + iframeMessageData.payload.parent_locator + '"]') + .find('ol.reorderable-container.ui-sortable'); + } + if (!iframeMessageData) { + scrollOffset = ViewUtils.getScrollOffset(buttonPanel); + } } placeholderElement = $placeholderEl.appendTo($container); - if (this.options.isIframeEmbed) { - if (iframeMessageData.payload.data && iframeMessageData.type === 'addXBlock') { - return this.onNewXBlock(placeholderElement, scrollOffset, false, iframeMessageData.payload.data); + if (this.options.isIframeEmbed && iframeMessageData) { + if (iframeMessageData.payload.data && iframeMessageData.type === 'addXBlock') { + return this.onNewXBlock(placeholderElement, scrollOffset, false, iframeMessageData.payload.data); + } + } + + if (this.options.isIframeEmbed && this.isSplitTestContentPage) { + this.postMessageToParent({ + type: 'addNewComponent', + message: 'Add new XBlock', + payload: {}, + }); + if (iframeMessageData) { + return; } } return $.postJSON(this.getURLRoot() + '/', requestData, _.bind(this.onNewXBlock, this, placeholderElement, scrollOffset, false)) + .always(function () { + if (self.options.isIframeEmbed && self.isSplitTestContentPage) { + self.postMessageToParent({ + type: 'hideProcessingNotification', + message: 'Hide processing notification', + payload: {} + }); + return true; + } + }) .fail(function() { // Remove the placeholder if the update failed placeholderElement.remove(); - }); + }); }, duplicateComponent: function(xblockElement) { @@ -966,17 +990,11 @@ function($, _, Backbone, gettext, BasePage, placeholderElement = $placeholderEl.insertAfter(xblockElement); if (this.options.isIframeEmbed) { - try { - window.parent.postMessage( - { - type: 'scrollToXBlock', - message: 'Scroll to XBlock', - payload: { scrollOffset: xblockElement.height() } - }, document.referrer - ); - } catch (e) { - console.error(e); - } + this.postMessageToParent({ + type: 'scrollToXBlock', + message: 'Scroll to XBlock', + payload: { scrollOffset: xblockElement.height() } + }); const messageHandler = ({ data }) => { if (data && data.type === 'completeXBlockDuplicating') { @@ -1028,7 +1046,6 @@ function($, _, Backbone, gettext, BasePage, getSelectedLibraryComponents: function() { var self = this; var locator = this.$el.find('.studio-xblock-wrapper').data('locator'); - console.log(ModuleUtils); $.getJSON( ModuleUtils.getUpdateUrl(locator) + '/handler/get_block_ids', function(data) { @@ -1065,19 +1082,16 @@ function($, _, Backbone, gettext, BasePage, }, viewXBlockContent: function(event) { - try { - if (this.options.isIframeEmbed) { - event.preventDefault(); - var usageId = event.currentTarget.href.split('/').pop() || ''; - window.parent.postMessage({ - type: 'handleViewXBlockContent', - message: 'View the content of the XBlock', - payload: { usageId }, - }, document.referrer); - return true; - } - } catch (e) { - console.error(e); + if (this.options.isIframeEmbed) { + event.preventDefault(); + const usageId = event.currentTarget.href.split('/').pop() || ''; + const isViewGroupLink = event.currentTarget.classList.contains('xblock-view-group-link'); + this.postMessageToParent({ + type: isViewGroupLink ? 'handleViewGroupConfigurations' : 'handleViewXBlockContent', + message: isViewGroupLink ? 'View the group configurations page' : 'View the content of the XBlock', + payload: { usageId }, + }); + return true; } }, @@ -1142,6 +1156,17 @@ function($, _, Backbone, gettext, BasePage, destinationUrl = this.$('.xblock-header-primary').attr("authoring_MFE_base_url") + '/' + blockType[1] + '/' + encodeURI(data.locator); } + if (this.options.isIframeEmbed && this.isSplitTestContentPage) { + return this.postMessageToParent({ + type: 'handleRedirectToXBlockEditPage', + message: 'Redirect to xBlock edit page', + payload: { + type: blockType[1], + locator: encodeURI(data.locator), + }, + }); + } + window.location.href = destinationUrl; return; } diff --git a/cms/static/sass/course-unit-mfe-iframe-bundle.scss b/cms/static/sass/course-unit-mfe-iframe-bundle.scss index 79c20ea26b49..fd0949e2e495 100644 --- a/cms/static/sass/course-unit-mfe-iframe-bundle.scss +++ b/cms/static/sass/course-unit-mfe-iframe-bundle.scss @@ -8,6 +8,11 @@ html { } } +body, +#main { + background-color: transparent; +} + [class*="view-"] .wrapper { .inner-wrapper { max-width: 100%; @@ -39,67 +44,105 @@ html { .actions-list .action-item .action-button { border-radius: 4px; + display: inline-flex; + align-items: center; + gap: ($baseline * .3); + padding: ($baseline * .15) ($baseline / 2); &:hover { background-color: $primary; color: $white; } + + .action-button-text { + line-height: 20px; + } } } - &.level-page .xblock-message { - padding: ($baseline * .75) ($baseline * 1.2); - border-radius: 0 0 4px 4px; + &.level-page { + .xblock-message { + padding: ($baseline * .75) ($baseline * 1.2); + border-radius: 0 0 4px 4px; - &.information { - color: $text-color; - background-color: $xblock-message-info-bg; - border-color: $xblock-message-info-border-color; - } + .xblock-message-list { + color: $black; + } - &.validation.has-warnings { - color: $black; - background-color: $xblock-message-warning-bg; - border-color: $xblock-message-warning-border-color; - border-top-width: 1px; + &.information, + &.validation.has-warnings, + &.validation.has-errors { + color: $black; + border-width: 0; + font-size: 16px; + line-height: 22px; + padding: ($baseline * 1.2); + box-shadow: 0 1px 2px rgba(0, 0, 0, .15), 0 1px 4px rgba(0, 0, 0, .15); + } + + &.information { + background-color: $xblock-message-info-bg; + + .icon { + color: $xblock-message-info-icon-color; + } + } - .icon { - color: $xblock-message-warning-border-color; + &.validation.has-warnings { + background-color: $xblock-message-warning-bg; + + .icon { + color: $xblock-message-warning-icon-color; + } + } + + &.validation.has-errors { + background-color: $xblock-message-error-bg; + + .icon { + color: $xblock-message-error-icon-color; + } + } + + a { + color: $primary; + } } - } - a { - color: $primary; - } + &.studio-xblock-wrapper > .wrapper-xblock-message .xblock-message, + .xblock > .wrapper-xblock-message .xblock-message { + border-radius: 4px; + margin-bottom: ($baseline * 1.4); + } } - .xblock-author_view-library_content > .wrapper-xblock-message .xblock-message { - font-size: 16px; - line-height: 22px; - border-radius: 4px; - padding: ($baseline * 1.2); - box-shadow: 0 1px 2px rgba(0, 0, 0, .15), 0 1px 4px rgba(0, 0, 0, .15); - margin-bottom: ($baseline * 1.4); + .xblock-author_view-split_test .wrapper-xblock { + background: $white; + box-shadow: 0 2px 4px rgba(0, 0, 0, .15), 0 2px 8px rgba(0, 0, 0, .15); } &.level-element { box-shadow: 0 2px 4px rgba(0, 0, 0, .15), 0 2px 8px rgba(0, 0, 0, .15); margin: 0 0 ($baseline * 1.4) 0; - } - &.level-element .xblock-header-primary { - background-color: $white; - } + .xblock-header-primary { + background-color: $white; + } - &.level-element .xblock-render { - background: $white; - margin: 0; - padding: $baseline; - border-bottom-left-radius: 6px; - border-bottom-right-radius: 6px; + .xblock-render { + background: $white; + margin: 0; + padding: $baseline; + border-bottom-left-radius: 6px; + border-bottom-right-radius: 6px; + } } .wrapper-xblock .header-actions .actions-list { + .wrapper-nav-sub { + z-index: 11; + } + .action-actions-menu:last-of-type .nav-sub { right: 120px; } @@ -176,6 +219,13 @@ html { } } } + + .wrapper-groups.is-inactive { + box-shadow: 0 2px 4px rgba(0, 0, 0, .15), 0 2px 8px rgba(0, 0, 0, .15); + border-radius: 6px; + border: none; + margin: ($baseline * 1.5) ($baseline / 2) 0; + } } .edit-xblock-modal select { @@ -443,8 +493,8 @@ html { } &.xmodule_DoneXBlock { - margin-top: 60px; - padding: 0 20px; + margin-top: ($baseline * 3); + padding: 0 $baseline; } .xblock-actions { @@ -578,7 +628,7 @@ html { } body [class*="view-"] .openassessment_editor_buttons.xblock-actions { - padding: 15px 2% 3px 2%; + padding: ($baseline * .75) 2% ($baseline * .15) 2%; } [class*="view-"] { @@ -634,7 +684,7 @@ body [class*="view-"] .openassessment_editor_buttons.xblock-actions { .list-input.settings-list { .field.comp-setting-entry.is-set .setting-input { color: $text-color; - margin-bottom: 5px; + margin-bottom: ($baseline * .25); } select { @@ -733,7 +783,7 @@ select { #openassessment_editor_header .editor_tabs .oa_editor_tab { @extend %light-button; - padding: 0 10px; + padding: 0 ($baseline / 2); } #openassessment_editor_header, @@ -762,7 +812,7 @@ select { #oa_rubric_editor_wrapper .openassessment_criterion_option .openassessment_criterion_option_point_wrapper label input { min-width: 70px; - font-size: 18px; + font-size: $base-font-size; height: 44px; } @@ -835,7 +885,7 @@ select { width: 100%; &.background-url { - margin-bottom: 10px; + margin-bottom: ($baseline / 2); } &.autozone-layout { @@ -858,3 +908,104 @@ select { width: 100%; } } + +.xblock-render { + .add-xblock-component { + background: transparent; + padding: $baseline; + + .new-component { + h5 { + margin-bottom: ($baseline * 1.2); + font-size: 22px; + font-weight: 700; + color: $black; + } + + .new-component-type { + display: flex; + flex-wrap: wrap; + gap: ($baseline * .6); + align-items: center; + justify-content: center; + + .add-xblock-component-button { + box-shadow: 0 1px 2px rgba(0, 0, 0, .15), 0 1px 4px rgba(0, 0, 0, .15); + width: 176px; + height: 110px; + color: $primary; + border-color: $primary; + background: transparent; + margin: 0; + display: inline-flex; + align-items: center; + justify-content: center; + flex-direction: column; + gap: ($baseline * .4); + + &:hover { + color: darken($primary, 10%); + background-color: lighten($primary, 80%); + border-color: darken($primary, 15%); + } + + .large-template-icon { + width: 24px; + height: 24px; + background: $primary; + + @each $name, $file in $template-icon-map { + &.large-#{$name}-icon { + mask: url("#{$static-path}/images/#{$file}.svg") center no-repeat; + } + } + } + + .name { + color: inherit; + font-size: 15.75px; + font-weight: 400; + } + + .beta { + color: $white; + background-color: $primary; + padding: ($baseline * .1) ($baseline * .4) ($baseline * .2); + font-size: 13.5px; + font-weight: 700; + line-height: 1; + margin: -($baseline * .3) 0 0; + } + } + } + } + + .new-component-templates { + border: 1px solid $border-color; + border-radius: 5px; + box-shadow: 0 1px 2px rgba(0, 0, 0, .15), 0 1px 4px rgba(0, 0, 0, .15); + margin: $baseline; + overflow: hidden; + + .button-component:hover { + background: $primary; + } + + .cancel-button { + @extend %primary-button; + } + } + } +} + +.paste-component { + margin: ($baseline * 1.2) ($baseline / 2) 0; + + .paste-component-whats-in-clipboard .clipboard-details-popup { + right: ($baseline / 2 * -1); + } + + .paste-component-button.button { + @extend %button-primary-outline; + } +} diff --git a/cms/static/sass/elements/_course-unit-mfe-iframe.scss b/cms/static/sass/elements/_course-unit-mfe-iframe.scss index dc7510994228..5985e9aadd5e 100644 --- a/cms/static/sass/elements/_course-unit-mfe-iframe.scss +++ b/cms/static/sass/elements/_course-unit-mfe-iframe.scss @@ -31,6 +31,8 @@ cursor: pointer; background-image: none; display: block; + box-shadow: none; + text-shadow: none; &:hover { background: darken($primary, 5%); @@ -46,6 +48,35 @@ } } +%button-primary-outline { + @extend %modal-actions-button; + + color: $primary; + border-color: $primary; + text-shadow: none; + font-weight: 400; + position: relative; + + &:focus { + color: $primary; + background: transparent; + + &:before { + content: ""; + position: absolute; + inset: -5px; + border: 2px solid $primary; + border-radius: 10px; + } + } + + &:hover { + color: darken($primary, 10%); + background-color: lighten($primary, 80%); + border-color: darken($primary, 15%); + } +} + %light-button { @extend %modal-actions-button; diff --git a/cms/static/sass/partials/cms/theme/_variables-v1.scss b/cms/static/sass/partials/cms/theme/_variables-v1.scss index 0b3fe6b6e49b..b60bc15f2e8c 100644 --- a/cms/static/sass/partials/cms/theme/_variables-v1.scss +++ b/cms/static/sass/partials/cms/theme/_variables-v1.scss @@ -317,6 +317,23 @@ $dark: #212529; $zindex-dropdown: 100; $xblock-message-info-bg: #eff8fa; -$xblock-message-info-border-color: #9cd2e6; +$xblock-message-info-icon-color: #9cd2e6; + $xblock-message-warning-bg: #fffdf0; -$xblock-message-warning-border-color: #fff6bf; +$xblock-message-warning-icon-color: #ffd900; + +$xblock-message-error-bg: #fbf2f3; +$xblock-message-error-icon-color: #c32d3a; + +$template-icon-map: ( + "library": "library-icon", + "library_v2": "library_v2-icon", + "itembank": "itembank-icon", + "advanced": "advanced-icon", + "html": "text-icon", + "openassessment": "openassessment-icon", + "problem": "problem-icon", + "video": "video-icon", + "drag-and-drop-v2": "drag-and-drop-v2-icon", + "text": "text-icon" +); diff --git a/cms/templates/container_chromeless.html b/cms/templates/container_chromeless.html index 2fe821e49d5e..a233e0dc7d64 100644 --- a/cms/templates/container_chromeless.html +++ b/cms/templates/container_chromeless.html @@ -201,6 +201,7 @@ outlineURL: "${outline_url | n, js_escaped_string}", clipboardData: ${user_clipboard | n, dump_js_escaped_json}, isIframeEmbed: true, + libraryContentPickerUrl: "${library_content_picker_url | n, js_escaped_string}", } ); diff --git a/common/djangoapps/student/api.py b/common/djangoapps/student/api.py index 2bf42f48289c..c5bd7b861871 100644 --- a/common/djangoapps/student/api.py +++ b/common/djangoapps/student/api.py @@ -4,6 +4,7 @@ """ +from typing import TYPE_CHECKING import logging from django.contrib.auth import get_user_model @@ -32,6 +33,10 @@ ) from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +if TYPE_CHECKING: + from django.contrib.auth.models import AnonymousUser, User # pylint: disable=imported-auth-user + from django.db.models.query import QuerySet + # This is done so that if these strings change within the app, we can keep exported constants the same ENROLLED_TO_ENROLLED = _ENROLLED_TO_ENROLLED @@ -92,13 +97,7 @@ def create_manual_enrollment_audit( else: enrollment = None - _create_manual_enrollment_audit( - enrolled_by, - user_email, - transition_state, - reason, - enrollment - ) + _create_manual_enrollment_audit(enrolled_by, user_email, transition_state, reason, enrollment) def get_access_role_by_role_name(role_name): @@ -132,7 +131,31 @@ def is_user_staff_or_instructor_in_course(user, course_key): course_key = CourseKey.from_string(course_key) return ( - GlobalStaff().has_user(user) or - CourseStaffRole(course_key).has_user(user) or - CourseInstructorRole(course_key).has_user(user) + GlobalStaff().has_user(user) + or CourseStaffRole(course_key).has_user(user) + or CourseInstructorRole(course_key).has_user(user) ) + + +def get_course_enrollments( + user: "AnonymousUser | User", + is_filtered: bool = False, + course_ids: list[str | None] | None = None, +) -> "QuerySet[CourseEnrollment]": + """ + Return enrollments for a user, potentially filtered by course_id. + + Because an empty `course_ids` value is a meaningful filter, the easiest way to verify + that the list should be filtered intentionally is to specify `is_filtered`. + + Arguments: + + * is_filtered (bool): whether or not the list is filtered + * course_ids (list): a list of course IDs to filter by. + """ + course_enrollments = CourseEnrollment.enrollments_for_user(user).select_related("course") + + if is_filtered: + course_enrollments = course_enrollments.filter(course_id__in=course_ids) + + return course_enrollments diff --git a/common/djangoapps/student/tests/test_api.py b/common/djangoapps/student/tests/test_api.py index ad462830a115..7cb20380cf9a 100644 --- a/common/djangoapps/student/tests/test_api.py +++ b/common/djangoapps/student/tests/test_api.py @@ -1,10 +1,16 @@ """ Test Student api.py """ + from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from common.djangoapps.student.api import is_user_enrolled_in_course, is_user_staff_or_instructor_in_course +from common.djangoapps.student.api import ( + is_user_enrolled_in_course, + is_user_staff_or_instructor_in_course, + get_course_enrollments, +) +from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import ( CourseEnrollmentFactory, GlobalStaffFactory, @@ -33,10 +39,7 @@ def test_is_user_enrolled_in_course(self): """ Verify the correct value is returned when a learner is actively enrolled in a course-run. """ - CourseEnrollmentFactory.create( - user_id=self.user.id, - course_id=self.course.id - ) + CourseEnrollmentFactory.create(user_id=self.user.id, course_id=self.course.id) result = is_user_enrolled_in_course(self.user, self.course_run_key) assert result @@ -45,11 +48,7 @@ def test_is_user_enrolled_in_course_not_active(self): """ Verify the correct value is returned when a learner is not actively enrolled in a course-run. """ - CourseEnrollmentFactory.create( - user_id=self.user.id, - course_id=self.course.id, - is_active=False - ) + CourseEnrollmentFactory.create(user_id=self.user.id, course_id=self.course.id, is_active=False) result = is_user_enrolled_in_course(self.user, self.course_run_key) assert not result @@ -79,3 +78,25 @@ def test_is_user_staff_or_instructor(self): assert is_user_staff_or_instructor_in_course(instructor, self.course_run_key) assert not is_user_staff_or_instructor_in_course(self.user, self.course_run_key) assert not is_user_staff_or_instructor_in_course(instructor_different_course, self.course_run_key) + + def test_get_course_enrollments(self): + """Verify all enrollments can be retrieved""" + course_2 = CourseFactory.create() + CourseEnrollmentFactory.create(user_id=self.user.id, course_id=self.course.id) + CourseEnrollmentFactory.create(user_id=self.user.id, course_id=course_2.id) + expected = CourseEnrollment.objects.all() + + result = get_course_enrollments(self.user) + + self.assertQuerySetEqual(expected, result) + + def test_get_filtered_course_enrollments(self): + """Verify a filtered subset of enrollments can be retrieved""" + course_2 = CourseFactory.create() + CourseEnrollmentFactory.create(user_id=self.user.id, course_id=self.course.id) + ce_2 = CourseEnrollmentFactory.create(user_id=self.user.id, course_id=course_2.id) + expected = CourseEnrollment.objects.filter(id=ce_2.id) + + result = get_course_enrollments(self.user, True, course_ids=[course_2.id]) + + self.assertQuerySetEqual(expected, result) diff --git a/common/static/data/geoip/GeoLite2-Country.mmdb b/common/static/data/geoip/GeoLite2-Country.mmdb index 1ec231eb6e7c..f994294a45c3 100644 Binary files a/common/static/data/geoip/GeoLite2-Country.mmdb and b/common/static/data/geoip/GeoLite2-Country.mmdb differ diff --git a/lms/envs/common.py b/lms/envs/common.py index 37a508606c1b..6b9d516eeb5e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3369,6 +3369,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring "openedx_learning.apps.authoring.components", "openedx_learning.apps.authoring.contents", "openedx_learning.apps.authoring.publishing", + "openedx_learning.apps.authoring.units", ] diff --git a/lms/templates/split_test_author_view.html b/lms/templates/split_test_author_view.html index 0c90081cdd6e..ddadf055ff7d 100644 --- a/lms/templates/split_test_author_view.html +++ b/lms/templates/split_test_author_view.html @@ -17,7 +17,7 @@

${Text(_("This content experiment uses group configuration '{group_configuration_name}'.")).format( - group_configuration_name=Text(HTML("{}")).format(group_configuration_url, user_partition.name) if show_link else user_partition.name + group_configuration_name=Text(HTML("{}")).format(group_configuration_url, user_partition.name) if show_link else user_partition.name )}

diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index dd8cfcdc891d..a597ae51e865 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -18,7 +18,7 @@ from meilisearch.errors import MeilisearchApiError, MeilisearchError from meilisearch.models.task import TaskInfo from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryCollectionLocator +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryCollectionLocator from openedx_learning.api import authoring as authoring_api from common.djangoapps.student.roles import GlobalStaff from rest_framework.request import Request @@ -40,6 +40,7 @@ meili_id_from_opaque_key, searchable_doc_for_course_block, searchable_doc_for_collection, + searchable_doc_for_container, searchable_doc_for_library_block, searchable_doc_for_usage_key, searchable_doc_collections, @@ -475,6 +476,31 @@ def index_collection_batch(batch, num_done, library_key) -> int: status_cb(f"Error indexing collection batch {p}: {err}") return num_done + ############## Containers ############## + def index_container_batch(batch, num_done, library_key) -> int: + docs = [] + for container in batch: + try: + container_key = lib_api.library_container_locator( + library_key, + container, + ) + doc = searchable_doc_for_container(container_key) + # TODO: when we add container tags + # doc.update(searchable_doc_tags_for_container(container_key)) + docs.append(doc) + except Exception as err: # pylint: disable=broad-except + status_cb(f"Error indexing container {container.key}: {err}") + num_done += 1 + + if docs: + try: + # Add docs in batch of 100 at once (usually faster than adding one at a time): + _wait_for_meili_task(client.index(index_name).add_documents(docs)) + except (TypeError, KeyError, MeilisearchError) as err: + status_cb(f"Error indexing container batch {p}: {err}") + return num_done + for lib_key in lib_keys: status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing blocks in library {lib_key}") lib_docs = index_library(lib_key) @@ -497,6 +523,22 @@ def index_collection_batch(batch, num_done, library_key) -> int: IncrementalIndexCompleted.objects.get_or_create(context_key=lib_key) status_cb(f"{num_collections_done}/{num_collections} collections indexed for library {lib_key}") + # Similarly, batch process Containers (units, sections, etc) in pages of 100 + containers = authoring_api.get_containers(library.learning_package_id) + num_containers = containers.count() + num_containers_done = 0 + status_cb(f"{num_containers_done}/{num_containers}. Now indexing containers in library {lib_key}") + paginator = Paginator(containers, 100) + for p in paginator.page_range: + num_containers_done = index_container_batch( + paginator.page(p).object_list, + num_containers_done, + lib_key, + ) + status_cb(f"{num_containers_done}/{num_containers} containers indexed for library {lib_key}") + if incremental: + IncrementalIndexCompleted.objects.get_or_create(context_key=lib_key) + num_contexts_done += 1 ############## Courses ############## @@ -732,6 +774,30 @@ def update_library_components_collections( _update_index_docs(docs) +def upsert_library_container_index_doc(container_key: LibraryContainerLocator) -> None: + """ + Creates, updates, or deletes the document for the given Library Container in the search index. + + TODO: add support for indexing a container's components, like upsert_library_collection_index_doc does. + """ + doc = searchable_doc_for_container(container_key) + + # Soft-deleted/disabled containers are removed from the index + # and their components updated. + if doc.get('_disabled'): + + _delete_index_doc(doc[Fields.id]) + + # Hard-deleted containers are also deleted from the index + elif not doc.get(Fields.type): + + _delete_index_doc(doc[Fields.id]) + + # Otherwise, upsert the container. + else: + _update_index_docs([doc]) + + def upsert_content_library_index_docs(library_key: LibraryLocatorV2) -> None: """ Creates or updates the documents for the given Content Library in the search index diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 3d739781fe71..061015751955 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -6,11 +6,12 @@ import logging from hashlib import blake2b -from django.utils.text import slugify from django.core.exceptions import ObjectDoesNotExist +from django.utils.text import slugify from opaque_keys.edx.keys import LearningContextKey, UsageKey +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 from openedx_learning.api import authoring as authoring_api -from opaque_keys.edx.locator import LibraryLocatorV2 +from openedx_learning.api.authoring_models import Collection from rest_framework.exceptions import NotFound from openedx.core.djangoapps.content.search.models import SearchAccess @@ -19,7 +20,6 @@ from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.djangoapps.xblock.data import LatestVersion -from openedx_learning.api.authoring_models import Collection log = logging.getLogger(__name__) @@ -100,6 +100,7 @@ class DocType: """ course_block = "course_block" library_block = "library_block" + library_container = "library_container" collection = "collection" @@ -546,3 +547,63 @@ def searchable_doc_for_collection( doc['_disabled'] = True return doc + + +def searchable_doc_for_container( + container_key: LibraryContainerLocator, +) -> dict: + """ + Generate a dictionary document suitable for ingestion into a search engine + like Meilisearch or Elasticsearch, so that the given container can be + found using faceted search. + + If no container is found for the given container key, the returned document + will contain only basic information derived from the container key, and no + Fields.type value will be included in the returned dict. + """ + doc = { + Fields.id: meili_id_from_opaque_key(container_key), + Fields.context_key: str(container_key.library_key), + Fields.org: str(container_key.org), + # In the future, this may be either course_container or library_container + Fields.type: DocType.library_container, + # To check if it is "unit", "section", "subsection", etc.. + Fields.block_type: container_key.container_type, + Fields.usage_key: str(container_key), # Field name isn't exact but this is the closest match + Fields.block_id: container_key.container_id, # Field name isn't exact but this is the closest match + Fields.access_id: _meili_access_id_from_context_key(container_key.library_key), + Fields.publish_status: PublishStatus.never, + } + + try: + container = lib_api.get_container(container_key) + except lib_api.ContentLibraryContainerNotFound: + # Container not found, so we can only return the base doc + return doc + + draft_num_children = lib_api.get_container_children_count(container_key, published=False) + publish_status = PublishStatus.published + if container.last_published is None: + publish_status = PublishStatus.never + elif container.has_unpublished_changes: + publish_status = PublishStatus.modified + + doc.update({ + Fields.display_name: container.display_name, + Fields.created: container.created.timestamp(), + Fields.modified: container.modified.timestamp(), + Fields.num_children: draft_num_children, + Fields.publish_status: publish_status, + }) + library = lib_api.get_library(container_key.library_key) + if library: + doc[Fields.breadcrumbs] = [{"display_name": library.title}] + + if container.published_version_num is not None: + published_num_children = lib_api.get_container_children_count(container_key, published=True) + doc[Fields.published] = { + # Fields.published_display_name: container_published.title, TODO: set the published title + Fields.published_num_children: published_num_children, + } + + return doc diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 24add6748d7d..4565165e87ea 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -14,6 +14,7 @@ ContentObjectChangedData, LibraryBlockData, LibraryCollectionData, + LibraryContainerData, XBlockData, ) from openedx_events.content_authoring.signals import ( @@ -25,6 +26,9 @@ LIBRARY_COLLECTION_CREATED, LIBRARY_COLLECTION_DELETED, LIBRARY_COLLECTION_UPDATED, + LIBRARY_CONTAINER_CREATED, + LIBRARY_CONTAINER_DELETED, + LIBRARY_CONTAINER_UPDATED, XBLOCK_CREATED, XBLOCK_DELETED, XBLOCK_UPDATED, @@ -45,6 +49,7 @@ delete_xblock_index_doc, update_content_library_index_docs, update_library_collection_index_doc, + update_library_container_index_doc, upsert_library_block_index_doc, upsert_xblock_index_doc, ) @@ -225,3 +230,31 @@ def content_object_associations_changed_handler(**kwargs) -> None: upsert_block_tags_index_docs(usage_key) if not content_object.changes or "collections" in content_object.changes: upsert_block_collections_index_docs(usage_key) + + +@receiver(LIBRARY_CONTAINER_CREATED) +@receiver(LIBRARY_CONTAINER_DELETED) +@receiver(LIBRARY_CONTAINER_UPDATED) +@only_if_meilisearch_enabled +def library_container_updated_handler(**kwargs) -> None: + """ + Create or update the index for the content library container + """ + library_container = kwargs.get("library_container", None) + if not library_container or not isinstance(library_container, LibraryContainerData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + if library_container.background: + update_library_container_index_doc.delay( + str(library_container.library_key), + library_container.container_key, + ) + else: + # Update container index synchronously to make sure that search index is updated before + # the frontend invalidates/refetches index. + # See content_library_updated_handler for more details. + update_library_container_index_doc.apply(args=[ + str(library_container.library_key), + library_container.container_key, + ]) diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index 98390a12f3b3..f23ca9aa304e 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -11,7 +11,7 @@ from edx_django_utils.monitoring import set_code_owner_attribute from meilisearch.errors import MeilisearchError from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from . import api @@ -110,3 +110,17 @@ def update_library_components_collections(library_key_str: str, collection_key: log.info("Updating document.collections for library %s collection %s components", library_key, collection_key) api.update_library_components_collections(library_key, collection_key) + + +@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) +@set_code_owner_attribute +def update_library_container_index_doc(library_key_str: str, container_key_str: str) -> None: + """ + Celery task to update the content index document for a library container + """ + library_key = LibraryLocatorV2.from_string(library_key_str) + container_key = LibraryContainerLocator.from_string(container_key_str) + + log.info("Updating content index documents for container %s in library%s", container_key, library_key) + + api.upsert_library_container_index_doc(container_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 8063307d61e9..813db0241db1 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -208,6 +208,36 @@ def setUp(self): "breadcrumbs": [{"display_name": "Library"}], } + # Create a unit: + with freeze_time(created_date): + self.unit = library_api.create_container( + library_key=self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit-1", + title="Unit 1", + user_id=None, + ) + self.unit_key = "lct:org1:lib:unit:unit-1" + self.unit_dict = { + "id": "lctorg1libunitunit-1-e4527f7c", + "block_id": "unit-1", + "block_type": "unit", + "usage_key": self.unit_key, + "type": "library_container", + "display_name": "Unit 1", + # description is not set for containers + "num_children": 0, + "publish_status": "never", + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + @override_settings(MEILISEARCH_ENABLED=False) def test_reindex_meilisearch_disabled(self, mock_meilisearch): with self.assertRaises(RuntimeError): @@ -231,14 +261,16 @@ def test_reindex_meilisearch(self, mock_meilisearch): doc_problem2["collections"] = {'display_name': [], 'key': []} doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} + doc_unit = copy.deepcopy(self.unit_dict) api.rebuild_index() - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 3 + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), call([doc_collection]), + call([doc_unit]), ], any_order=True, ) @@ -259,14 +291,16 @@ def test_reindex_meilisearch_incremental(self, mock_meilisearch): doc_problem2["collections"] = {"display_name": [], "key": []} doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} + doc_unit = copy.deepcopy(self.unit_dict) api.rebuild_index(incremental=True) - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 3 + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), call([doc_collection]), + call([doc_unit]), ], any_order=True, ) @@ -280,13 +314,13 @@ def simulated_interruption(message): with pytest.raises(Exception, match="Simulated interruption"): api.rebuild_index(simulated_interruption, incremental=True) - # two more calls due to collections - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 5 + # three more calls due to collections and containers + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 7 assert IncrementalIndexCompleted.objects.all().count() == 1 api.rebuild_index(incremental=True) assert IncrementalIndexCompleted.objects.all().count() == 0 # one missing course indexed - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 6 + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 8 @override_settings(MEILISEARCH_ENABLED=True) def test_reset_meilisearch_index(self, mock_meilisearch): @@ -340,6 +374,22 @@ def test_reindex_meilisearch_collection_error(self, mock_meilisearch): f"Error indexing collection {self.collection}: Failed to generate document" ) + @override_settings(MEILISEARCH_ENABLED=True) + @patch( + "openedx.core.djangoapps.content.search.api.searchable_doc_for_container", + Mock(side_effect=Exception("Failed to generate document")), + ) + def test_reindex_meilisearch_container_error(self, mock_meilisearch): + + mock_logger = Mock() + api.rebuild_index(mock_logger) + assert call( + [self.unit_dict] + ) not in mock_meilisearch.return_value.index.return_value.add_documents.mock_calls + mock_logger.assert_any_call( + "Error indexing container unit-1: Failed to generate document" + ) + @override_settings(MEILISEARCH_ENABLED=True) def test_reindex_meilisearch_library_block_error(self, mock_meilisearch): diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 3bab2795b9f5..ee2253347f20 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -3,26 +3,30 @@ """ from dataclasses import replace from datetime import datetime, timezone -from organizations.models import Organization from freezegun import freeze_time +from openedx_learning.api import authoring as authoring_api +from organizations.models import Organization -from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.content_libraries import api as library_api +from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import BlockFactory, ToyCourseFactory +from openedx_learning.api import authoring as authoring_api + try: # This import errors in the lms because content.search is not an installed app there. from ..documents import ( - searchable_doc_for_course_block, - searchable_doc_tags, - searchable_doc_tags_for_collection, searchable_doc_collections, searchable_doc_for_collection, + searchable_doc_for_container, + searchable_doc_for_course_block, searchable_doc_for_library_block, + searchable_doc_tags, + searchable_doc_tags_for_collection, ) from ..models import SearchAccess except RuntimeError: @@ -30,6 +34,7 @@ searchable_doc_tags = lambda x: x searchable_doc_tags_for_collection = lambda x: x searchable_doc_for_collection = lambda x: x + searchable_doc_for_container = lambda x: x searchable_doc_for_library_block = lambda x: x SearchAccess = {} @@ -494,6 +499,143 @@ def test_collection_with_published_library(self): } } + def test_draft_container(self): + """ + Test creating a search document for a draft-only container + """ + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + container_meta = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit1", + title="A Unit in the Search Index", + user_id=None, + ) + + doc = searchable_doc_for_container(container_meta.container_key) + + assert doc == { + "id": "lctedx2012_fallunitunit1-edd13a0c", + "block_id": "unit1", + "block_type": "unit", + "usage_key": "lct:edX:2012_Fall:unit:unit1", + "type": "library_container", + "org": "edX", + "display_name": "A Unit in the Search Index", + # description is not set for containers + "num_children": 0, + "publish_status": "never", + "context_key": "lib:edX:2012_Fall", + "access_id": self.library_access_id, + "breadcrumbs": [{"display_name": "some content_library"}], + "created": 1680674828.0, + "publish_status": "never", + "modified": 1680674828.0, + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + + def test_published_container(self): + """ + Test creating a search document for a published container + """ + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + container_meta = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit1", + title="A Unit in the Search Index", + user_id=None, + ) + library_api.update_container_children( + container_meta.container_key, + [self.library_block.usage_key], + user_id=None, + ) + library_api.publish_changes(self.library.key) + + doc = searchable_doc_for_container(container_meta.container_key) + + assert doc == { + "id": "lctedx2012_fallunitunit1-edd13a0c", + "block_id": "unit1", + "block_type": "unit", + "usage_key": "lct:edX:2012_Fall:unit:unit1", + "type": "library_container", + "org": "edX", + "display_name": "A Unit in the Search Index", + # description is not set for containers + "num_children": 1, + "publish_status": "published", + "context_key": "lib:edX:2012_Fall", + "access_id": self.library_access_id, + "breadcrumbs": [{"display_name": "some content_library"}], + "created": 1680674828.0, + "modified": 1680674828.0, + "published": {"num_children": 1}, + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + + def test_published_container_with_changes(self): + """ + Test creating a search document for a published container + """ + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + container_meta = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit1", + title="A Unit in the Search Index", + user_id=None, + ) + library_api.update_container_children( + container_meta.container_key, + [self.library_block.usage_key], + user_id=None, + ) + library_api.publish_changes(self.library.key) + block_2 = library_api.create_library_block( + self.library.key, + "html", + "text3", + ) + + # Add another component after publish + with freeze_time(created_date): + library_api.update_container_children( + container_meta.container_key, + [block_2.usage_key], + user_id=None, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + + doc = searchable_doc_for_container(container_meta.container_key) + + assert doc == { + "id": "lctedx2012_fallunitunit1-edd13a0c", + "block_id": "unit1", + "block_type": "unit", + "usage_key": "lct:edX:2012_Fall:unit:unit1", + "type": "library_container", + "org": "edX", + "display_name": "A Unit in the Search Index", + # description is not set for containers + "num_children": 2, + "publish_status": "modified", + "context_key": "lib:edX:2012_Fall", + "access_id": self.library_access_id, + "breadcrumbs": [{"display_name": "some content_library"}], + "created": 1680674828.0, + "modified": 1680674828.0, + "published": {"num_children": 1}, + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + def test_mathjax_plain_text_conversion_for_search(self): """ Test how an HTML block with mathjax equations gets converted to plain text in search description. diff --git a/openedx/core/djangoapps/content_libraries/api/__init__.py b/openedx/core/djangoapps/content_libraries/api/__init__.py new file mode 100644 index 000000000000..d4d9fe047fb9 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/__init__.py @@ -0,0 +1,7 @@ +""" +Python API for working with content libraries +""" +from .containers import * +from .libraries import * +from .blocks import * +from . import permissions diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py new file mode 100644 index 000000000000..383a8d8fbd07 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -0,0 +1,30 @@ +""" +Content libraries API methods related to XBlocks/Components. + +These methods don't enforce permissions (only the REST APIs do). +""" +# pylint: disable=unused-import + +# TODO: move all the API methods related to blocks and assets in here from 'libraries.py' +# TODO: use __all__ to limit what symbols are public. + +from .libraries import ( + LibraryXBlockMetadata, + LibraryXBlockStaticFile, + LibraryXBlockType, + get_library_components, + get_library_block, + set_library_block_olx, + library_component_usage_key, + get_component_from_usage_key, + validate_can_add_block_to_library, + create_library_block, + import_staged_content_from_user_clipboard, + get_or_create_olx_media_type, + delete_library_block, + restore_library_block, + get_library_block_static_asset_files, + add_library_block_static_asset_file, + delete_library_block_static_asset_file, + publish_component_changes, +) diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py new file mode 100644 index 000000000000..f0513bf4c68c --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -0,0 +1,337 @@ +""" +API for containers (Sections, Subsections, Units) in Content Libraries +""" +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime +from enum import Enum +from uuid import uuid4 + +from django.utils.text import slugify +from opaque_keys.edx.locator import ( + LibraryContainerLocator, + LibraryLocatorV2, + UsageKeyV2, + LibraryUsageLocatorV2, +) +from openedx_events.content_authoring.data import LibraryContainerData +from openedx_events.content_authoring.signals import ( + LIBRARY_CONTAINER_CREATED, + LIBRARY_CONTAINER_DELETED, + LIBRARY_CONTAINER_UPDATED, +) +from openedx_learning.api import authoring as authoring_api +from openedx_learning.api.authoring_models import Container + +from openedx.core.djangoapps.xblock.api import get_component_from_usage_key + +from ..models import ContentLibrary +from .libraries import LibraryXBlockMetadata, PublishableItem + + +# The public API is only the following symbols: +__all__ = [ + "ContentLibraryContainerNotFound", + "ContainerMetadata", + "ContainerType", + "get_container", + "create_container", + "get_container_children", + "get_container_children_count", + "library_container_locator", + "update_container", + "delete_container", + "update_container_children", + "get_containers_contains_component", +] + + +ContentLibraryContainerNotFound = Container.DoesNotExist + + +class ContainerType(Enum): + Unit = "unit" + + +@dataclass(frozen=True, kw_only=True) +class ContainerMetadata(PublishableItem): + """ + Class that represents the metadata about a Container (e.g. Unit) in a content library. + """ + container_key: LibraryContainerLocator + container_type: ContainerType + + @classmethod + def from_container(cls, library_key, container: Container, associated_collections=None): + """ + Construct a ContainerMetadata object from a Container object. + """ + last_publish_log = container.versioning.last_publish_log + container_key = library_container_locator( + library_key, + container=container, + ) + container_type = ContainerType(container_key.container_type) + + published_by = "" + if last_publish_log and last_publish_log.published_by: + published_by = last_publish_log.published_by.username + + draft = container.versioning.draft + published = container.versioning.published + last_draft_created = draft.created if draft else None + if draft and draft.publishable_entity_version.created_by: + last_draft_created_by = draft.publishable_entity_version.created_by.username + else: + last_draft_created_by = "" + + return cls( + container_key=container_key, # LibraryContainerLocator + container_type=container_type, + display_name=draft.title, + created=container.created, + modified=draft.created, + draft_version_num=draft.version_num, + published_version_num=published.version_num if published else None, + last_published=None if last_publish_log is None else last_publish_log.published_at, + published_by=published_by, + last_draft_created=last_draft_created, + last_draft_created_by=last_draft_created_by, + has_unpublished_changes=authoring_api.contains_unpublished_changes(container.pk), + collections=associated_collections or [], + ) + + +def library_container_locator( + library_key: LibraryLocatorV2, + container: Container, +) -> LibraryContainerLocator: + """ + Returns a LibraryContainerLocator for the given library + container. + + Currently only supports Unit-type containers; will support other container types in future. + """ + assert container.unit is not None + container_type = ContainerType.Unit + + return LibraryContainerLocator( + library_key, + container_type=container_type.value, + container_id=container.publishable_entity.key, + ) + + +def _get_container(container_key: LibraryContainerLocator) -> Container: + """ + Internal method to fetch the Container object from its LibraryContainerLocator + + Raises ContentLibraryContainerNotFound if no container found, or if the container has been soft deleted. + """ + assert isinstance(container_key, LibraryContainerLocator) + content_library = ContentLibrary.objects.get_by_key(container_key.library_key) + learning_package = content_library.learning_package + assert learning_package is not None + container = authoring_api.get_container_by_key( + learning_package.id, + key=container_key.container_id, + ) + if container and container.versioning.draft: + return container + raise ContentLibraryContainerNotFound + + +def get_container(container_key: LibraryContainerLocator) -> ContainerMetadata: + """ + Get a container (a Section, Subsection, or Unit). + """ + container = _get_container(container_key) + container_meta = ContainerMetadata.from_container(container_key.library_key, container) + assert container_meta.container_type.value == container_key.container_type + return container_meta + + +def create_container( + library_key: LibraryLocatorV2, + container_type: ContainerType, + slug: str | None, + title: str, + user_id: int | None, +) -> ContainerMetadata: + """ + Create a container (e.g. a Unit) in the specified content library. + + It will initially be empty. + """ + assert isinstance(library_key, LibraryLocatorV2) + content_library = ContentLibrary.objects.get_by_key(library_key) + assert content_library.learning_package_id # Should never happen but we made this a nullable field so need to check + if slug is None: + # Automatically generate a slug. Append a random suffix so it should be unique. + slug = slugify(title, allow_unicode=True) + '-' + uuid4().hex[-6:] + # Make sure the slug is valid by first creating a key for the new container: + container_key = LibraryContainerLocator( + library_key=library_key, + container_type=container_type.value, + container_id=slug, + ) + # Then try creating the actual container: + match container_type: + case ContainerType.Unit: + container, _initial_version = authoring_api.create_unit_and_version( + content_library.learning_package_id, + key=slug, + title=title, + created=datetime.now(), + created_by=user_id, + ) + case _: + raise ValueError(f"Invalid container type: {container_type}") + + LIBRARY_CONTAINER_CREATED.send_event( + library_container=LibraryContainerData( + library_key=library_key, + container_key=str(container_key), + ) + ) + + return ContainerMetadata.from_container(library_key, container) + + +def update_container( + container_key: LibraryContainerLocator, + display_name: str, + user_id: int | None, +) -> ContainerMetadata: + """ + Update a container (e.g. a Unit) title. + """ + container = _get_container(container_key) + library_key = container_key.library_key + + assert container.unit + unit_version = authoring_api.create_next_unit_version( + container.unit, + title=display_name, + created=datetime.now(), + created_by=user_id, + ) + + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + library_key=library_key, + container_key=str(container_key), + ) + ) + + return ContainerMetadata.from_container(library_key, unit_version.container) + + +def delete_container( + container_key: LibraryContainerLocator, +) -> None: + """ + Delete a container (e.g. a Unit) (soft delete). + + No-op if container doesn't exist or has already been soft-deleted. + """ + try: + container = _get_container(container_key) + except ContentLibraryContainerNotFound: + return + + authoring_api.soft_delete_draft(container.pk) + + LIBRARY_CONTAINER_DELETED.send_event( + library_container=LibraryContainerData( + library_key=container_key.library_key, + container_key=str(container_key), + ) + ) + + # TODO: trigger a LIBRARY_COLLECTION_UPDATED for each collection the container was in + + +def get_container_children( + container_key: LibraryContainerLocator, + published=False, +) -> list[authoring_api.ContainerEntityListEntry]: + """ + Get the entities contained in the given container (e.g. the components/xblocks in a unit) + """ + container = _get_container(container_key) + if container_key.container_type == ContainerType.Unit.value: + child_components = authoring_api.get_components_in_unit(container.unit, published=published) + return [LibraryXBlockMetadata.from_component( + container_key.library_key, + entry.component + ) for entry in child_components] + else: + child_entities = authoring_api.get_entities_in_container(container, published=published) + return [ContainerMetadata.from_container( + container_key.library_key, + entry.entity + ) for entry in child_entities] + + +def get_container_children_count( + container_key: LibraryContainerLocator, + published=False, +) -> int: + """ + Get the count of entities contained in the given container (e.g. the components/xblocks in a unit) + """ + container = _get_container(container_key) + return authoring_api.get_container_children_count(container, published=published) + + +def update_container_children( + container_key: LibraryContainerLocator, + children_ids: list[UsageKeyV2] | list[LibraryContainerLocator], + user_id: int | None, + entities_action: authoring_api.ChildrenEntitiesAction = authoring_api.ChildrenEntitiesAction.REPLACE, +): + """ + Adds children components or containers to given container. + """ + library_key = container_key.library_key + container_type = container_key.container_type + container = _get_container(container_key) + match container_type: + case ContainerType.Unit.value: + components = [get_component_from_usage_key(key) for key in children_ids] # type: ignore[arg-type] + new_version = authoring_api.create_next_unit_version( + container.unit, + components=components, # type: ignore[arg-type] + created=datetime.now(), + created_by=user_id, + entities_action=entities_action, + ) + case _: + raise ValueError(f"Invalid container type: {container_type}") + + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + library_key=library_key, + container_key=str(container_key), + ) + ) + + return ContainerMetadata.from_container(library_key, new_version.container) + + +def get_containers_contains_component( + usage_key: LibraryUsageLocatorV2 +) -> list[ContainerMetadata]: + """ + Get containers that contains the component. + """ + assert isinstance(usage_key, LibraryUsageLocatorV2) + component = get_component_from_usage_key(usage_key) + containers = authoring_api.get_containers_with_entity( + component.publishable_entity.pk, + ) + return [ + ContainerMetadata.from_container(usage_key.context_key, container) + for container in containers + ] diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api/libraries.py similarity index 93% rename from openedx/core/djangoapps/content_libraries/api.py rename to openedx/core/djangoapps/content_libraries/api/libraries.py index 36ace3f84fb2..3884614ae445 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -62,7 +62,7 @@ import requests from django.conf import settings -from django.contrib.auth.models import AbstractUser, Group +from django.contrib.auth.models import AbstractUser, AnonymousUser, Group from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.core.validators import validate_unicode_slug from django.db import IntegrityError, transaction @@ -82,6 +82,7 @@ ContentLibraryData, LibraryBlockData, LibraryCollectionData, + LibraryContainerData, ContentObjectChangedData, ) from openedx_events.content_authoring.signals import ( @@ -92,6 +93,7 @@ LIBRARY_BLOCK_DELETED, LIBRARY_BLOCK_UPDATED, LIBRARY_COLLECTION_UPDATED, + LIBRARY_CONTAINER_UPDATED, CONTENT_OBJECT_ASSOCIATIONS_CHANGED, ) from openedx_learning.api import authoring as authoring_api @@ -113,15 +115,64 @@ xblock_type_display_name, ) from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_learning_core +from openedx.core.djangoapps.content_libraries import api as lib_api from openedx.core.types import User as UserType from xmodule.modulestore.django import modulestore -from . import permissions, tasks -from .constants import ALL_RIGHTS_RESERVED -from .models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask +from .. import permissions, tasks +from ..constants import ALL_RIGHTS_RESERVED +from ..models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask log = logging.getLogger(__name__) +# The public API is only the following symbols: +__all__ = [ + # Exceptions - maybe move them to a new file? + "ContentLibraryNotFound", + "ContentLibraryCollectionNotFound", + "ContentLibraryBlockNotFound", + "LibraryAlreadyExists", + "LibraryCollectionAlreadyExists", + "LibraryBlockAlreadyExists", + "BlockLimitReachedError", + "IncompatibleTypesError", + "InvalidNameError", + "LibraryPermissionIntegrityError", + # Library Models + "ContentLibrary", # Should this be public or not? + "ContentLibraryMetadata", + "AccessLevel", + "ContentLibraryPermissionEntry", + "CollectionMetadata", + # Library API methods + "user_can_create_library", + "get_libraries_for_user", + "get_metadata", + "require_permission_for_library_key", + "get_library", + "create_library", + "get_library_team", + "get_library_user_permissions", + "set_library_user_permissions", + "set_library_group_permissions", + "update_library", + "delete_library", + "get_allowed_block_types", + "publish_changes", + "revert_changes", + # Collections - TODO: move to a new file + "create_library_collection", + "update_library_collection", + "update_library_collection_components", + "set_library_component_collections", + "get_library_collection_usage_key", + "get_library_collection_from_usage_key", + # Import - TODO: move to a new file + "EdxModulestoreImportClient", + "EdxApiImportClient", + "import_blocks_create_task", +] + # Exceptions # ========== @@ -229,24 +280,40 @@ class CollectionMetadata: @dataclass(frozen=True) -class LibraryXBlockMetadata: +class LibraryItem: """ - Class that represents the metadata about an XBlock in a content library. + Common fields for anything that can be found in a content library. """ - usage_key: LibraryUsageLocatorV2 created: datetime modified: datetime + display_name: str + + +@dataclass(frozen=True, kw_only=True) +class PublishableItem(LibraryItem): + """ + Common fields for anything that can be found in a content library that has + draft/publish support. + """ draft_version_num: int published_version_num: int | None = None - display_name: str = "" last_published: datetime | None = None - # THe username of the user who last published this. + # The username of the user who last published this. published_by: str = "" last_draft_created: datetime | None = None # The username of the user who created the last draft. last_draft_created_by: str = "" has_unpublished_changes: bool = False collections: list[CollectionMetadata] = field(default_factory=list) + can_stand_alone: bool = True + + +@dataclass(frozen=True, kw_only=True) +class LibraryXBlockMetadata(PublishableItem): + """ + Class that represents the metadata about an XBlock in a content library. + """ + usage_key: LibraryUsageLocatorV2 @classmethod def from_component(cls, library_key, component, associated_collections=None): @@ -280,6 +347,7 @@ def from_component(cls, library_key, component, associated_collections=None): last_draft_created_by=last_draft_created_by, has_unpublished_changes=component.versioning.has_unpublished_changes, collections=associated_collections or [], + can_stand_alone=component.publishable_entity.can_stand_alone, ) @@ -416,6 +484,7 @@ def get_library(library_key: LibraryLocatorV2) -> ContentLibraryMetadata: """ ref = ContentLibrary.objects.get_by_key(library_key) learning_package = ref.learning_package + assert learning_package is not None # Shouldn't happen - this is just for the type checker num_blocks = authoring_api.get_all_drafts(learning_package.id).count() last_publish_log = authoring_api.get_last_publish(learning_package.id) last_draft_log = authoring_api.get_entities_with_unpublished_changes(learning_package.id) \ @@ -455,7 +524,7 @@ def get_library(library_key: LibraryLocatorV2) -> ContentLibraryMetadata: return ContentLibraryMetadata( key=library_key, title=learning_package.title, - description=ref.learning_package.description, + description=learning_package.description, num_blocks=num_blocks, version=version, last_published=None if last_publish_log is None else last_publish_log.published_at, @@ -557,6 +626,8 @@ def get_library_user_permissions(library_key: LibraryLocatorV2, user: UserType) Fetch the specified user's access information. Will return None if no permissions have been granted. """ + if isinstance(user, AnonymousUser): + return None # Mostly here for the type checker ref = ContentLibrary.objects.get_by_key(library_key) grant = ref.permission_grants.filter(user=user).first() if grant is None: @@ -574,6 +645,8 @@ def set_library_user_permissions(library_key: LibraryLocatorV2, user: UserType, access_level should be one of the AccessLevel values defined above. """ + if isinstance(user, AnonymousUser): + raise TypeError("Invalid user type") # Mostly here for the type checker ref = ContentLibrary.objects.get_by_key(library_key) current_grant = get_library_user_permissions(library_key, user) if current_grant and current_grant.access_level == AccessLevel.ADMIN_LEVEL: @@ -633,6 +706,8 @@ def update_library( return content_lib = ContentLibrary.objects.get_by_key(library_key) + learning_package_id = content_lib.learning_package_id + assert learning_package_id is not None with transaction.atomic(): # We need to make updates to both the ContentLibrary and its linked @@ -643,12 +718,12 @@ def update_library( if allow_public_read is not None: content_lib.allow_public_read = allow_public_read if library_license is not None: - content_lib.library_license = library_license + content_lib.license = library_license content_lib.save() if learning_pkg_changed: authoring_api.update_learning_package( - content_lib.learning_package_id, + learning_package_id, title=title, description=description, ) @@ -675,7 +750,8 @@ def delete_library(library_key: LibraryLocatorV2) -> None: # TODO: We should eventually detach the LearningPackage and delete it # asynchronously, especially if we need to delete a bunch of stuff # on the filesystem for it. - learning_package.delete() + if learning_package: + learning_package.delete() CONTENT_LIBRARY_DELETED.send_event( content_library=ContentLibraryData( @@ -709,6 +785,7 @@ def get_library_components( """ lib = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] learning_package = lib.learning_package + assert learning_package is not None components = authoring_api.get_components( learning_package.id, draft=True, @@ -827,6 +904,18 @@ def set_library_block_olx(usage_key: LibraryUsageLocatorV2, new_olx_str: str) -> ) ) + # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger + # container indexing asynchronously. + affected_containers = lib_api.get_containers_contains_component(usage_key) + for container in affected_containers: + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + library_key=usage_key.lib_key, + container_key=str(container.container_key), + background=True, + ) + ) + return new_component_version @@ -860,7 +949,8 @@ def validate_can_add_block_to_library( content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] # If adding a component would take us over our max, return an error. - component_count = authoring_api.get_all_drafts(content_library.learning_package.id).count() + assert content_library.learning_package_id is not None + component_count = authoring_api.get_all_drafts(content_library.learning_package_id).count() if component_count + 1 > settings.MAX_BLOCKS_PER_CONTENT_LIBRARY: raise BlockLimitReachedError( _("Library cannot have more than {} Components").format( @@ -885,9 +975,17 @@ def validate_can_add_block_to_library( return content_library, usage_key -def create_library_block(library_key, block_type, definition_id, user_id=None): +def create_library_block( + library_key: LibraryLocatorV2, + block_type: str, + definition_id: str, + user_id: int | None = None, + can_stand_alone: bool = True, +): """ Create a new XBlock in this library of the specified type (e.g. "html"). + + Set can_stand_alone = False when a component is created under a container, like unit. """ # It's in the serializer as ``definition_id``, but for our purposes, it's # the block_id. See the comments in ``LibraryXBlockCreationSerializer`` for @@ -896,7 +994,7 @@ def create_library_block(library_key, block_type, definition_id, user_id=None): content_library, usage_key = validate_can_add_block_to_library(library_key, block_type, block_id) - _create_component_for_block(content_library, usage_key, user_id) + _create_component_for_block(content_library, usage_key, user_id, can_stand_alone) # Now return the metadata about the new block: LIBRARY_BLOCK_CREATED.send_event( @@ -1062,6 +1160,7 @@ def _create_component_for_block( content_lib: ContentLibrary, usage_key: LibraryUsageLocatorV2, user_id: int | None = None, + can_stand_alone: bool = True, ): """ Create a Component for an XBlock type, initialize it, and return the ComponentVersion. @@ -1071,6 +1170,8 @@ def _create_component_for_block( will be set as the current draft. This function does not publish the Component. + Set can_stand_alone = False when a component is created under a container, like unit. + TODO: We should probably shift this to openedx.core.djangoapps.xblock.api (along with its caller) since it gives runtime storage specifics. The Library-specific logic stays in this module, so "create a block for my lib" @@ -1095,6 +1196,7 @@ def _create_component_for_block( title=display_name, created=now, created_by=user_id, + can_stand_alone=can_stand_alone, ) content = authoring_api.get_or_create_text_content( learning_package.id, @@ -1118,6 +1220,7 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr component = get_component_from_usage_key(usage_key) library_key = usage_key.context_key affected_collections = authoring_api.get_entity_collections(component.learning_package_id, component.key) + affected_containers = lib_api.get_containers_contains_component(usage_key) authoring_api.soft_delete_draft(component.pk) @@ -1141,6 +1244,19 @@ def delete_library_block(usage_key: LibraryUsageLocatorV2, remove_from_parent=Tr ) ) + # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger + # container indexing asynchronously. + # + # To update the components count in containers + for container in affected_containers: + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + library_key=library_key, + container_key=str(container.container_key), + background=True, + ) + ) + def restore_library_block(usage_key: LibraryUsageLocatorV2) -> None: """ @@ -1356,7 +1472,7 @@ def publish_changes(library_key: LibraryLocatorV2, user_id: int | None = None): Publish all pending changes to the specified library. """ learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package - + assert learning_package is not None # shouldn't happen but it's technically possible. authoring_api.publish_all_drafts(learning_package.id, published_by=user_id) CONTENT_LIBRARY_UPDATED.send_event( @@ -1398,6 +1514,7 @@ def revert_changes(library_key: LibraryLocatorV2) -> None: last published version. """ learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package + assert learning_package is not None # shouldn't happen but it's technically possible. authoring_api.reset_drafts_to_published(learning_package.id) CONTENT_LIBRARY_UPDATED.send_event( @@ -1652,6 +1769,7 @@ def get_library_collection_from_usage_key( library_key = collection_usage_key.library_key collection_key = collection_usage_key.collection_id content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] + assert content_library.learning_package_id is not None # shouldn't happen but it's technically possible. try: return authoring_api.get_collection( content_library.learning_package_id, diff --git a/openedx/core/djangoapps/content_libraries/api/permissions.py b/openedx/core/djangoapps/content_libraries/api/permissions.py new file mode 100644 index 000000000000..6064b80d6f9e --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/permissions.py @@ -0,0 +1,14 @@ +""" +Public permissions that are part of the content libraries API +""" +# pylint: disable=unused-import + +from ..permissions import ( + CAN_CREATE_CONTENT_LIBRARY, + CAN_DELETE_THIS_CONTENT_LIBRARY, + CAN_EDIT_THIS_CONTENT_LIBRARY, + CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM, + CAN_LEARN_FROM_THIS_CONTENT_LIBRARY, + CAN_VIEW_THIS_CONTENT_LIBRARY, + CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM +) diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py index 4bda10eb12fa..d5f121d8396e 100644 --- a/openedx/core/djangoapps/content_libraries/library_context.py +++ b/openedx/core/djangoapps/content_libraries/library_context.py @@ -6,8 +6,8 @@ from django.core.exceptions import PermissionDenied from rest_framework.exceptions import NotFound -from openedx_events.content_authoring.data import LibraryBlockData -from openedx_events.content_authoring.signals import LIBRARY_BLOCK_UPDATED +from openedx_events.content_authoring.data import LibraryBlockData, LibraryContainerData +from openedx_events.content_authoring.signals import LIBRARY_BLOCK_UPDATED, LIBRARY_CONTAINER_UPDATED from opaque_keys.edx.keys import UsageKeyV2 from opaque_keys.edx.locator import LibraryUsageLocatorV2, LibraryLocatorV2 from openedx_learning.api import authoring as authoring_api @@ -114,3 +114,19 @@ def send_block_updated_event(self, usage_key: UsageKeyV2): usage_key=usage_key, ) ) + + def send_container_updated_events(self, usage_key: UsageKeyV2): + """ + Send "container updated" events for containers that contains the library block + with the given usage_key. + """ + assert isinstance(usage_key, LibraryUsageLocatorV2) + affected_containers = api.get_containers_contains_component(usage_key) + for container in affected_containers: + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + library_key=usage_key.lib_key, + container_key=str(container.container_key), + background=True, + ) + ) diff --git a/openedx/core/djangoapps/content_libraries/models.py b/openedx/core/djangoapps/content_libraries/models.py index 61e28b944851..415821145605 100644 --- a/openedx/core/djangoapps/content_libraries/models.py +++ b/openedx/core/djangoapps/content_libraries/models.py @@ -36,6 +36,7 @@ import contextlib import logging +from typing import ClassVar import uuid from django.contrib.auth import get_user_model @@ -67,11 +68,11 @@ User = get_user_model() -class ContentLibraryManager(models.Manager): +class ContentLibraryManager(models.Manager["ContentLibrary"]): """ Custom manager for ContentLibrary class. """ - def get_by_key(self, library_key): + def get_by_key(self, library_key) -> "ContentLibrary": """ Get the ContentLibrary for the given LibraryLocatorV2 key. """ @@ -92,7 +93,7 @@ class ContentLibrary(models.Model): .. no_pii: """ - objects: ContentLibraryManager[ContentLibrary] = ContentLibraryManager() + objects: ClassVar[ContentLibraryManager] = ContentLibraryManager() id = models.AutoField(primary_key=True) # Every Library is uniquely and permanently identified by an 'org' and a diff --git a/openedx/core/djangoapps/content_libraries/rest_api/__init__.py b/openedx/core/djangoapps/content_libraries/rest_api/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py new file mode 100644 index 000000000000..6c24c35394b9 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -0,0 +1,19 @@ +""" +Content Library REST APIs related to XBlocks/Components and their static assets +""" +# pylint: disable=unused-import + +# TODO: move the block and block asset related views from 'libraries' into this file +from .libraries import ( + LibraryBlockAssetListView, + LibraryBlockAssetView, + LibraryBlockCollectionsView, + LibraryBlockLtiUrlView, + LibraryBlockOlxView, + LibraryBlockPublishView, + LibraryBlockRestore, + LibraryBlocksView, + LibraryBlockView, + LibraryComponentAssetView, + LibraryComponentDraftAssetView, +) diff --git a/openedx/core/djangoapps/content_libraries/views_collections.py b/openedx/core/djangoapps/content_libraries/rest_api/collections.py similarity index 94% rename from openedx/core/djangoapps/content_libraries/views_collections.py rename to openedx/core/djangoapps/content_libraries/rest_api/collections.py index 21c4b12dd3da..c49822ae2f0f 100644 --- a/openedx/core/djangoapps/content_libraries/views_collections.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/collections.py @@ -1,7 +1,6 @@ """ Collections API Views """ - from __future__ import annotations from django.db.models import QuerySet @@ -17,13 +16,13 @@ from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Collection -from openedx.core.djangoapps.content_libraries import api, permissions -from openedx.core.djangoapps.content_libraries.models import ContentLibrary -from openedx.core.djangoapps.content_libraries.views import convert_exceptions -from openedx.core.djangoapps.content_libraries.serializers import ( +from .. import api, permissions +from ..models import ContentLibrary +from .utils import convert_exceptions +from .serializers import ( ContentLibraryCollectionSerializer, - ContentLibraryCollectionComponentsUpdateSerializer, ContentLibraryCollectionUpdateSerializer, + ContentLibraryComponentKeysSerializer, ) from openedx.core.types.http import RestRequest @@ -201,7 +200,7 @@ def update_components(self, request: RestRequest, *args, **kwargs) -> Response: content_library = self.get_content_library() collection_key = kwargs["key"] - serializer = ContentLibraryCollectionComponentsUpdateSerializer(data=request.data) + serializer = ContentLibraryComponentKeysSerializer(data=request.data) serializer.is_valid(raise_exception=True) usage_keys = serializer.validated_data["usage_keys"] diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py new file mode 100644 index 000000000000..95e468b4a43a --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -0,0 +1,275 @@ +""" +REST API views for containers (sections, subsections, units) in content libraries +""" +from __future__ import annotations + +import logging + +from django.contrib.auth import get_user_model +from django.db.transaction import non_atomic_requests +from django.utils.decorators import method_decorator +from drf_yasg.utils import swagger_auto_schema + +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator +from openedx_learning.api import authoring as authoring_api +from rest_framework.generics import GenericAPIView +from rest_framework.response import Response +from rest_framework.status import HTTP_204_NO_CONTENT + +from openedx.core.djangoapps.content_libraries import api, permissions +from openedx.core.lib.api.view_utils import view_auth_classes +from . import serializers +from .utils import convert_exceptions + +User = get_user_model() +log = logging.getLogger(__name__) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainersView(GenericAPIView): + """ + Views to work with Containers in a specific content library. + """ + serializer_class = serializers.LibraryContainerMetadataSerializer + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.LibraryContainerMetadataSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def post(self, request, lib_key_str): + """ + Create a new Container in this content library + """ + library_key = LibraryLocatorV2.from_string(lib_key_str) + api.require_permission_for_library_key(library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) + serializer = serializers.LibraryContainerMetadataSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + container_type = serializer.validated_data['container_type'] + container = api.create_container( + library_key, + container_type, + title=serializer.validated_data['display_name'], + slug=serializer.validated_data.get('slug'), + user_id=request.user.id, + ) + + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerView(GenericAPIView): + """ + View to retrieve or update data about a specific container (a section, subsection, or unit) + """ + serializer_class = serializers.LibraryContainerMetadataSerializer + + @convert_exceptions + @swagger_auto_schema( + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def get(self, request, container_key: LibraryContainerLocator): + """ + Get information about a container + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, + ) + container = api.get_container(container_key) + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.LibraryContainerUpdateSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def patch(self, request, container_key: LibraryContainerLocator): + """ + Update a Container. + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + serializer = serializers.LibraryContainerUpdateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + container = api.update_container( + container_key, + display_name=serializer.validated_data['display_name'], + user_id=request.user.id, + ) + + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + @convert_exceptions + def delete(self, request, container_key: LibraryContainerLocator): + """ + Delete a Container (soft delete). + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + + api.delete_container( + container_key, + ) + + return Response({}, status=HTTP_204_NO_CONTENT) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerChildrenView(GenericAPIView): + """ + View to get or update children of specific container (a section, subsection, or unit) + """ + serializer_class = serializers.LibraryXBlockMetadataSerializer + + @convert_exceptions + @swagger_auto_schema( + responses={200: list[serializers.LibraryXBlockMetadataSerializer]} + ) + def get(self, request, container_key: LibraryContainerLocator): + """ + Get children components of given container + Example: + GET /api/libraries/v2/containers//children/ + Result: + [ + { + 'block_type': 'problem', + 'can_stand_alone': True, + 'collections': [], + 'created': '2025-03-21T13:53:55Z', + 'def_key': None, + 'display_name': 'Blank Problem', + 'has_unpublished_changes': True, + 'id': 'lb:CL-TEST:containers:problem:Problem1', + 'last_draft_created': '2025-03-21T13:53:55Z', + 'last_draft_created_by': 'Bob', + 'last_published': None, + 'modified': '2025-03-21T13:53:55Z', + 'published_by': None, + }, + { + 'block_type': 'html', + 'can_stand_alone': False, + 'collections': [], + 'created': '2025-03-21T13:53:55Z', + 'def_key': None, + 'display_name': 'Text', + 'has_unpublished_changes': True, + 'id': 'lb:CL-TEST:containers:html:Html1', + 'last_draft_created': '2025-03-21T13:53:55Z', + 'last_draft_created_by': 'Bob', + 'last_published': None, + 'modified': '2025-03-21T13:53:55Z', + 'published_by': None, + } + ] + """ + published = request.GET.get('published', False) + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, + ) + child_entities = api.get_container_children(container_key, published) + if container_key.container_type == api.ContainerType.Unit.value: + data = serializers.LibraryXBlockMetadataSerializer(child_entities, many=True).data + else: + data = serializers.LibraryContainerMetadataSerializer(child_entities, many=True).data + return Response(data) + + def _update_component_children( + self, + request, + container_key: LibraryContainerLocator, + action: authoring_api.ChildrenEntitiesAction, + ): + """ + Helper function to update children in container. + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + serializer = serializers.ContentLibraryComponentKeysSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + # Only components under units are supported for now. + assert container_key.container_type == api.ContainerType.Unit.value + + container = api.update_container_children( + container_key, + children_ids=serializer.validated_data["usage_keys"], + user_id=request.user.id, + entities_action=action, + ) + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.ContentLibraryComponentKeysSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def post(self, request, container_key: LibraryContainerLocator): + """ + Add components to unit + Example: + POST /api/libraries/v2/containers//children/ + Request body: + {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} + """ + return self._update_component_children( + request, + container_key, + action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.ContentLibraryComponentKeysSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def delete(self, request, container_key: LibraryContainerLocator): + """ + Remove components from unit + Example: + DELETE /api/libraries/v2/containers//children/ + Request body: + {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} + """ + return self._update_component_children( + request, + container_key, + action=authoring_api.ChildrenEntitiesAction.REMOVE, + ) + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.ContentLibraryComponentKeysSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def patch(self, request, container_key: LibraryContainerLocator): + """ + Replace components in unit, can be used to reorder components as well. + Example: + PATCH /api/libraries/v2/containers//children/ + Request body: + {"usage_keys": ['lb:CL-TEST:containers:problem:Problem1', 'lb:CL-TEST:containers:html:Html1']} + """ + return self._update_component_children( + request, + container_key, + action=authoring_api.ChildrenEntitiesAction.REPLACE, + ) diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py similarity index 96% rename from openedx/core/djangoapps/content_libraries/views.py rename to openedx/core/djangoapps/content_libraries/rest_api/libraries.py index a1b7a2602450..5e370dc40e33 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py @@ -62,8 +62,6 @@ to Learning Core) atomic: https://github.com/openedx/edx-platform/pull/30456 """ - -from functools import wraps import itertools import json import logging @@ -86,7 +84,6 @@ from pylti1p3.exception import LtiException, OIDCException import edx_api_doc_tools as apidocs -from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_learning.api import authoring from organizations.api import ensure_organization @@ -105,7 +102,7 @@ user_can_create_organizations, ) from openedx.core.djangoapps.content_libraries import api, permissions -from openedx.core.djangoapps.content_libraries.serializers import ( +from openedx.core.djangoapps.content_libraries.rest_api.serializers import ( ContentLibraryBlockImportTaskCreateSerializer, ContentLibraryBlockImportTaskSerializer, ContentLibraryFilterSerializer, @@ -129,50 +126,14 @@ from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.types.http import RestRequest -from .models import ContentLibrary, LtiGradedResource, LtiProfile +from .utils import convert_exceptions +from ..models import ContentLibrary, LtiGradedResource, LtiProfile User = get_user_model() log = logging.getLogger(__name__) -def convert_exceptions(fn): - """ - Catch any Content Library API exceptions that occur and convert them to - DRF exceptions so DRF will return an appropriate HTTP response - """ - - @wraps(fn) - def wrapped_fn(*args, **kwargs): - try: - return fn(*args, **kwargs) - except InvalidKeyError as exc: - log.exception(str(exc)) - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryNotFound: - log.exception("Content library not found") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryBlockNotFound: - log.exception("XBlock not found in content library") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryCollectionNotFound: - log.exception("Collection not found in content library") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.LibraryCollectionAlreadyExists as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.LibraryBlockAlreadyExists as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.InvalidNameError as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.BlockLimitReachedError as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - return wrapped_fn - - class LibraryApiPaginationDocs: """ API docs for query params related to paginating ContentLibraryMetadata objects. diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py similarity index 81% rename from openedx/core/djangoapps/content_libraries/serializers.py rename to openedx/core/djangoapps/content_libraries/rest_api/serializers.py index c2a26220d4c7..46bec36e3c94 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -10,6 +10,7 @@ from opaque_keys import InvalidKeyError from openedx_learning.api.authoring_models import Collection +from openedx.core.djangoapps.content_libraries.api.containers import ContainerMetadata, ContainerType from openedx.core.djangoapps.content_libraries.constants import ( ALL_RIGHTS_RESERVED, LICENSE_OPTIONS, @@ -19,7 +20,7 @@ ContentLibrary ) from openedx.core.lib.api.serializers import CourseKeyField -from . import permissions +from .. import permissions DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%SZ' @@ -158,6 +159,7 @@ class LibraryXBlockMetadataSerializer(serializers.Serializer): tags_count = serializers.IntegerField(read_only=True) collections = CollectionMetadataSerializer(many=True, required=False) + can_stand_alone = serializers.BooleanField(read_only=True) class LibraryXBlockTypeSerializer(serializers.Serializer): @@ -192,6 +194,9 @@ class LibraryXBlockCreationSerializer(serializers.Serializer): # creating new block from scratch staged_content = serializers.CharField(required=False) + # Optional param defaults to True, set to False if block is being created under a container. + can_stand_alone = serializers.BooleanField(required=False, default=True) + class LibraryPasteClipboardSerializer(serializers.Serializer): """ @@ -230,6 +235,52 @@ class LibraryXBlockStaticFilesSerializer(serializers.Serializer): files = LibraryXBlockStaticFileSerializer(many=True) +class LibraryContainerMetadataSerializer(serializers.Serializer): + """ + Serializer for Containers like Sections, Subsections, Units + + Converts from ContainerMetadata to JSON-compatible data + """ + container_key = serializers.CharField(read_only=True) + container_type = serializers.CharField() + display_name = serializers.CharField() + last_published = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + published_by = serializers.CharField(read_only=True) + last_draft_created = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + last_draft_created_by = serializers.CharField(read_only=True) + has_unpublished_changes = serializers.BooleanField(read_only=True) + created = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + modified = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + tags_count = serializers.IntegerField(read_only=True) + collections = CollectionMetadataSerializer(many=True, required=False, read_only=True) + + # When creating a new container in a library, the slug becomes the ID part of + # the definition key and usage key: + slug = serializers.CharField(write_only=True, required=False) + + def to_representation(self, instance: ContainerMetadata): + """ Convert to JSON-serializable data types """ + data = super().to_representation(instance) + data["container_type"] = instance.container_type.value # Force to a string, not an enum value instance + return data + + def to_internal_value(self, data): + """ + Convert JSON-ish data back to native python types. + Returns a dictionary, not a ContainerMetadata instance. + """ + result = super().to_internal_value(data) + result["container_type"] = ContainerType(data["container_type"]) + return result + + +class LibraryContainerUpdateSerializer(serializers.Serializer): + """ + Serializer for updating metadata for Containers like Sections, Subsections, Units + """ + display_name = serializers.CharField() + + class ContentLibraryBlockImportTaskSerializer(serializers.ModelSerializer): """ Serializer for a Content Library block import task. @@ -298,7 +349,7 @@ def to_internal_value(self, value: str) -> UsageKeyV2: raise ValidationError from err -class ContentLibraryCollectionComponentsUpdateSerializer(serializers.Serializer): +class ContentLibraryComponentKeysSerializer(serializers.Serializer): """ Serializer for adding/removing Components to/from a Collection. """ diff --git a/openedx/core/djangoapps/content_libraries/rest_api/url_converters.py b/openedx/core/djangoapps/content_libraries/rest_api/url_converters.py new file mode 100644 index 000000000000..c8c4b1c6ebda --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/url_converters.py @@ -0,0 +1,23 @@ +""" +URL pattern converters +https://docs.djangoproject.com/en/5.1/topics/http/urls/#registering-custom-path-converters +""" +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import LibraryContainerLocator + + +class LibraryContainerLocatorConverter: + """ + Converter that matches library container IDs like: + lct:CL-TEST:containers:unit:u1 + """ + regex = r'[\w-]+(:[\w\-.]+)+' + + def to_python(self, value: str) -> LibraryContainerLocator: + try: + return LibraryContainerLocator.from_string(value) + except InvalidKeyError as exc: + raise ValueError from exc + + def to_url(self, value: LibraryContainerLocator) -> str: + return str(value) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/utils.py b/openedx/core/djangoapps/content_libraries/rest_api/utils.py new file mode 100644 index 000000000000..99825fbe75f8 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/utils.py @@ -0,0 +1,52 @@ +""" +REST API utilities for content libraries +""" +from functools import wraps +import logging + +from opaque_keys import InvalidKeyError +from rest_framework.exceptions import NotFound, ValidationError + +from .. import api + +log = logging.getLogger(__name__) + + +def convert_exceptions(fn): + """ + Catch any Content Library API exceptions that occur and convert them to + DRF exceptions so DRF will return an appropriate HTTP response + """ + + @wraps(fn) + def wrapped_fn(*args, **kwargs): + try: + return fn(*args, **kwargs) + except InvalidKeyError as exc: + log.exception(str(exc)) + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryNotFound: + log.exception("Content library not found") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryBlockNotFound: + log.exception("XBlock not found in content library") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryCollectionNotFound: + log.exception("Collection not found in content library") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryContainerNotFound: + log.exception("Container not found in content library") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.LibraryCollectionAlreadyExists as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + except api.LibraryBlockAlreadyExists as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + except api.InvalidNameError as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + except api.BlockLimitReachedError as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + return wrapped_fn diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 77de1c028aa7..6adb8184ea00 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -22,6 +22,7 @@ URL_LIB_LINKS = URL_LIB_DETAIL + 'links/' # Get the list of links in this library, or add a new one URL_LIB_COMMIT = URL_LIB_DETAIL + 'commit/' # Commit (POST) or revert (DELETE) all pending changes to this library URL_LIB_BLOCKS = URL_LIB_DETAIL + 'blocks/' # Get the list of XBlocks in this library, or add a new one +URL_LIB_CONTAINERS = URL_LIB_DETAIL + 'containers/' # Create a new container in this library URL_LIB_TEAM = URL_LIB_DETAIL + 'team/' # Get the list of users/groups authorized to use this library URL_LIB_TEAM_USER = URL_LIB_TEAM + 'user/{username}/' # Add/edit/remove a user's permission to use this library URL_LIB_TEAM_GROUP = URL_LIB_TEAM + 'group/{group_name}/' # Add/edit/remove a group's permission to use this library @@ -31,6 +32,8 @@ URL_LIB_BLOCK_OLX = URL_LIB_BLOCK + 'olx/' # Get or set the OLX of the specified XBlock URL_LIB_BLOCK_ASSETS = URL_LIB_BLOCK + 'assets/' # List the static asset files of the specified XBlock URL_LIB_BLOCK_ASSET_FILE = URL_LIB_BLOCK + 'assets/{file_name}' # Get, delete, or upload a specific static asset file +URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library +URL_LIB_CONTAINER_COMPONENTS = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container URL_LIB_LTI_PREFIX = URL_PREFIX + 'lti/1.3/' URL_LIB_LTI_JWKS = URL_LIB_LTI_PREFIX + 'pub/jwks/' @@ -227,9 +230,21 @@ def _get_library_blocks(self, lib_key, query_params_dict=None, expect_response=2 expect_response ) - def _add_block_to_library(self, lib_key, block_type, slug, parent_block=None, expect_response=200): + def _add_block_to_library( + self, + lib_key, + block_type, + slug, + parent_block=None, + can_stand_alone=True, + expect_response=200, + ): """ Add a new XBlock to the library """ - data = {"block_type": block_type, "definition_id": slug} + data = { + "block_type": block_type, + "definition_id": slug, + "can_stand_alone": can_stand_alone, + } if parent_block: data["parent_block"] = parent_block return self._api('post', URL_LIB_BLOCKS.format(lib_key=lib_key), data, expect_response) @@ -350,3 +365,74 @@ def _get_library_block_fields(self, block_key, version=None, expect_response=200 def _set_library_block_fields(self, block_key, new_fields, expect_response=200): """ Set the fields of a specific block in the library. This API is only used by the MFE editors. """ return self._api('post', URL_BLOCK_FIELDS_URL.format(block_key=block_key), new_fields, expect_response) + + def _create_container(self, lib_key, container_type, slug: str | None, display_name: str, expect_response=200): + """ Create a container (unit etc.) """ + data = {"container_type": container_type, "display_name": display_name} + if slug: + data["slug"] = slug + return self._api('post', URL_LIB_CONTAINERS.format(lib_key=lib_key), data, expect_response) + + def _get_container(self, container_key: str, expect_response=200): + """ Get a container (unit etc.) """ + return self._api('get', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response) + + def _update_container(self, container_key: str, display_name: str, expect_response=200): + """ Update a container (unit etc.) """ + data = {"display_name": display_name} + return self._api('patch', URL_LIB_CONTAINER.format(container_key=container_key), data, expect_response) + + def _delete_container(self, container_key: str, expect_response=204): + """ Delete a container (unit etc.) """ + return self._api('delete', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response) + + def _get_container_components(self, container_key: str, expect_response=200): + """ Get container components""" + return self._api( + 'get', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + None, + expect_response + ) + + def _add_container_components( + self, + container_key: str, + children_ids: list[str], + expect_response=200, + ): + """ Add container components""" + return self._api( + 'post', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + {'usage_keys': children_ids}, + expect_response + ) + + def _remove_container_components( + self, + container_key: str, + children_ids: list[str], + expect_response=200, + ): + """ Remove container components""" + return self._api( + 'delete', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + {'usage_keys': children_ids}, + expect_response + ) + + def _patch_container_components( + self, + container_key: str, + children_ids: list[str], + expect_response=200, + ): + """ Update container components""" + return self._api( + 'patch', + URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + {'usage_keys': children_ids}, + expect_response + ) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 203cc7a9397a..7ea797dfbb8d 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -5,6 +5,7 @@ import base64 import hashlib from unittest import mock +from datetime import datetime, timezone from django.test import TestCase @@ -16,12 +17,14 @@ from openedx_events.content_authoring.data import ( ContentObjectChangedData, LibraryCollectionData, + LibraryContainerData, ) from openedx_events.content_authoring.signals import ( CONTENT_OBJECT_ASSOCIATIONS_CHANGED, LIBRARY_COLLECTION_CREATED, LIBRARY_COLLECTION_DELETED, LIBRARY_COLLECTION_UPDATED, + LIBRARY_CONTAINER_UPDATED, ) from openedx_events.tests.utils import OpenEdxEventsTestMixin from openedx_learning.api import authoring as authoring_api @@ -65,11 +68,11 @@ def test_import_blocks_from_course_without_course(self): with self.assertRaises(ValueError): self.client.import_blocks_from_course('foobar', lambda *_: None) - @mock.patch('openedx.core.djangoapps.content_libraries.api.create_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block_static_asset_files') - @mock.patch('openedx.core.djangoapps.content_libraries.api.publish_changes') - @mock.patch('openedx.core.djangoapps.content_libraries.api.set_library_block_olx') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.create_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block_static_asset_files') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.publish_changes') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.set_library_block_olx') def test_import_blocks_from_course_on_block_with_olx( self, mock_set_library_block_olx, @@ -101,9 +104,9 @@ def test_import_blocks_from_course_on_block_with_olx( mock.ANY, 'fake-olx') mock_publish_changes.assert_called_once() - @mock.patch('openedx.core.djangoapps.content_libraries.api.create_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block_static_asset_files') - @mock.patch('openedx.core.djangoapps.content_libraries.api.set_library_block_olx') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.create_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block_static_asset_files') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.set_library_block_olx') def test_import_block_when_called_twice_same_block_but_different_course( self, mock_set_library_block_olx, @@ -138,7 +141,7 @@ def test_import_block_when_called_twice_same_block_but_different_course( mock_set_library_block_olx.assert_called_once() -@mock.patch('openedx.core.djangoapps.content_libraries.api.OAuthAPIClient') +@mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.OAuthAPIClient') class EdxApiImportClientTest(TestCase): """ Tests for EdxApiImportClient. @@ -195,11 +198,11 @@ def mock_oauth_client_response(self, mock_oauth_client, *, content=None, excepti return mock_response, mock_content return mock_response - @mock.patch('openedx.core.djangoapps.content_libraries.api.add_library_block_static_asset_file') - @mock.patch('openedx.core.djangoapps.content_libraries.api.create_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block_static_asset_files') - @mock.patch('openedx.core.djangoapps.content_libraries.api.publish_changes') - @mock.patch('openedx.core.djangoapps.content_libraries.api.set_library_block_olx') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.add_library_block_static_asset_file') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.create_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block_static_asset_files') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.publish_changes') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.set_library_block_olx') def test_import_block_when_url_is_from_studio( self, mock_set_library_block_olx, @@ -742,3 +745,107 @@ def test_delete_component_and_revert(self): }, event_receiver.call_args_list[1].kwargs, ) + + +class ContentLibraryContainersTest(ContentLibrariesRestApiTest, TestCase): + """ + Tests for Content Library API containers methods. + """ + def setUp(self): + super().setUp() + + # Create Content Libraries + self._create_library("test-lib-cont-1", "Test Library 1") + + # Fetch the created ContentLibrare objects so we can access their learning_package.id + self.lib1 = ContentLibrary.objects.get(slug="test-lib-cont-1") + + # Create Units + self.unit1 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-1', 'Unit 1', None) + self.unit2 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-2', 'Unit 2', None) + + # Create XBlocks + # Create some library blocks in lib1 + self.problem_block = self._add_block_to_library( + self.lib1.library_key, "problem", "problem1", + ) + self.problem_block_usage_key = UsageKey.from_string(self.problem_block["id"]) + self.html_block = self._add_block_to_library( + self.lib1.library_key, "html", "html1", + ) + self.html_block_usage_key = UsageKey.from_string(self.html_block["id"]) + + # Add content to units + api.update_container_children( + self.unit1.container_key, + [self.problem_block_usage_key, self.html_block_usage_key], + None, + ) + api.update_container_children( + self.unit2.container_key, + [self.html_block_usage_key], + None, + ) + + def test_get_containers_contains_component(self): + problem_block_containers = api.get_containers_contains_component(self.problem_block_usage_key) + html_block_containers = api.get_containers_contains_component(self.html_block_usage_key) + + assert len(problem_block_containers) == 1 + assert problem_block_containers[0].container_key == self.unit1.container_key + + assert len(html_block_containers) == 2 + assert html_block_containers[0].container_key == self.unit1.container_key + assert html_block_containers[1].container_key == self.unit2.container_key + + + def _validate_calls_of_html_block(self, event_mock): + assert event_mock.call_count == 2 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + library_key=self.lib1.library_key, + container_key=str(self.unit1.container_key), + background=True, + ) + }, + event_mock.call_args_list[0].kwargs, + ) + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + library_key=self.lib1.library_key, + container_key=str(self.unit2.container_key), + background=True, + ) + }, + event_mock.call_args_list[1].kwargs, + ) + + def test_call_container_update_signal_when_delete_component(self): + container_update_event_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(container_update_event_receiver) + + api.delete_library_block(self.html_block_usage_key) + self._validate_calls_of_html_block(container_update_event_receiver) + + + def test_call_container_update_signal_when_update_olx(self): + block_olx = "Hello world!" + container_update_event_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(container_update_event_receiver) + + self._set_library_block_olx(self.html_block_usage_key, block_olx) + self._validate_calls_of_html_block(container_update_event_receiver) + + def test_call_container_update_signal_when_update_component(self): + block_olx = "Hello world!" + container_update_event_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(container_update_event_receiver) + + self._set_library_block_fields(self.html_block_usage_key, {"data": block_olx, "metadata": {}}) + self._validate_calls_of_html_block(container_update_event_receiver) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py new file mode 100644 index 000000000000..52546396f2bb --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -0,0 +1,332 @@ +""" +Tests for Learning-Core-based Content Libraries +""" +from datetime import datetime, timezone +from unittest import mock + +import ddt +from freezegun import freeze_time + +from opaque_keys.edx.locator import LibraryLocatorV2 +from openedx_events.content_authoring.data import LibraryContainerData +from openedx_events.content_authoring.signals import ( + LIBRARY_CONTAINER_CREATED, + LIBRARY_CONTAINER_DELETED, + LIBRARY_CONTAINER_UPDATED, +) +from openedx_events.tests.utils import OpenEdxEventsTestMixin + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest +from openedx.core.djangolib.testing.utils import skip_unless_cms + + +@skip_unless_cms +@ddt.ddt +class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): + """ + Tests for containers (Sections, Subsections, Units) in Content Libraries. + + These tests use the REST API, which in turn relies on the Python API. + Some tests may use the python API directly if necessary to provide + coverage of any code paths not accessible via the REST API. + + In general, these tests should + (1) Use public APIs only - don't directly create data using other methods, + which results in a less realistic test and ties the test suite too + closely to specific implementation details. + (Exception: users can be provisioned using a user factory) + (2) Assert that fields are present in responses, but don't assert that the + entire response has some specific shape. That way, things like adding + new fields to an API response, which are backwards compatible, won't + break any tests, but backwards-incompatible API changes will. + """ + ENABLED_OPENEDX_EVENTS = [ + LIBRARY_CONTAINER_CREATED.event_type, + LIBRARY_CONTAINER_DELETED.event_type, + LIBRARY_CONTAINER_UPDATED.event_type, + ] + + def test_unit_crud(self): + """ + Test Create, Read, Update, and Delete of a Unit + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) + + create_receiver = mock.Mock() + LIBRARY_CONTAINER_CREATED.connect(create_receiver) + + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) + + delete_receiver = mock.Mock() + LIBRARY_CONTAINER_DELETED.connect(delete_receiver) + + # Create a unit: + create_date = datetime(2024, 9, 8, 7, 6, 5, tzinfo=timezone.utc) + with freeze_time(create_date): + container_data = self._create_container(lib["id"], "unit", slug="u1", display_name="Test Unit") + expected_data = { + "container_key": "lct:CL-TEST:containers:unit:u1", + "container_type": "unit", + "display_name": "Test Unit", + "last_published": None, + "published_by": "", + "last_draft_created": "2024-09-08T07:06:05Z", + "last_draft_created_by": 'Bob', + 'has_unpublished_changes': True, + 'created': '2024-09-08T07:06:05Z', + 'modified': '2024-09-08T07:06:05Z', + 'collections': [], + } + + self.assertDictContainsEntries(container_data, expected_data) + assert create_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_CREATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + create_receiver.call_args_list[0].kwargs, + ) + + # Fetch the unit: + unit_as_read = self._get_container(container_data["container_key"]) + # make sure it contains the same data when we read it back: + self.assertDictContainsEntries(unit_as_read, expected_data) + + # Update the unit: + modified_date = datetime(2024, 10, 9, 8, 7, 6, tzinfo=timezone.utc) + with freeze_time(modified_date): + container_data = self._update_container("lct:CL-TEST:containers:unit:u1", display_name="Unit ABC") + expected_data['last_draft_created'] = expected_data['modified'] = '2024-10-09T08:07:06Z' + expected_data['display_name'] = 'Unit ABC' + self.assertDictContainsEntries(container_data, expected_data) + + assert update_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + update_receiver.call_args_list[0].kwargs, + ) + + # Re-fetch the unit + unit_as_re_read = self._get_container(container_data["container_key"]) + # make sure it contains the same data when we read it back: + self.assertDictContainsEntries(unit_as_re_read, expected_data) + + # Delete the unit + self._delete_container(container_data["container_key"]) + self._get_container(container_data["container_key"], expect_response=404) + assert delete_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_DELETED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + delete_receiver.call_args_list[0].kwargs, + ) + + def test_unit_permissions(self): + """ + Test that a regular user with read-only permissions on the library cannot create, update, or delete units. + """ + lib = self._create_library(slug="containers2", title="Container Test Library 2", description="Unit permissions") + container_data = self._create_container(lib["id"], "unit", slug="u2", display_name="Test Unit") + + random_user = UserFactory.create(username="Random", email="random@example.com") + with self.as_user(random_user): + self._create_container(lib["id"], "unit", slug="u3", display_name="Test Unit", expect_response=403) + self._get_container(container_data["container_key"], expect_response=403) + self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403) + self._delete_container(container_data["container_key"], expect_response=403) + + # Granting read-only permissions on the library should only allow retrieval, nothing else. + self._add_user_by_email(lib["id"], random_user.email, access_level="read") + with self.as_user(random_user): + self._create_container(lib["id"], "unit", slug="u2", display_name="Test Unit", expect_response=403) + self._get_container(container_data["container_key"], expect_response=200) + self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403) + self._delete_container(container_data["container_key"], expect_response=403) + + def test_unit_gets_auto_slugs(self): + """ + Test that we can create units by specifying only a title, and they get + unique slugs assigned automatically. + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + + # Create two units, specifying their titles but not their slugs/keys: + container1_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + container2_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + # Notice the container IDs below are slugified from the title: "alpha-bravo-NNNNN" + assert container1_data["container_key"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") + assert container2_data["container_key"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") + assert container1_data["container_key"] != container2_data["container_key"] + + def test_unit_add_children(self): + """ + Test that we can add and get unit children components + """ + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) + + # Create container and add some components + container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) + html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], html_block["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 2 + assert data[0]['id'] == problem_block['id'] + assert not data[0]['can_stand_alone'] + assert data[1]['id'] == html_block['id'] + assert not data[1]['can_stand_alone'] + problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) + html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + # Add two more components + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block_2["id"], html_block_2["id"]] + ) + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key=container_data["container_key"], + ), + }, + update_receiver.call_args_list[0].kwargs, + ) + data = self._get_container_components(container_data["container_key"]) + # Verify total number of components to be 2 + 2 = 4 + assert len(data) == 4 + assert data[2]['id'] == problem_block_2['id'] + assert not data[2]['can_stand_alone'] + assert data[3]['id'] == html_block_2['id'] + assert data[3]['can_stand_alone'] + + def test_unit_remove_children(self): + """ + Test that we can remove unit children components + """ + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) + + # Create container and add some components + container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) + html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) + problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) + html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], html_block["id"], problem_block_2["id"], html_block_2["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 4 + # Remove both problem blocks. + self._remove_container_components( + container_data["container_key"], + children_ids=[problem_block_2["id"], problem_block["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 2 + assert data[0]['id'] == html_block['id'] + assert data[1]['id'] == html_block_2['id'] + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key=container_data["container_key"], + ), + }, + update_receiver.call_args_list[0].kwargs, + ) + + def test_unit_replace_children(self): + """ + Test that we can completely replace/reorder unit children components. + """ + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) + + # Create container and add some components + container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) + html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) + problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) + html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + self._add_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], html_block["id"], problem_block_2["id"], html_block_2["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 4 + assert data[0]['id'] == problem_block['id'] + assert data[1]['id'] == html_block['id'] + assert data[2]['id'] == problem_block_2['id'] + assert data[3]['id'] == html_block_2['id'] + + # Reorder the components + self._patch_container_components( + container_data["container_key"], + children_ids=[problem_block["id"], problem_block_2["id"], html_block["id"], html_block_2["id"]] + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 4 + assert data[0]['id'] == problem_block['id'] + assert data[1]['id'] == problem_block_2['id'] + assert data[2]['id'] == html_block['id'] + assert data[3]['id'] == html_block_2['id'] + + # Replace with new components + new_problem_block = self._add_block_to_library(lib["id"], "problem", "New_Problem", can_stand_alone=False) + new_html_block = self._add_block_to_library(lib["id"], "html", "New_Html", can_stand_alone=False) + self._patch_container_components( + container_data["container_key"], + children_ids=[new_problem_block["id"], new_html_block["id"]], + ) + data = self._get_container_components(container_data["container_key"]) + assert len(data) == 2 + assert data[0]['id'] == new_problem_block['id'] + assert data[1]['id'] == new_html_block['id'] + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key=container_data["container_key"], + ), + }, + update_receiver.call_args_list[0].kwargs, + ) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 7295e3bab0b5..bcb1573a27c0 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -150,10 +150,10 @@ def test_library_org_validation(self): self._create_library(slug="existing-org-1", title="Library in an existing org", org="CL-TEST") @patch( - "openedx.core.djangoapps.content_libraries.views.user_can_create_organizations", + "openedx.core.djangoapps.content_libraries.rest_api.libraries.user_can_create_organizations", ) @patch( - "openedx.core.djangoapps.content_libraries.views.get_allowed_organizations_for_libraries", + "openedx.core.djangoapps.content_libraries.rest_api.libraries.get_allowed_organizations_for_libraries", ) @override_settings(ORGANIZATIONS_AUTOCREATE=False) def test_library_org_no_autocreate(self, mock_get_allowed_organizations, mock_can_create_organizations): @@ -198,7 +198,10 @@ def test_library_org_no_autocreate(self, mock_get_allowed_organizations, mock_ca assert mock_get_allowed_organizations.call_count == 2 @skip("This endpoint shouldn't support num_blocks and has_unpublished_*.") - @patch("openedx.core.djangoapps.content_libraries.views.LibraryRootView.pagination_class.page_size", new=2) + @patch( + "openedx.core.djangoapps.content_libraries.rest_api.libraries.LibraryRootView.pagination_class.page_size", + new=2, + ) def test_list_library(self): """ Test the /libraries API and its pagination @@ -496,7 +499,10 @@ def test_library_blocks_studio_view(self): assert 'resources' in fragment assert 'Hello world!' in fragment['content'] - @patch("openedx.core.djangoapps.content_libraries.views.LibraryBlocksView.pagination_class.page_size", new=2) + @patch( + "openedx.core.djangoapps.content_libraries.rest_api.libraries.LibraryBlocksView.pagination_class.page_size", + new=2, + ) def test_list_library_blocks(self): """ Test the /libraries/{lib_key_str}/blocks API and its pagination diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 1272d79b3873..99e98a2cc424 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -2,26 +2,29 @@ URL configuration for Studio's Content Libraries REST API """ -from django.urls import include, path, re_path +from django.urls import include, path, re_path, register_converter from rest_framework import routers -from . import views -from . import views_collections +from .rest_api import blocks, collections, containers, libraries, url_converters # Django application name. app_name = 'openedx.core.djangoapps.content_libraries' +# URL converters + +register_converter(url_converters.LibraryContainerLocatorConverter, "lib_container_key") + # Router for importing blocks from courseware. import_blocks_router = routers.DefaultRouter() -import_blocks_router.register(r'tasks', views.LibraryImportTaskViewSet, basename='import-block-task') +import_blocks_router.register(r'tasks', libraries.LibraryImportTaskViewSet, basename='import-block-task') library_collections_router = routers.DefaultRouter() library_collections_router.register( - r'collections', views_collections.LibraryCollectionsView, basename="library-collections" + r'collections', collections.LibraryCollectionsView, basename="library-collections" ) # These URLs are only used in Studio. The LMS already provides all the @@ -31,61 +34,73 @@ urlpatterns = [ path('api/libraries/v2/', include([ # list of libraries / create a library: - path('', views.LibraryRootView.as_view()), + path('', libraries.LibraryRootView.as_view()), path('/', include([ # get data about a library, update a library, or delete a library: - path('', views.LibraryDetailsView.as_view()), + path('', libraries.LibraryDetailsView.as_view()), # Get the list of XBlock types that can be added to this library - path('block_types/', views.LibraryBlockTypesView.as_view()), + path('block_types/', libraries.LibraryBlockTypesView.as_view()), # Get the list of XBlocks in this library, or add a new one: - path('blocks/', views.LibraryBlocksView.as_view()), - # Commit (POST) or revert (DELETE) all pending changes to this library: - path('commit/', views.LibraryCommitView.as_view()), + path('blocks/', blocks.LibraryBlocksView.as_view()), + # Add a new container (unit etc.) to this library: + path('containers/', containers.LibraryContainersView.as_view()), + # Publish (POST) or revert (DELETE) all pending changes to this library: + path('commit/', libraries.LibraryCommitView.as_view()), # Get the list of users/groups who have permission to view/edit/administer this library: - path('team/', views.LibraryTeamView.as_view()), + path('team/', libraries.LibraryTeamView.as_view()), # Add/Edit (PUT) or remove (DELETE) a user's permission to use this library - path('team/user//', views.LibraryTeamUserView.as_view()), + path('team/user//', libraries.LibraryTeamUserView.as_view()), # Add/Edit (PUT) or remove (DELETE) a group's permission to use this library - path('team/group//', views.LibraryTeamGroupView.as_view()), + path('team/group//', libraries.LibraryTeamGroupView.as_view()), # Import blocks into this library. path('import_blocks/', include(import_blocks_router.urls)), # Paste contents of clipboard into library - path('paste_clipboard/', views.LibraryPasteClipboardView.as_view()), + path('paste_clipboard/', libraries.LibraryPasteClipboardView.as_view()), # Library Collections path('', include(library_collections_router.urls)), ])), path('blocks//', include([ # Get metadata about a specific XBlock in this library, or delete the block: - path('', views.LibraryBlockView.as_view()), - path('restore/', views.LibraryBlockRestore.as_view()), + path('', blocks.LibraryBlockView.as_view()), + # Restore a soft-deleted block + path('restore/', blocks.LibraryBlockRestore.as_view()), # Update collections for a given component - path('collections/', views.LibraryBlockCollectionsView.as_view(), name='update-collections'), + path('collections/', blocks.LibraryBlockCollectionsView.as_view(), name='update-collections'), # Get the LTI URL of a specific XBlock - path('lti/', views.LibraryBlockLtiUrlView.as_view(), name='lti-url'), + path('lti/', blocks.LibraryBlockLtiUrlView.as_view(), name='lti-url'), # Get the OLX source code of the specified block: - path('olx/', views.LibraryBlockOlxView.as_view()), + path('olx/', blocks.LibraryBlockOlxView.as_view()), # CRUD for static asset files associated with a block in the library: - path('assets/', views.LibraryBlockAssetListView.as_view()), - path('assets/', views.LibraryBlockAssetView.as_view()), - path('publish/', views.LibraryBlockPublishView.as_view()), + path('assets/', blocks.LibraryBlockAssetListView.as_view()), + path('assets/', blocks.LibraryBlockAssetView.as_view()), + path('publish/', blocks.LibraryBlockPublishView.as_view()), # Future: discard changes for just this one block - # Future: set a block's tags (tags are stored in a Tag bundle and linked in) + ])), + # Containers are Sections, Subsections, and Units + path('containers//', include([ + # Get metadata about a specific container in this library, update or delete the container: + path('', containers.LibraryContainerView.as_view()), + # update components under container + path('children/', containers.LibraryContainerChildrenView.as_view()), + # Update collections for a given container + # path('collections/', views.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'), + # path('publish/', views.LibraryContainerPublishView.as_view()), ])), re_path(r'^lti/1.3/', include([ - path('login/', views.LtiToolLoginView.as_view(), name='lti-login'), - path('launch/', views.LtiToolLaunchView.as_view(), name='lti-launch'), - path('pub/jwks/', views.LtiToolJwksView.as_view(), name='lti-pub-jwks'), + path('login/', libraries.LtiToolLoginView.as_view(), name='lti-login'), + path('launch/', libraries.LtiToolLaunchView.as_view(), name='lti-launch'), + path('pub/jwks/', libraries.LtiToolJwksView.as_view(), name='lti-pub-jwks'), ])), ])), path('library_assets/', include([ path( 'component_versions//', - views.LibraryComponentAssetView.as_view(), + blocks.LibraryComponentAssetView.as_view(), name='library-assets', ), path( 'blocks//', - views.LibraryComponentDraftAssetView.as_view(), + blocks.LibraryComponentDraftAssetView.as_view(), name='library-draft-assets', ), ]) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 96f6886b622b..9163ebd58aa0 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -12,7 +12,7 @@ import openedx_tagging.core.tagging.api as oel_tagging from django.db.models import Exists, OuterRef, Q, QuerySet from django.utils.timezone import now -from opaque_keys.edx.keys import CourseKey, LibraryCollectionKey +from opaque_keys.edx.keys import CourseKey, LibraryItemKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy from openedx_tagging.core.tagging.models.utils import TAGS_CSV_SEPARATOR @@ -230,7 +230,7 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]: """ content_key = get_content_key_from_string(object_id) - if isinstance(content_key, (UsageKey, LibraryCollectionKey)): + if isinstance(content_key, (UsageKey, LibraryItemKey)): raise ValueError("The object_id must be a CourseKey or a LibraryLocatorV2.") all_object_tags, taxonomies = get_all_object_tags(content_key) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index b693f7ee0f56..d85c87d62b17 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -5,8 +5,8 @@ import ddt from django.test.testcases import TestCase from fs.osfs import OSFS -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey -from opaque_keys.edx.locator import LibraryLocatorV2 +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryCollectionLocator from openedx_tagging.core.tagging.models import ObjectTag from organizations.models import Organization from .test_objecttag_export_helpers import TestGetAllObjectTagsMixin, TaggedCourseMixin @@ -381,7 +381,7 @@ def test_copy_cross_org_tags(self): self._test_copy_object_tags(src_key, dst_key, expected_tags) def test_tag_collection(self): - collection_key = LibraryCollectionKey.from_string("lib-collection:orgA:libX:1") + collection_key = LibraryCollectionLocator.from_string("lib-collection:orgA:libX:1") api.tag_object( object_id=str(collection_key), diff --git a/openedx/core/djangoapps/content_tagging/types.py b/openedx/core/djangoapps/content_tagging/types.py index 9ffb090d61e3..3684e5705376 100644 --- a/openedx/core/djangoapps/content_tagging/types.py +++ b/openedx/core/djangoapps/content_tagging/types.py @@ -5,11 +5,11 @@ from typing import Dict, List, Union -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey +from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryItemKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy -ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, LibraryCollectionKey] +ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, LibraryItemKey] ContextKey = Union[LibraryLocatorV2, CourseKey] TagValuesByTaxonomyIdDict = Dict[int, List[str]] diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index 39dd925c1acd..419b960927fd 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -5,7 +5,7 @@ from edx_django_utils.cache import RequestCache from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey +from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryItemKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy from organizations.models import Organization @@ -28,11 +28,11 @@ def get_content_key_from_string(key_str: str) -> ContentKey: return UsageKey.from_string(key_str) except InvalidKeyError: try: - return LibraryCollectionKey.from_string(key_str) + return LibraryItemKey.from_string(key_str) except InvalidKeyError as usage_key_error: raise ValueError( "object_id must be one of the following " - "keys: CourseKey, LibraryLocatorV2, UsageKey or LibCollectionKey" + "keys: CourseKey, LibraryLocatorV2, UsageKey or LibraryItemKey" ) from usage_key_error @@ -44,8 +44,8 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey: if isinstance(content_key, (CourseKey, LibraryLocatorV2)): return content_key - # If the content key is a LibraryCollectionKey, return the LibraryLocatorV2 - if isinstance(content_key, LibraryCollectionKey): + # If the content key is a LibraryItemKey, return the LibraryLocatorV2 + if isinstance(content_key, LibraryItemKey): return content_key.library_key # If the content key is a UsageKey, return the context key diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index 3e68ff2ce85d..952cda06d528 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -793,6 +793,7 @@ def setUp(self): Notification.objects.create(user=self.user, app_name='App Name 1', notification_type='Type B') Notification.objects.create(user=self.user, app_name='App Name 2', notification_type='Type A') Notification.objects.create(user=self.user, app_name='App Name 3', notification_type='Type C') + Notification.objects.create(user=self.user, app_name='App Name 4', notification_type='Type D', web=False) @override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) @ddt.unpack diff --git a/openedx/core/djangoapps/notifications/views.py b/openedx/core/djangoapps/notifications/views.py index b5a120e3059c..da969cc50764 100644 --- a/openedx/core/djangoapps/notifications/views.py +++ b/openedx/core/djangoapps/notifications/views.py @@ -332,7 +332,7 @@ def get(self, request): # Get the unseen notifications count for each app name. count_by_app_name = ( Notification.objects - .filter(user_id=request.user, last_seen__isnull=True) + .filter(user_id=request.user, last_seen__isnull=True, web=True) .values('app_name') .annotate(count=Count('*')) ) diff --git a/openedx/core/djangoapps/programs/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/programs/rest_api/v1/tests/test_views.py index e80d4c615aae..2864f41a9216 100644 --- a/openedx/core/djangoapps/programs/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/programs/rest_api/v1/tests/test_views.py @@ -1,11 +1,12 @@ """ -Unit tests for Learner Dashboard REST APIs and Views +Unit tests for Programs REST APIs and Views """ from unittest import mock from uuid import uuid4 from django.core.cache import cache +from django.test.utils import override_settings from django.urls import reverse_lazy from enterprise.models import EnterpriseCourseEnrollment @@ -31,6 +32,7 @@ with_site_configuration, ) from openedx.core.djangolib.testing.utils import skip_unless_lms +from openedx.features.enterprise_support.api import enterprise_is_enabled from openedx.features.enterprise_support.tests.factories import ( EnterpriseCourseEnrollmentFactory, EnterpriseCustomerFactory, @@ -192,6 +194,8 @@ def setUp(self): ) @with_site_configuration(configuration={"COURSE_CATALOG_API_URL": "foo"}) + @override_settings(FEATURES=dict(ENABLE_ENTERPRISE_INTEGRATION=True)) + @enterprise_is_enabled() def test_program_list(self): """ Verify API returns proper response. @@ -221,6 +225,8 @@ def test_program_list(self): } @with_site_configuration(configuration={"COURSE_CATALOG_API_URL": "foo"}) + @override_settings(FEATURES=dict(ENABLE_ENTERPRISE_INTEGRATION=True)) + @enterprise_is_enabled() def test_program_empty_list_if_no_enterprise_enrollments(self): """ Verify API returns empty response if no enterprise enrollments exists for a learner. diff --git a/openedx/core/djangoapps/programs/rest_api/v1/views.py b/openedx/core/djangoapps/programs/rest_api/v1/views.py index 69f47d268a5c..a5bf939e1ef6 100644 --- a/openedx/core/djangoapps/programs/rest_api/v1/views.py +++ b/openedx/core/djangoapps/programs/rest_api/v1/views.py @@ -3,12 +3,12 @@ from typing import Any, TYPE_CHECKING import logging -from enterprise.models import EnterpriseCourseEnrollment +from django.db.models.query import EmptyQuerySet from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView -from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.api import get_course_enrollments from openedx.core.djangoapps.programs.utils import ( ProgramProgressMeter, get_certificates, @@ -16,11 +16,14 @@ get_program_and_course_data, get_program_urls, ) +from openedx.features.enterprise_support.api import get_enterprise_course_enrollments, enterprise_is_enabled if TYPE_CHECKING: from django.http import HttpRequest, HttpResponse from django.contrib.auth.models import AnonymousUser, User # pylint: disable=imported-auth-user from django.contrib.sites.models import Site + from django.db.models.query import QuerySet + from common.djangoapps.student.models import CourseEnrollment logger = logging.getLogger(__name__) @@ -86,7 +89,7 @@ def get(self, request: "HttpRequest", enterprise_uuid: str) -> "HttpResponse": """ user: "AnonymousUser | User" = request.user - enrollments = self._get_enterprise_course_enrollments(enterprise_uuid, user) + enrollments = list(self._get_enterprise_course_enrollments(enterprise_uuid, user)) # return empty reponse if no enterprise enrollments exists for a user if not enrollments: return Response([]) @@ -170,26 +173,22 @@ def transform_authoring_organizations(authoring_organizations) -> list[dict[str, return programs + @enterprise_is_enabled(otherwise=EmptyQuerySet) def _get_enterprise_course_enrollments( self, enterprise_uuid: str, user: "AnonymousUser | User" - ) -> list[CourseEnrollment]: + ) -> "QuerySet[CourseEnrollment]": """ Return only enterprise enrollments for a user. """ - enterprise_enrollment_course_ids = list( - EnterpriseCourseEnrollment.objects.filter( - enterprise_customer_user__user_id=user.id, - enterprise_customer_user__enterprise_customer__uuid=enterprise_uuid, - ).values_list("course_id", flat=True) + enterprise_enrollment_course_ids = ( + get_enterprise_course_enrollments(user) + .filter(enterprise_customer_user__enterprise_customer__uuid=enterprise_uuid) + .values_list("course_id", flat=True) ) - course_enrollments = ( - CourseEnrollment.enrollments_for_user(user) - .filter(course_id__in=enterprise_enrollment_course_ids) - .select_related("course") - ) + course_enrollments = get_course_enrollments(user, True, list(enterprise_enrollment_course_ids)) - return list(course_enrollments) + return course_enrollments class ProgramProgressDetailView(APIView): diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index 31080ba46631..76263c4b405c 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -701,6 +701,7 @@ def _collect_one_click_purchase_eligibility_data(self): # lint-amnesty, pylint: is_learner_eligible_for_one_click_purchase = self.data["is_program_eligible_for_one_click_purchase"] bundle_uuid = self.data.get("uuid") skus = [] + course_keys = [] bundle_variant = "full" if is_learner_eligible_for_one_click_purchase: # lint-amnesty, pylint: disable=too-many-nested-blocks @@ -719,6 +720,7 @@ def _collect_one_click_purchase_eligibility_data(self): # lint-amnesty, pylint: # we are assuming that, for any given course, there is at most one paid entitlement available. if entitlement["mode"] in applicable_seat_types: skus.append(entitlement["sku"]) + course_keys.append(course.get("key")) entitlement_product = True break if not entitlement_product: @@ -728,6 +730,7 @@ def _collect_one_click_purchase_eligibility_data(self): # lint-amnesty, pylint: for seat in published_course_runs[0]["seats"]: if seat["type"] in applicable_seat_types and seat["sku"]: skus.append(seat["sku"]) + course_keys.append(course.get("key")) break else: # If a course in the program has more than 1 published course run @@ -744,14 +747,14 @@ def _collect_one_click_purchase_eligibility_data(self): # lint-amnesty, pylint: if is_anonymous or ALWAYS_CALCULATE_PROGRAM_PRICE_AS_ANONYMOUS_USER.is_enabled(): # The bundle uuid is necessary to see the program's discounted price if bundle_uuid: - params = dict(sku=skus, is_anonymous=True, bundle=bundle_uuid) + params = dict(sku=skus, is_anonymous=True, bundle=bundle_uuid, course_key=course_keys) else: - params = dict(sku=skus, is_anonymous=True) + params = dict(sku=skus, is_anonymous=True, course_key=course_keys) else: if bundle_uuid: - params = dict(sku=skus, username=self.user.username, bundle=bundle_uuid) + params = dict(sku=skus, username=self.user.username, bundle=bundle_uuid, course_key=course_keys) else: - params = dict(sku=skus, username=self.user.username) + params = dict(sku=skus, username=self.user.username, course_key=course_keys) response = get_program_price_info(self.user, params) response.raise_for_status() diff --git a/openedx/core/djangoapps/session_inactivity_timeout/middleware.py b/openedx/core/djangoapps/session_inactivity_timeout/middleware.py index 79609e4766e5..08e9d0e0e879 100644 --- a/openedx/core/djangoapps/session_inactivity_timeout/middleware.py +++ b/openedx/core/djangoapps/session_inactivity_timeout/middleware.py @@ -9,7 +9,7 @@ This was taken from StackOverflow (http://stackoverflow.com/questions/14830669/how-to-expire-django-session-in-5minutes) If left unset, session expiration will be handled by Django's SESSION_COOKIE_AGE, -which defauts to 1209600 (2 weeks, in seconds). +which defaults to 1209600 (2 weeks, in seconds). """ diff --git a/openedx/core/djangoapps/xblock/learning_context/learning_context.py b/openedx/core/djangoapps/xblock/learning_context/learning_context.py index b535e84ca7c1..dc7a21f1c397 100644 --- a/openedx/core/djangoapps/xblock/learning_context/learning_context.py +++ b/openedx/core/djangoapps/xblock/learning_context/learning_context.py @@ -76,3 +76,11 @@ def send_block_updated_event(self, usage_key): usage_key: the UsageKeyV2 subclass used for this learning context """ + + def send_container_updated_events(self, usage_key): + """ + Send "container updated" events for containers that contains the block with + the given usage_key in this context. + + usage_key: the UsageKeyV2 subclass used for this learning context + """ diff --git a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py index c3885fbf11cc..57582989f60d 100644 --- a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py @@ -317,6 +317,7 @@ def save_block(self, block): # Signal that we've modified this block learning_context = get_learning_context_impl(usage_key) learning_context.send_block_updated_event(usage_key) + learning_context.send_container_updated_events(usage_key) def _get_component_from_usage_key(self, usage_key): """ diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 2972b1876f57..2870901ee6be 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -112,7 +112,7 @@ numpy<2.0.0 # Date: 2023-09-18 # pinning this version to avoid updates while the library is being developed # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-learning==0.18.3 +openedx-learning==0.19.2 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3ae198a322fe..fb768f360ae0 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -477,7 +477,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/kernel.in edx-name-affirmation==3.0.1 # via -r requirements/edx/kernel.in -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/kernel.in # edx-bulk-grades @@ -820,7 +820,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/kernel.in -openedx-learning==0.18.3 +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 50bb602efab9..aaecf53c55c1 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -766,7 +766,7 @@ edx-name-affirmation==3.0.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1383,7 +1383,7 @@ openedx-forum==0.1.9 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-learning==0.18.3 +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 5b1a92fe3ca8..6a22620520c8 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -561,7 +561,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/base.txt edx-name-affirmation==3.0.1 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/base.txt # edx-bulk-grades @@ -992,7 +992,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning==0.18.3 +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index a17b9db4c868..55fa2f29082d 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -75,7 +75,7 @@ edx-event-bus-kafka>=5.6.0 # Kafka implementation of event bus edx-event-bus-redis edx-milestones edx-name-affirmation -edx-opaque-keys +edx-opaque-keys>=2.12.0 edx-organizations edx-proctoring>=2.0.1 # using hash to support django42 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 32034f3163e5..a677d1ab17f5 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -588,7 +588,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/base.txt edx-name-affirmation==3.0.1 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/base.txt # edx-bulk-grades @@ -1050,7 +1050,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning==0.18.3 +openedx-learning==0.19.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/xmodule/capa/safe_exec/safe_exec.py b/xmodule/capa/safe_exec/safe_exec.py index bfc98bb22109..882db11fd1ef 100644 --- a/xmodule/capa/safe_exec/safe_exec.py +++ b/xmodule/capa/safe_exec/safe_exec.py @@ -31,7 +31,13 @@ # the ./tmp directory that codejail creates in each sandbox, rather than trying # to use a global temp dir (which should be blocked by AppArmor anyhow). # This is needed for matplotlib among other things. +# +# matplotlib will complain on stderr about the non-standard temp dir if we +# don't explicitly tell it "no really, use this". This pollutes the output +# when codejail returns an error message (which includes stderr). So we also +# set MPLCONFIGDIR as a special case. os.environ["TMPDIR"] = os.getcwd() + "/tmp" +os.environ["MPLCONFIGDIR"] = os.environ["TMPDIR"] import random2 as random_module import sys