diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 553b7c031566..bfd2d0090bd6 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -13,6 +13,7 @@ from django.http import Http404, HttpResponse, HttpResponseBadRequest from django.utils.translation import ugettext as _ from django.views.decorators.http import require_http_methods +from edx_django_utils.plugins import pluggable_override from edx_proctoring.api import ( does_backend_support_onboarding, get_exam_by_content_id, @@ -1117,6 +1118,7 @@ def _get_gating_info(course, xblock): return info +@pluggable_override('OVERRIDE_CREATE_XBLOCK_INFO') def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False, # lint-amnesty, pylint: disable=too-many-statements course_outline=False, include_children_predicate=NEVER, parent_xblock=None, graders=None, user=None, course=None, is_concise=False): @@ -1135,6 +1137,13 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F In addition, an optional include_children_predicate argument can be provided to define whether or not a particular xblock should have its children included. + + You can customize the behavior of this function using the `OVERRIDE_CREATE_XBLOCK_INFO` pluggable override point. + For example: + >>> def create_xblock_info(default_fn, xblock, *args, **kwargs): + ... xblock_info = default_fn(xblock, *args, **kwargs) + ... xblock_info['icon'] = xblock.icon_override + ... return xblock_info """ is_library_block = isinstance(xblock.location, LibraryUsageLocator) is_xblock_unit = is_unit(xblock, parent_xblock) diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 2f237986c9c0..076938532463 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -3,6 +3,7 @@ """ import copy +import itertools import json import logging import os @@ -16,6 +17,7 @@ import ddt import lxml +from bson import ObjectId from django.conf import settings from django.core.files.storage import FileSystemStorage from django.test.utils import override_settings @@ -928,7 +930,7 @@ def setUp(self): self.source_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) self.addCleanup(shutil.rmtree, self.export_dir, ignore_errors=True) - def _setup_source_course_with_library_content(self, publish=False): + def _setup_source_course_with_library_content(self, publish=False, version=None): """ Sets up course with library content. """ @@ -947,7 +949,9 @@ def _setup_source_course_with_library_content(self, publish=False): parent_location=sequential.location, display_name='Test Unit' ) - lc_block = self._add_library_content_block(vertical, self.lib_key, publish_item=publish) + lc_block = self._add_library_content_block( + vertical, self.lib_key, publish_item=publish, other_settings=dict(source_library_version=version) + ) self._refresh_children(lc_block) def get_lib_content_block_children(self, block_location): @@ -986,13 +990,18 @@ def assert_names(self, source_child_location, dest_child_location): dest_child = self.store.get_item(dest_child_location) self.assertEqual(source_child.display_name, dest_child.display_name) - @ddt.data(True, False) - def test_library_content_on_course_export_import(self, publish_item): + @ddt.data(*itertools.product([False, True], repeat=2)) + @ddt.unpack + def test_library_content_on_course_export_import(self, publish_item, generate_version): """ Verify that library contents in destination and source courses are same after importing the source course into destination course. + + If a library with the specified version does not exist in the modulestore, the import should not fail. """ - self._setup_source_course_with_library_content(publish=publish_item) + self._setup_source_course_with_library_content( + publish=publish_item, version=str(ObjectId()) if generate_version else None + ) # Create a course to import source course. dest_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) @@ -1099,7 +1108,25 @@ def assert_problem_definition(self, course_location, expected_problem_content): '\n 
\n    x=10 print("hello \n")\n
\n ' '\n\n' - ] + ], + [ + '' + '' + '' + '

' + '""' + 'Text' + '

' + '', + + '\n ' + '\n ' + '

' + '""' + 'Text' + '

\n' + '\n' + ], ) @ddt.unpack def test_problem_content_on_course_export_import(self, problem_data, expected_problem_content): diff --git a/cms/envs/common.py b/cms/envs/common.py index ce4e36a6f4cf..6c1f0355a20d 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -464,6 +464,19 @@ # in the LMS and CMS. # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/429' 'DISABLE_UNENROLLMENT': False, + +# .. toggle_name: MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: If enabled, the Library Content Block is marked as complete when users view it. + # Otherwise (by default), all children of this block must be completed. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2021-07-25 + # .. toggle_target_removal_date: None + # .. toggle_tickets: https://github.com/edx/edx-platform/pull/28268 + # .. toggle_warnings: For consistency in user-experience, keep the value in sync with the setting of the same name + # in the LMS and CMS. + 'MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW': False, } ENABLE_JASMINE = False @@ -818,6 +831,7 @@ EditInfoMixin, AuthoringMixin, ) +XBLOCK_EXTRA_MIXINS = () XBLOCK_SELECT_FUNCTION = prefer_xmodules diff --git a/cms/envs/production.py b/cms/envs/production.py index 6bc60b1d4ea5..3c6de5a5df72 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -583,3 +583,6 @@ def get_env_setting(setting): } LOGO_IMAGE_EXTRA_TEXT = ENV_TOKENS.get('LOGO_IMAGE_EXTRA_TEXT', '') + +############## XBlock extra mixins ############################ +XBLOCK_MIXINS += tuple(XBLOCK_EXTRA_MIXINS) diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 7419a0ae1077..70ebcb6153cc 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -1108,6 +1108,19 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', }, options)); }, + /** + * This function allows comprehensive themes to create custom editors without adding boilerplate code. + * + * A simple example theme for this can be found at https://github.com/open-craft/custom-unit-icons-theme + **/ + getCustomEditModal: function(tabs, editors, xblockInfo, options) { + return new SettingsXBlockModal($.extend({ + tabs: tabs, + editors: editors, + model: xblockInfo + }, options)); + }, + getPublishModal: function(xblockInfo, options) { return new PublishXBlockModal($.extend({ editors: [PublishEditor], diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index cd86d6399795..08f25b3eed19 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -34,6 +34,9 @@ define([ collapsedClass: 'is-collapsed' }, + // Extracting this to a variable allows comprehensive themes to replace or extend `CourseOutlineView`. + outlineViewClass: CourseOutlineView, + initialize: function() { var self = this; this.initialState = this.options.initialState; @@ -90,7 +93,7 @@ define([ this.highlightsEnableView.render(); } - this.outlineView = new CourseOutlineView({ + this.outlineView = new this.outlineViewClass({ el: this.$('.outline'), model: this.model, isRoot: true, diff --git a/common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js b/common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js new file mode 100644 index 000000000000..81eb7d2105e7 --- /dev/null +++ b/common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js @@ -0,0 +1,12 @@ +/* JavaScript for reset option that can be done on a randomized LibraryContentBlock */ +function LibraryContentReset(runtime, element) { + $('.problem-reset-btn', element).click((e) => { + e.preventDefault(); + $.post({ + url: runtime.handlerUrl(element, 'reset_selected_children'), + success() { + location.reload(); + }, + }); + }); +} diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 172128d907db..8414737a271b 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -305,14 +305,22 @@ def index_dictionary(self): # Make optioninput's options index friendly by replacing the actual tag with the values capa_content = re.sub(r'\s*|\S*<\/optioninput>', r'\1', self.data) - # Removing solutions and hints, as well as script and style + # Remove the following tags with content that can leak hints or solutions: + # - `solution` (with optional attributes) and `solutionset`. + # - `targetedfeedback` (with optional attributes) and `targetedfeedbackset`. + # - `answer` (with optional attributes). + # - `script` (with optional attributes). + # - `style` (with optional attributes). + # - various types of hints (with optional attributes) and `hintpart`. capa_content = re.sub( re.compile( r""" - .*? | - | - | - <[a-z]*hint.*?>.*? + .*? | + .*? | + .*? | + .*? | + .*? | + <[a-z]*hint.*?>.*? """, re.DOTALL | re.VERBOSE), diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index c303f022c73a..29e772f8bee1 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -8,17 +8,21 @@ import random from copy import copy from gettext import ngettext +from rest_framework import status import bleach +from django.conf import settings +from django.utils.decorators import classproperty from lazy import lazy from lxml import etree +from lxml.etree import XMLSyntaxError from opaque_keys.edx.locator import LibraryLocator from pkg_resources import resource_string from web_fragments.fragment import Fragment from webob import Response from xblock.completable import XBlockCompletionMode from xblock.core import XBlock -from xblock.fields import Integer, List, Scope, String +from xblock.fields import Integer, List, Scope, String, Boolean from capa.responsetypes import registry from xmodule.mako_module import MakoTemplateBlockBase @@ -114,7 +118,18 @@ class LibraryContentBlock( show_in_read_only_mode = True - completion_mode = XBlockCompletionMode.AGGREGATOR + # noinspection PyMethodParameters + @classproperty + def completion_mode(cls): # pylint: disable=no-self-argument + """ + Allow overriding the completion mode with a feature flag. + + This is a property, so it can be dynamically overridden in tests, as it is not evaluated at runtime. + """ + if settings.FEATURES.get('MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW', False): + return XBlockCompletionMode.COMPLETABLE + + return XBlockCompletionMode.AGGREGATOR display_name = String( display_name=_("Display Name"), @@ -163,6 +178,14 @@ class LibraryContentBlock( default=[], scope=Scope.user_state, ) + # This cannot be called `show_reset_button`, because children blocks inherit this as a default value. + allow_resetting_children = Boolean( + display_name=_("Show Reset Button"), + help=_("Determines whether a 'Reset Problems' button is shown, so users may reset their answers and reshuffle " + "selected items."), + scope=Scope.settings, + default=False + ) @property def source_library_key(self): @@ -333,6 +356,27 @@ def selected_children(self): return self.selected + @XBlock.handler + def reset_selected_children(self, _, __): + """ + Resets the XBlock's state for a user. + + This resets the state of all `selected` children and then clears the `selected` field + so that the new blocks are randomly chosen for this user. + """ + if not self.allow_resetting_children: + return Response('"Resetting selected children" is not allowed for this XBlock', + status=status.HTTP_400_BAD_REQUEST) + + for block_type, block_id in self.selected_children(): + block = self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) + if hasattr(block, 'reset_problem'): + block.reset_problem(None) + block.save() + + self.selected = [] + return Response() + def _get_selected_child_blocks(self): """ Generator returning XBlock instances of the children selected for the @@ -370,7 +414,11 @@ def student_view(self, context): # lint-amnesty, pylint: disable=missing-functi 'show_bookmark_button': False, 'watched_completable_blocks': set(), 'completion_delay_ms': None, + 'reset_button': self.allow_resetting_children, })) + + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_reset.js')) + fragment.initialize_js('LibraryContentReset') return fragment def author_view(self, context): @@ -664,10 +712,20 @@ def get_content_titles(self): @classmethod def definition_from_xml(cls, xml_object, system): - children = [ - system.process_xml(etree.tostring(child)).scope_ids.usage_id - for child in xml_object.getchildren() - ] + children = [] + + for child in xml_object.getchildren(): + try: + children.append(system.process_xml(etree.tostring(child)).scope_ids.usage_id) + except (XMLSyntaxError, AttributeError): + msg = ( + "Unable to load child when parsing Library Content Block. " + "This can happen when a comment is manually added to the course export." + ) + logger.error(msg) + if system.error_tracker is not None: + system.error_tracker(msg) + definition = { attr_name: json.loads(attr_value) for attr_name, attr_value in xml_object.attrib.items() diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index cb40f58aa416..7ec7fdf2d587 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -40,8 +40,7 @@ from .exceptions import ItemNotFoundError from .inheritance import InheritanceKeyValueStore, compute_inherited_metadata, inheriting_field_data -edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, - remove_comments=True, remove_blank_text=True) +edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, remove_blank_text=True) etree.set_default_parser(edx_xml_parser) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 0e967acf057b..1279531d0928 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -883,16 +883,20 @@ def _convert_ref_fields_to_new_namespace(reference): if lib_content_block_already_published: return block - # Update library content block's children on draft branch - with store.branch_setting(branch_setting=ModuleStoreEnum.Branch.draft_preferred): - LibraryToolsService(store, user_id).update_children( - block, - version=block.source_library_version, - ) - - # Publish it if importing the course for branch setting published_only. - if store.get_branch_setting() == ModuleStoreEnum.Branch.published_only: - store.publish(block.location, user_id) + try: + # Update library content block's children on draft branch + with store.branch_setting(branch_setting=ModuleStoreEnum.Branch.draft_preferred): + LibraryToolsService(store, user_id).update_children( + block, + version=block.source_library_version, + ) + except ValueError as err: + # The specified library version does not exist. + log.error(err) + else: + # Publish it if importing the course for branch setting published_only. + if store.get_branch_setting() == ModuleStoreEnum.Branch.published_only: + store.publish(block.location, user_id) return block diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index a3351277b3a6..6abd073ed869 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -2535,25 +2535,32 @@ def test_response_types_multiple_tags(self): def test_solutions_not_indexed(self): xml = textwrap.dedent(""" - -
-

Explanation

- -

This is what the 1st solution.

- -
- - - -
-

Explanation

- -

This is the 2nd solution.

- -
- - - + Test solution. + Test solution with attribute. + + Test solutionset. + Test solution within solutionset. + + + Test feedback. + Test feedback with attribute. + + Test FeedbackSet. + Test feedback within feedbackset. + + + Test answer. + Test answer with attribute. + + + + + + + + Test choicehint. + Test hint. + Test hintpart. """) name = "Blank Common Capa Problem" @@ -2668,7 +2675,7 @@ def test_indexing_non_latin_problem(self): """) name = "Non latin Input" descriptor = self._create_descriptor(sample_text_input_problem_xml, name=name) - capa_content = " FX1_VAL='Καλημέρα' Δοκιμή με μεταβλητές με Ελληνικούς χαρακτήρες μέσα σε python: $FX1_VAL " + capa_content = " Δοκιμή με μεταβλητές με Ελληνικούς χαρακτήρες μέσα σε python: $FX1_VAL " descriptor_dict = descriptor.index_dictionary() assert descriptor_dict['content']['capa_content'] == smart_text(capa_content) diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 8a287acf88f2..a2d057b984c3 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -3,6 +3,7 @@ Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. """ +import ddt from unittest.mock import Mock, patch from bson.objectid import ObjectId @@ -11,6 +12,7 @@ from search.search_engine_base import SearchEngine from web_fragments.fragment import Fragment from xblock.runtime import Runtime as VanillaRuntime +from rest_framework import status from xmodule.library_content_module import ANY_CAPA_TYPE_VALUE, LibraryContentBlock from xmodule.library_tools import LibraryToolsService @@ -20,6 +22,7 @@ from xmodule.tests import get_test_system from xmodule.validation import StudioValidationMessage from xmodule.x_module import AUTHOR_VIEW +from xmodule.capa_module import ProblemBlock from .test_course_module import DummySystem as TestImportSystem @@ -30,6 +33,7 @@ class LibraryContentTest(MixedSplitTestCase): """ Base class for tests of LibraryContentBlock (library_content_block.py) """ + def setUp(self): super().setUp() @@ -74,18 +78,14 @@ class TestLibraryContentExportImport(LibraryContentTest): """ Export and import tests for LibraryContentBlock """ + def setUp(self): + super().setUp() - maxDiff = None - - def test_xml_export_import_cycle(self): - """ - Test the export-import cycle. - """ # Children will only set after calling this. self.lc_block.refresh_children() - lc_block = self.store.get_item(self.lc_block.location) + self.lc_block = self.store.get_item(self.lc_block.location) - expected_olx = ( + self.expected_olx = ( '\n' ' \n' @@ -94,46 +94,81 @@ def test_xml_export_import_cycle(self): ' \n' '\n' ).format( - block=lc_block, + block=self.lc_block, ) - export_fs = MemoryFS() # Set the virtual FS to export the olx to. - lc_block.runtime._descriptor_system.export_fs = export_fs # pylint: disable=protected-access + self.export_fs = MemoryFS() + self.lc_block.runtime._descriptor_system.export_fs = self.export_fs # pylint: disable=protected-access + + # Prepare runtime for the import. + self.runtime = TestImportSystem(load_error_modules=True, course_id=self.lc_block.location.course_key) + self.runtime.resources_fs = self.export_fs + self.id_generator = Mock() # Export the olx. node = etree.Element("unknown_root") - lc_block.add_xml_to_node(node) + self.lc_block.add_xml_to_node(node) - # Read it back - with export_fs.open('{dir}/{file_name}.xml'.format( - dir=lc_block.scope_ids.usage_id.block_type, - file_name=lc_block.scope_ids.usage_id.block_id + def _verify_xblock_properties(self, imported_lc_block): + """ + Check the new XBlock has the same properties as the old one. + """ + assert imported_lc_block.display_name == self.lc_block.display_name + assert imported_lc_block.source_library_id == self.lc_block.source_library_id + assert imported_lc_block.source_library_version == self.lc_block.source_library_version + assert imported_lc_block.mode == self.lc_block.mode + assert imported_lc_block.max_count == self.lc_block.max_count + assert imported_lc_block.capa_type == self.lc_block.capa_type + assert len(imported_lc_block.children) == len(self.lc_block.children) + assert imported_lc_block.children == self.lc_block.children + + def test_xml_export_import_cycle(self): + """ + Test the export-import cycle. + """ + # Read back the olx. + with self.export_fs.open('{dir}/{file_name}.xml'.format( + dir=self.lc_block.scope_ids.usage_id.block_type, + file_name=self.lc_block.scope_ids.usage_id.block_id )) as f: exported_olx = f.read() # And compare. - assert exported_olx == expected_olx - - runtime = TestImportSystem(load_error_modules=True, course_id=lc_block.location.course_key) - runtime.resources_fs = export_fs + assert exported_olx == self.expected_olx # Now import it. olx_element = etree.fromstring(exported_olx) - id_generator = Mock() - imported_lc_block = LibraryContentBlock.parse_xml(olx_element, runtime, None, id_generator) + imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None, self.id_generator) - # Check the new XBlock has the same properties as the old one. - assert imported_lc_block.display_name == lc_block.display_name - assert imported_lc_block.source_library_id == lc_block.source_library_id - assert imported_lc_block.source_library_version == lc_block.source_library_version - assert imported_lc_block.mode == lc_block.mode - assert imported_lc_block.max_count == lc_block.max_count - assert imported_lc_block.capa_type == lc_block.capa_type - assert len(imported_lc_block.children) == 4 - assert imported_lc_block.children == lc_block.children + self._verify_xblock_properties(imported_lc_block) + def test_xml_import_with_comments(self): + """ + Test that XML comments within LibraryContentBlock are ignored during the import. + """ + olx_with_comments = ( + '\n' + '\n' + '\n' + ' \n' + ' \n' + ' \n' + ' \n' + '\n' + ).format( + block=self.lc_block, + ) + + # Import the olx. + olx_element = etree.fromstring(olx_with_comments) + imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None, self.id_generator) + self._verify_xblock_properties(imported_lc_block) + + +@ddt.ddt class LibraryContentBlockTestMixin: """ Basic unit tests for LibraryContentBlock @@ -348,6 +383,39 @@ def _change_count_and_refresh_children(self, count): assert len(selected) == count return selected + @ddt.data( + # User resets selected children with reset button on content block + (True, 8), + # User resets selected children without reset button on content block + (False, 8), + ) + @ddt.unpack + def test_reset_selected_children_capa_blocks(self, allow_resetting_children, max_count): + """ + Tests that the `reset_selected_children` method of a content block resets only + XBlocks that have a `reset_problem` attribute when `allow_resetting_children` is True + + This test block has 4 HTML XBlocks and 4 Problem XBlocks. Therefore, if we ensure + that the `reset_problem` has been called len(self.problem_types) times, then + it means that this is working correctly + """ + self.lc_block.allow_resetting_children = allow_resetting_children + self.lc_block.max_count = max_count + # Add some capa blocks + self._create_capa_problems() + self.lc_block.refresh_children() + self.lc_block = self.store.get_item(self.lc_block.location) + + with patch.object(ProblemBlock, 'reset_problem', return_value={'success': True}) as reset_problem: + response = self.lc_block.reset_selected_children(None, None) + + if allow_resetting_children: + assert reset_problem.call_count == len(self.problem_types) + assert response.status_code == status.HTTP_200_OK + else: + reset_problem.assert_not_called() + assert response.status_code == status.HTTP_400_BAD_REQUEST + @patch('xmodule.library_tools.SearchEngine.get_search_engine', Mock(return_value=None, autospec=True)) class TestLibraryContentBlockNoSearchIndex(LibraryContentBlockTestMixin, LibraryContentTest): @@ -366,6 +434,7 @@ class TestLibraryContentBlockWithSearchIndex(LibraryContentBlockTestMixin, Libra """ Tests for library container with mocked search engine response. """ + def _get_search_response(self, field_dictionary=None): """ Mocks search response as returned by search engine """ target_type = field_dictionary.get('problem_types') diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 84e6c6064155..f7329c188152 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -20,9 +20,7 @@ log = logging.getLogger(__name__) # assume all XML files are persisted as utf-8. -EDX_XML_PARSER = XMLParser(dtd_validation=False, load_dtd=False, - remove_comments=True, remove_blank_text=True, - encoding='utf-8') +EDX_XML_PARSER = XMLParser(dtd_validation=False, load_dtd=False, remove_blank_text=True, encoding='utf-8') def name_to_pathname(name): diff --git a/common/static/common/js/discussion/views/discussion_inline_view.js b/common/static/common/js/discussion/views/discussion_inline_view.js index 8da4d74a8fba..7d87a69bcad8 100644 --- a/common/static/common/js/discussion/views/discussion_inline_view.js +++ b/common/static/common/js/discussion/views/discussion_inline_view.js @@ -154,6 +154,7 @@ }); this.threadView.render(); this.listenTo(this.threadView.showView, 'thread:_delete', this.navigateToAllPosts); + this.$(".forum-nav-thread[data-id='" + threadId + "']").removeClass('never-read'); this.threadListView.$el.addClass('is-hidden'); this.$('.inline-thread').removeClass('is-hidden'); }, diff --git a/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js index 4e6b0ee5a7e3..957ecf29719e 100644 --- a/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_inline_view_spec.js @@ -35,6 +35,7 @@ }); createTestView = function(test) { + var testView; var courseSettings = DiscussionSpecHelper.createTestCourseSettings({ groups: [ { @@ -62,7 +63,7 @@ children: [] } }); - var testView = new DiscussionInlineView({ + testView = new DiscussionInlineView({ el: $('.discussion-module') }); testView.render(); @@ -234,6 +235,22 @@ // Verify that the individual thread is no longer shown expect(testView.$('.group-visibility-label').length).toBe(0); }); + + it('marks a thread as read once it is opened', function() { + var testView = createTestView(this); + var thread; + showDiscussion(this, testView); + thread = testView.$('.forum-nav-thread'); + + // The thread is marked as unread. + expect(thread).toHaveClass('never-read'); + + // Navigate to the thread. + thread.find('.forum-nav-thread-link').click(); + + // The thread is no longer marked as unread. + expect(thread).not.toHaveClass('never-read'); + }); }); }); }()); diff --git a/common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss b/common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss index 79e0a7a08f5a..8bd18361d183 100644 --- a/common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss +++ b/common/static/sass/edx-pattern-library-shims/_breadcrumbs.scss @@ -57,5 +57,10 @@ @include rtl { @include transform(rotateY(180deg)); } + + // Hide a trailing separator. + &:last-child { + display: none; + } } } diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index 26cd63542a44..3b8c127b37ba 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -96,6 +96,7 @@ def enrolled_students_features(course_key, features): """ include_cohort_column = 'cohort' in features include_team_column = 'team' in features + include_city_column = 'city' in features include_enrollment_mode = 'enrollment_mode' in features include_verification_status = 'verification_status' in features include_program_enrollments = 'external_user_key' in features @@ -151,6 +152,13 @@ def extract_student(student, features): for meta_feature, meta_key in meta_features: student_dict[meta_feature] = meta_dict.get(meta_key) + # There are two separate places where the city value can be stored, + # one used by account settings and the other used by the registration form. + # If the account settings value (meta.city) is set, it takes precedence. + meta_city = meta_dict.get('city') + if include_city_column and meta_city: + student_dict['city'] = meta_city + if include_cohort_column: # Note that we use student.course_groups.all() here instead of # student.course_groups.filter(). The latter creates a fresh query, diff --git a/lms/envs/common.py b/lms/envs/common.py index 3ccb04ab601c..dfd5469fad12 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -932,7 +932,7 @@ # .. toggle_name: FEATURES['DISABLE_UNENROLLMENT'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False - # .. toggle_description: Set to True to disable self-unenrollments via REST API. + # .. toggle_description: Set to True to disable self-unenrollments via REST API. # This also hides the "Unenroll" button on the Learner Dashboard. # .. toggle_use_cases: open_edx # .. toggle_creation_date: 2021-10-11 @@ -953,6 +953,19 @@ # .. toggle_warnings: None # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/439' 'ENABLE_REDIRECT_UNAUTHENTICATED_USERS_TO_LOGIN': False, + + # .. toggle_name: MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: If enabled, the Library Content Block is marked as complete when users view it. + # Otherwise (by default), all children of this block must be completed. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2021-07-25 + # .. toggle_target_removal_date: None + # .. toggle_tickets: https://github.com/edx/edx-platform/pull/28268 + # .. toggle_warnings: For consistency in user-experience, keep the value in sync with the setting of the same name + # in the LMS and CMS. + 'MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW': False, } # Specifies extra XBlock fields that should available when requested via the Course Blocks API diff --git a/lms/static/js/views/fields.js b/lms/static/js/views/fields.js index db7befc82866..4a1f27f6db23 100644 --- a/lms/static/js/views/fields.js +++ b/lms/static/js/views/fields.js @@ -1,4 +1,5 @@ -(function(define, undefined) { +/* eslint no-underscore-dangle: ["error", { "allow": ["_super"] }] */ +(function(define, undef) { 'use strict'; define([ 'gettext', 'jquery', 'underscore', 'backbone', @@ -25,7 +26,7 @@ fieldType: 'generic', className: function() { - return 'u-field' + ' u-field-' + this.fieldType + ' u-field-' + this.options.valueAttribute; + return 'u-field u-field-' + this.fieldType + ' u-field-' + this.options.valueAttribute; }, tagName: 'div', @@ -100,21 +101,22 @@ return HtmlUtils.setHtml(this.$('.u-field-title'), title); }, - getMessage: function(message_status) { - if ((message_status + 'Message') in this) { - return this[message_status + 'Message'].call(this); + getMessage: function(messageStatus) { + if ((messageStatus + 'Message') in this) { + return this[messageStatus + 'Message'].call(this); } else if (this.showMessages) { - return HtmlUtils.joinHtml(this.indicators[message_status], this.messages[message_status]); + return HtmlUtils.joinHtml(this.indicators[messageStatus], this.messages[messageStatus]); } - return this.indicators[message_status]; + return this.indicators[messageStatus]; }, showHelpMessage: function(message) { + var msg = message; if (_.isUndefined(message) || _.isNull(message)) { - message = this.helpMessage; + msg = this.helpMessage; } this.$('.u-field-message-notification').html(''); - HtmlUtils.setHtml(this.$('.u-field-message-help'), message); + HtmlUtils.setHtml(this.$('.u-field-message-help'), msg); }, getNotificationMessage: function() { @@ -139,19 +141,19 @@ }, showSuccessMessage: function() { - var successMessage = this.getMessage('success'); + var context = Date.now(), + successMessage = this.getMessage('success'), + view = this; + this.showNotificationMessage(successMessage); if (this.options.refreshPageOnSave) { - if ("focusNextID" in this.options) { - $.cookie('focus_id', this.options.focusNextID ); + if ('focusNextID' in this.options) { + $.cookie('focus_id', this.options.focusNextID); } location.reload(true); } - var view = this; - - var context = Date.now(); this.lastSuccessMessageContext = context; setTimeout(function() { @@ -167,11 +169,14 @@ }, showErrorMessage: function(xhr) { + var errors, + validationErrorMessage, + message; if (xhr.status === 400) { try { - var errors = JSON.parse(xhr.responseText), - validationErrorMessage = errors.field_errors[this.options.valueAttribute].user_message, - message = HtmlUtils.joinHtml(this.indicators.validationError, validationErrorMessage); + errors = JSON.parse(xhr.responseText); + validationErrorMessage = errors.field_errors[this.options.valueAttribute].user_message; + message = HtmlUtils.joinHtml(this.indicators.validationError, validationErrorMessage); this.showNotificationMessage(message); } catch (error) { this.showNotificationMessage(this.getMessage('error')); @@ -202,19 +207,19 @@ }, saveAttributes: function(attributes, options) { + var view = this; + var defaultOptions = { + contentType: 'application/merge-patch+json', + patch: true, + wait: true, + success: function() { + view.saveSucceeded(); + }, + error: function(model, xhr) { + view.showErrorMessage(xhr); + } + }; if (this.persistChanges === true) { - var view = this; - var defaultOptions = { - contentType: 'application/merge-patch+json', - patch: true, - wait: true, - success: function() { - view.saveSucceeded(); - }, - error: function(model, xhr) { - view.showErrorMessage(xhr); - } - }; this.showInProgressMessage(); this.model.save(attributes, _.extend(defaultOptions, options)); } @@ -383,7 +388,12 @@ updateValueInField: function() { var value = (_.isUndefined(this.modelValue()) || _.isNull(this.modelValue())) ? '' : this.modelValue(); - this.$('.u-field-value input').val(value); + + var fieldHasFocus = (document.activeElement === this.$('.u-field-value input')[0]); + var fieldChanged = this.fieldValue() !== value; + if (!fieldHasFocus || !fieldChanged) { + this.$('.u-field-value input').val(value); + } }, saveValue: function() { @@ -419,7 +429,7 @@ editable: this.editable, title: this.options.title, screenReaderTitle: this.options.screenReaderTitle || this.options.title, - titleVisible: this.options.titleVisible !== undefined ? this.options.titleVisible : true, + titleVisible: this.options.titleVisible !== undef ? this.options.titleVisible : true, iconName: this.options.iconName, showBlankOption: (!this.options.required || !this.modelValueIsSet()), groupOptions: this.createGroupOptions(), @@ -468,8 +478,9 @@ }, displayValue: function(value) { + var option; if (value) { - var option = this.optionForValue(value); + option = this.optionForValue(value); return (option ? option[1] : ''); } else { return ''; @@ -477,11 +488,19 @@ }, updateValueInField: function() { + var value; // str + var fieldHasFocus; // bool + var fieldChanged; // bool if (this.editable !== 'never') { - this.$('.u-field-value select').val(this.modelValue() || ''); + value = this.modelValue() || ''; + fieldHasFocus = (document.activeElement === this.$('.u-field-value select')[0]); + fieldChanged = this.fieldValue() !== value; + if (!fieldHasFocus || !fieldChanged) { + this.$('.u-field-value select').val(value); + } } - var value = this.displayValue(this.modelValue() || ''); + value = this.displayValue(this.modelValue() || ''); if (this.modelValueIsSet() === false) { value = this.options.placeholderValue || ''; } @@ -613,24 +632,21 @@ 'aria-live': 'assertive', 'aria-atomic': true }); - } - else if (remainingCharCount < 60) { + } else if (remainingCharCount < 60) { $charCount.attr('aria-atomic', 'false'); - } - else if (remainingCharCount < 70) { + } else if (remainingCharCount < 70) { $charCount.attr({ 'aria-live': 'polite', 'aria-atomic': true }); } $charCount.text(curCharCount); - } }, adjustTextareaHeight: function() { - if (this.persistChanges === false) { return; } var textarea = this.$('textarea'); + if (this.persistChanges === false) { return; } textarea.css('height', 'auto').css('height', textarea.prop('scrollHeight') + 10); }, diff --git a/lms/static/sass/course/courseware/_courseware.scss b/lms/static/sass/course/courseware/_courseware.scss index 67a5a704050b..33f8dae798ad 100644 --- a/lms/static/sass/course/courseware/_courseware.scss +++ b/lms/static/sass/course/courseware/_courseware.scss @@ -635,6 +635,17 @@ html.video-fullscreen { border-bottom: 1px solid #ddd; margin-bottom: ($baseline*0.75); padding: 0 0 15px; + + .problem-reset-btn-wrapper { + position: relative; + .problem-reset-btn { + &:hover, + &:focus, + &:active { + color: $primary; + } + } + } } .vert > .xblock-student_view.is-hidden, diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index 2e53b5611aec..9700101c9367 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -112,6 +112,48 @@ % endif + + ${HTML(fragment.foot_html())} @@ -195,7 +237,9 @@ % endif - ${sequence_title} + % if sequence_title: + ${sequence_title} + % endif diff --git a/lms/templates/preview_menu.html b/lms/templates/preview_menu.html index b9d5d223ddeb..fa26e6bc4f06 100644 --- a/lms/templates/preview_menu.html +++ b/lms/templates/preview_menu.html @@ -107,7 +107,7 @@ diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index 131bbfc8cadc..0e52e3c7f426 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -69,6 +69,12 @@

${unit_title}

% endfor +% if reset_button: +
+ +
+% endif + <%static:require_module_async module_name="js/dateutil_factory" class_name="DateUtilFactory"> DateUtilFactory.transform('.localized-datetime'); diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index 0eaa4f437fad..6fba208f31cc 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -560,6 +560,6 @@ def get_icon(block): """ A function that returns the CSS class representing an icon to use for this particular XBlock (in the courseware navigation bar). Mostly used for Vertical/Unit XBlocks. - It can be overridden by setting `GET_UNIT_ICON_IMPL` to an alternative implementation. + It can be overridden by setting `OVERRIDE_GET_UNIT_ICON` to an alternative implementation. """ return block.get_icon_class() diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index fd23895d5a43..f5df16219843 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -7,6 +7,8 @@ from completion.models import BlockCompletion from completion.services import CompletionService from completion.test_utils import CompletionWaffleTestMixin +from django.conf import settings +from django.test import override_settings from opaque_keys.edx.keys import CourseKey from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -183,6 +185,19 @@ def test_can_mark_block_complete_on_view(self): assert self.completion_service.can_mark_block_complete_on_view(self.html) is True assert self.completion_service.can_mark_block_complete_on_view(self.problem) is False + @override_settings(FEATURES={**settings.FEATURES, 'MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW': True}) + def test_can_mark_library_content_complete_on_view(self): + library = LibraryFactory.create(modulestore=self.store) + lib_vertical = ItemFactory.create(parent=self.sequence, category='vertical', publish_item=False) + library_content_block = ItemFactory.create( + parent=lib_vertical, + category='library_content', + max_count=1, + source_library_id=str(library.location.library_key), + user_id=self.user.id, + ) + self.assertTrue(self.completion_service.can_mark_block_complete_on_view(library_content_block)) + def test_vertical_completion_with_library_content(self): library = LibraryFactory.create(modulestore=self.store) ItemFactory.create(parent=library, category='problem', publish_item=False, user_id=self.user.id) @@ -196,6 +211,9 @@ def test_vertical_completion_with_library_content(self): source_library_id=str(library.location.library_key), user_id=self.user.id, ) + # Library Content Block needs its children to be completed. + self.assertFalse(self.completion_service.can_mark_block_complete_on_view(library_content_block)) + library_content_block.refresh_children() lib_vertical = self.store.get_item(lib_vertical.location) self._bind_course_module(lib_vertical)