From 2c562da9db8d356b6ad1254e36834b85bf767029 Mon Sep 17 00:00:00 2001 From: Paul Aswa Date: Thu, 4 Mar 2021 23:46:46 +0300 Subject: [PATCH 01/12] fix: hide new post indicator on thread open This fixes the removal of the new post indicator in the Discussion XBlock once a thread has been opened. (cherry picked from commit b62478c3655ea6e65167cb15567d178168c71940) --- .../views/discussion_inline_view.js | 1 + .../view/discussion_inline_view_spec.js | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) 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'); + }); }); }); }()); From 7df457f386c18d7e8c2a8deb35d92473bf15d609 Mon Sep 17 00:00:00 2001 From: dylan-grafmyre <75762916+dylan-grafmyre@users.noreply.github.com> Date: Tue, 4 May 2021 09:17:15 -0700 Subject: [PATCH 02/12] fix: prevent a backbone fieldview race condition that can delete user input (#25950) * prevent a backbone fieldview race condition that can delete user input eslint for effected * Fixing quality * Allow _super in fields.js * Fixing new issues * Revert some changes * Fixing errors, formatting * Fix bug * Fix eslint rule allowing _super * Refactor code Co-authored-by: Dylan Grafmyre Co-authored-by: Paulo Viadanna (cherry picked from commit 8995f1bf322e1a9aaced07daedfe22b8e9d657a3) --- lms/static/js/views/fields.js | 98 ++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 41 deletions(-) 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); }, From 48bdb21602255ad538ac5f4df3815ce1a98897e7 Mon Sep 17 00:00:00 2001 From: Arjun Singh Yadav Date: Thu, 1 Jul 2021 00:04:44 +0530 Subject: [PATCH 03/12] fix: set answer notification alignment to bottom (#27022) On focus to the answer submition notification, the notification was scrolled to center of screen. This behaviour was on Chrome(87+) browser. This commits overrides the focus jQuery plugin to set the alignment of answer notifications to the bottom of the viewport. Fix the custom focus jQuery plugin Co-authored-by: Agrendalath (cherry picked from commit 52ab785764b4f1edd33a574dd8979e4f23f97867) --- lms/templates/courseware/courseware.html | 42 ++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index 2e53b5611aec..9391881336b5 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -112,6 +112,48 @@ % endif + + ${HTML(fragment.foot_html())} From eaf7d2e7c82a22e5b796a47755ef72dd79d63bfa Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 1 Jun 2021 19:30:34 +0200 Subject: [PATCH 04/12] fix: handle opening verticals in new tabs with the "View In Studio" button #24020 introduced a workaround for viewing the current vertical in Studio However, replacing `window.location.href` on `click` did not support opening it in a new tab. (cherry picked from commit 01ada0b49fd877b057406f6d4b9c3b0a821b4e65) --- lms/templates/preview_menu.html | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) 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 @@ From 18a703c0cb6dc576786b08947ce76a40ef63c811 Mon Sep 17 00:00:00 2001 From: Piotr Surowiec Date: Thu, 22 Jul 2021 15:52:25 +0200 Subject: [PATCH 05/12] fix: hide unavailable sequence title in course breadcrumbs (#28028) When a sequence title was unavailable for a user (e.g. before starting a timed exam), the `None` value was displayed in course breadcrumbs. This hides it in such cases and ensures that a trailing breadcrumb separator is not displayed. (cherry picked from commit a5d9dd4fb9f4264626776c67c4013a7687214669) --- .../static/sass/edx-pattern-library-shims/_breadcrumbs.scss | 5 +++++ lms/templates/courseware/courseware.html | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) 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/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index 9391881336b5..9700101c9367 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -237,7 +237,9 @@ % endif - ${sequence_title} + % if sequence_title: + ${sequence_title} + % endif From 948b5afe284eee155285ef5da9f4185f73741428 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 16 Jul 2021 17:37:16 +0200 Subject: [PATCH 06/12] fix: handle invalid source library version on course import Importing a course with an invalid source library version (e.g. created on another instance) should not make the import fail. (cherry picked from commit 4b8421dfd80e670d280558692845e29155e482e9) --- .../views/tests/test_import_export.py | 19 +++++++++++---- .../xmodule/modulestore/xml_importer.py | 24 +++++++++++-------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 2f237986c9c0..2104c713d4dd 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) 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 From 66637de65ca25eb7ee01e2f406beb4e4ef6cb5b4 Mon Sep 17 00:00:00 2001 From: Josh McLaughlin Date: Mon, 19 Apr 2021 07:48:49 -0700 Subject: [PATCH 07/12] fix: Use UserProfile.meta for city field if set in the profile information report There are two distinct ways to store a city for users in edx-platform: one directly in UserProfile.city, and another in UserProfile.meta. Depending on configuration, both fields can be used. Though the UserProfile.meta['city'] field is not set unless specifically configured, so we choose this over the UserProfile.city field when generating the student features report. Additional details and discussion available on edx/edx-platform#23171 (cherry picked from commit ddffeaac4a0470ea6cca03b86615fe1e119cccb0) --- lms/djangoapps/instructor_analytics/basic.py | 8 ++++++++ 1 file changed, 8 insertions(+) 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, From 43814005dce4af2dc2315715c68d696c7f950b69 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 9 Apr 2021 00:14:53 +0200 Subject: [PATCH 08/12] feat: support adding custom editors to Studio This: 1. Introduces a variable for the Course Outline view in Studio. A custom theme can override it to add new editors. 2. Exports a function for creating new editor modals. A custom theme can use it to create editors without adding boilerplate code. 3. Adds a pluggable override for XBlock fields that are passed to the Studio. Without this, custom editors in Studio cannot retrieve values of XBlock fields. (cherry picked from commit e633cc9c249a31f308ac67036b7a62609fb29499) --- cms/djangoapps/contentstore/views/item.py | 9 +++++++++ cms/envs/common.py | 1 + cms/envs/production.py | 3 +++ cms/static/js/views/modals/course_outline_modals.js | 13 +++++++++++++ cms/static/js/views/pages/course_outline.js | 5 ++++- openedx/core/lib/xblock_utils/__init__.py | 2 +- 6 files changed, 31 insertions(+), 2 deletions(-) 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/envs/common.py b/cms/envs/common.py index ce4e36a6f4cf..307c250035be 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -818,6 +818,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/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() From 13c240fb65e0aaedf0f6803853ea4f766d1b149d Mon Sep 17 00:00:00 2001 From: Piotr Surowiec Date: Tue, 16 Nov 2021 18:42:08 +0100 Subject: [PATCH 09/12] fix: do not remove comments from XML during course import (#28557) This is a follow-up to edx#1087, which reverted this change. According to the PR comments, parsing strings with XML comments inside them was causing errors. This does not seem to be the case anymore - these strings are just hidden when the block is rendered, but they are not breaking XBlocks. This also handles (ignores) the comments that could be added directly to the LibraryContentBlock in the XML export by users. (cherry picked from commit 4bf829dcbd7b02b825678a11eee216a84e4abc04) --- .../views/tests/test_import_export.py | 20 +++- .../xmodule/xmodule/library_content_module.py | 19 +++- common/lib/xmodule/xmodule/modulestore/xml.py | 3 +- .../xmodule/tests/test_library_content.py | 94 ++++++++++++------- common/lib/xmodule/xmodule/xml_module.py | 4 +- 5 files changed, 98 insertions(+), 42 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 2104c713d4dd..076938532463 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -1108,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/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index c303f022c73a..1263c864ab75 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -12,6 +12,7 @@ import bleach 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 @@ -664,10 +665,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/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 8a287acf88f2..628f471e88eb 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -74,18 +74,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,44 +90,78 @@ 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) + + 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 - # 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 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) - - # 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 + imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None, self.id_generator) + + 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) class LibraryContentBlockTestMixin: 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): From e232726e0f78f76d2608348cfb881c09237b199d Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Sun, 25 Jul 2021 02:06:51 +0200 Subject: [PATCH 10/12] feat: allow marking Library Content Block as complete on view edx/edx-platform#24365 has changed the completion mode of these blocks. Before Koa, it was sufficient to view the block to get a completion checkmark. Since Koa, all children of the block must be completed. This adds a toggle to change the completion behavior back to the previous one so that the user experience can be consistent if needed. --- cms/envs/common.py | 13 +++++++++++++ .../xmodule/xmodule/library_content_module.py | 15 ++++++++++++++- lms/envs/common.py | 15 ++++++++++++++- .../completion_integration/test_services.py | 18 ++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 307c250035be..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 diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 1263c864ab75..56262a18b58d 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -10,6 +10,8 @@ from gettext import ngettext 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 @@ -115,7 +117,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"), 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/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) From f36d08c493b28deb1cf575eadbf0877407b83a14 Mon Sep 17 00:00:00 2001 From: tinumide Date: Wed, 1 Sep 2021 13:47:39 +0000 Subject: [PATCH 11/12] feat: add reset option to Randomized Content Block This adds a reset option to a Randomized Content Block (LibraryContentBlock) to allow an OpenEdx user to reset questions in a Randomized Content Block. This is achieved by adding a `Reset Problems` button to the UI of a vertical with randomized content, and a reset handler to process the request. --- .../public/js/library_content_reset.js | 12 ++++++ .../xmodule/xmodule/library_content_module.py | 36 ++++++++++++++++- .../xmodule/tests/test_library_content.py | 39 +++++++++++++++++++ .../sass/course/courseware/_courseware.scss | 11 ++++++ lms/templates/vert_module.html | 6 +++ 5 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js 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/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 56262a18b58d..29e772f8bee1 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -8,6 +8,7 @@ import random from copy import copy from gettext import ngettext +from rest_framework import status import bleach from django.conf import settings @@ -21,7 +22,7 @@ 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 @@ -177,6 +178,14 @@ def completion_mode(cls): # pylint: disable=no-self-argument 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): @@ -347,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 @@ -384,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): diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 628f471e88eb..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() @@ -164,6 +168,7 @@ def test_xml_import_with_comments(self): self._verify_xblock_properties(imported_lc_block) +@ddt.ddt class LibraryContentBlockTestMixin: """ Basic unit tests for LibraryContentBlock @@ -378,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): @@ -396,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/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/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'); From dcdcc2557b14b429034094db6d2642fce3739746 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Mon, 25 Oct 2021 21:15:45 +0200 Subject: [PATCH 12/12] fix: do not index solutions in CAPA blocks Most tags that could contain solutions or hints were already being removed, but the regex did not include the case when they contained attributes. --- common/lib/xmodule/xmodule/capa_module.py | 18 +++++-- .../xmodule/xmodule/tests/test_capa_module.py | 47 +++++++++++-------- 2 files changed, 40 insertions(+), 25 deletions(-) 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/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)